Support for configuring end user consent requirement for clients

Developers can decide if consent is required and for everyone else it is *always* required.
This commit is contained in:
Andreas Svanberg 2025-04-16 16:34:34 +02:00
parent 8307bc4906
commit ea5c3a1c00
Signed by: ansv7779
GPG Key ID: 729B051CFFD42F92
17 changed files with 193 additions and 25 deletions

@ -12,6 +12,7 @@ import org.springframework.security.config.Customizer;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.DelegatingPasswordEncoder;
import org.springframework.security.crypto.password.NoOpPasswordEncoder;
@ -28,6 +29,7 @@ import se.su.dsv.oauth2.shibboleth.Entitlement;
import se.su.dsv.oauth2.shibboleth.ShibbolethConfigurer;
import se.su.dsv.oauth2.shibboleth.ShibbolethTokenPopulator;
import se.su.dsv.oauth2.staging.StagingSecurityConfigurer;
import se.su.dsv.oauth2.web.client.DeveloperAccessCheck;
import java.util.Map;
import java.util.Optional;
@ -162,4 +164,20 @@ public class AuthorizationServer extends SpringBootServletInitializer {
return delegatingPasswordEncoder;
}
@Bean
public DeveloperAccessCheck developerAccessCheck(Config config) {
final String developerAuthority = Entitlement.asAuthority(config.developerEntitlement());
return authentication -> {
if (!authentication.isAuthenticated()) {
return false;
}
for (GrantedAuthority authority : authentication.getAuthorities()) {
if (developerAuthority.equals(authority.getAuthority())) {
return true;
}
}
return false;
};
}
}

@ -34,7 +34,7 @@ public class EmbeddedConfiguration {
String scopeString = System.getenv("CLIENT_SCOPES");
return new ClientRow(clientId, clientId, clientId, "dev@localhost",
redirectUri, scopeString, clientSecret);
redirectUri, scopeString, clientSecret, false);
}
private static class InMemoryClientrepository implements ClientRepository {

@ -29,7 +29,7 @@ public class JDBCClientRepository implements ClientRepository {
@Override
public List<ClientRow> getClients(final Principal owner) {
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret FROM v2_client WHERE id IN (SELECT client_id FROM v2_client_owner WHERE owner = :owner)")
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret, requires_consent FROM v2_client WHERE id IN (SELECT client_id FROM v2_client_owner WHERE owner = :owner)")
.param("owner", owner.getName())
.query(ClientRow.class)
.list();
@ -45,7 +45,7 @@ public class JDBCClientRepository implements ClientRepository {
@Override
public Optional<ClientRow> getClientRowById(final String id) {
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret FROM v2_client WHERE id = :id")
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret, requires_consent FROM v2_client WHERE id = :id")
.param("id", id)
.query(ClientRow.class)
.optional();
@ -53,7 +53,7 @@ public class JDBCClientRepository implements ClientRepository {
@Override
public Optional<ClientRow> getClientRowByClientId(final String clientId) {
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret FROM v2_client WHERE client_id = :clientId")
return getJdbc().sql("SELECT id, client_id, name, contact_email, redirect_uri, scopes, client_secret, requires_consent FROM v2_client WHERE client_id = :clientId")
.param("clientId", clientId)
.query(ClientRow.class)
.optional();
@ -62,14 +62,15 @@ public class JDBCClientRepository implements ClientRepository {
@Override
public void addNewClient(final ClientRow clientRow) {
getJdbc().sql("""
INSERT INTO v2_client (id, client_id, client_secret, name, redirect_uri, contact_email, scopes)
VALUES (:id, :clientId, :clientSecret, :name, :redirectUri, :contactEmail, :scopes)
INSERT INTO v2_client (id, client_id, client_secret, name, redirect_uri, contact_email, scopes, requires_consent)
VALUES (:id, :clientId, :clientSecret, :name, :redirectUri, :contactEmail, :scopes, :requiresConsent)
ON DUPLICATE KEY UPDATE
client_id = VALUES(client_id),
client_secret = VALUES(client_secret),
name = VALUES(name),
redirect_uri = VALUES(redirect_uri),
contact_email = VALUES(contact_email),
requires_consent = VALUES(requires_consent),
scopes = VALUES(scopes);
""")
.paramSource(clientRow)

@ -11,6 +11,7 @@ public record Client(
String redirectUri,
boolean isPublic,
Set<String> scopes,
List<String> owners)
List<String> owners,
boolean requiresConsent)
{
}

@ -8,10 +8,11 @@ public record ClientData(
URI redirectURI,
boolean isPublic,
Set<String> scopes,
String contactEmail)
String contactEmail,
boolean requiresConsent)
{
// When creating a resource server client
public ClientData(String clientName, String contactEmail) {
this(clientName, null, false, Set.of(), contactEmail);
this(clientName, null, false, Set.of(), contactEmail, true);
}
}

@ -44,7 +44,7 @@ public class ClientManager implements RegisteredClientRepository, ClientManageme
String scopeString = toScopeString(clientData);
ClientRow clientRow = new ClientRow(id, clientId, clientData.clientName(), clientData.contactEmail(),
redirectURI, scopeString, encodedClientSecret);
redirectURI, scopeString, encodedClientSecret, clientData.requiresConsent());
clientRepository.addNewClient(clientRow);
@ -66,7 +66,7 @@ public class ClientManager implements RegisteredClientRepository, ClientManageme
ClientRow updated = new ClientRow(id, currentClient.clientId(), clientData.clientName(),
clientData.contactEmail(), getRedirectUri(clientData),
toScopeString(clientData), currentClient.clientSecret());
toScopeString(clientData), currentClient.clientSecret(), clientData.requiresConsent());
clientRepository.update(updated);
return toClient(updated);
}
@ -133,7 +133,8 @@ public class ClientManager implements RegisteredClientRepository, ClientManageme
clientRow.redirectUri(),
isPublic,
scopes,
owners);
owners,
clientRow.requiresConsent());
}
// Used by various components of the OAuth 2.0 infrastructure to upgrade
@ -193,7 +194,10 @@ public class ClientManager implements RegisteredClientRepository, ClientManageme
redirectUris.add(clientRow.redirectUri());
}
})
.clientSettings(ClientSettings.builder().requireProofKey(clientRow.isPublic()).build())
.clientSettings(ClientSettings.builder()
.requireAuthorizationConsent(clientRow.requiresConsent())
.requireProofKey(clientRow.isPublic())
.build())
.scopes(currentScopes -> currentScopes.addAll(clientRow.scopeSet()))
.build();
}

@ -12,7 +12,8 @@ public record ClientRow(
String contactEmail,
String redirectUri,
String scopes,
String clientSecret)
String clientSecret,
boolean requiresConsent)
{
public Set<String> scopeSet() {
if (scopes == null) {

@ -1,6 +1,7 @@
package se.su.dsv.oauth2.web.client;
import org.springframework.http.HttpStatus;
import org.springframework.security.core.Authentication;
import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings;
import org.springframework.security.web.csrf.CsrfToken;
import org.springframework.stereotype.Controller;
@ -28,13 +29,16 @@ public class ClientAdminController {
private final ClientManagementService clientManagementService;
private final AuthorizationServerSettings authorizationServerSettings;
private final DeveloperAccessCheck developerAccessCheck;
public ClientAdminController(
final ClientManagementService clientManagementService,
AuthorizationServerSettings authorizationServerSettings)
AuthorizationServerSettings authorizationServerSettings,
DeveloperAccessCheck developerAccessCheck)
{
this.clientManagementService = clientManagementService;
this.authorizationServerSettings = authorizationServerSettings;
this.developerAccessCheck = developerAccessCheck;
}
@GetMapping
@ -46,13 +50,13 @@ public class ClientAdminController {
@GetMapping("/new")
public String newClient(Model model) {
model.addAttribute("newClient", new NewClientRequest(null, null, null, null, null));
model.addAttribute("newClient", NewClientRequest.empty());
return "admin/client/new";
}
@PostMapping("/new")
public String createClient(
Principal principal,
Authentication principal,
Model model,
RedirectAttributes redirectAttributes,
@ModelAttribute("newClient") NewClientRequest newClientRequest,
@ -62,12 +66,14 @@ public class ClientAdminController {
model.addAttribute("errors", bindingResult.getAllErrors());
return "admin/client/new";
}
boolean requiresConsent = determineConsentRequired(principal, newClientRequest);
ClientData clientData = new ClientData(
newClientRequest.name(),
newClientRequest.redirectUri(),
newClientRequest.isPublic(),
newClientRequest.scopes(),
newClientRequest.contact());
newClientRequest.contact(),
requiresConsent);
NewClient newClient = clientManagementService.createClient(principal, clientData);
redirectAttributes.addFlashAttribute("message", "New client created");
redirectAttributes.addFlashAttribute("clientId", newClient.clientId());
@ -132,7 +138,7 @@ public class ClientAdminController {
@PostMapping("/{id}/edit")
public String editClient(
@PathVariable("id") String id,
Principal principal,
Authentication principal,
Model model,
RedirectAttributes redirectAttributes,
@ModelAttribute("newClient") NewClientRequest newClientRequest,
@ -142,17 +148,29 @@ public class ClientAdminController {
model.addAttribute("errors", bindingResult.getAllErrors());
return "admin/client/edit";
}
boolean requiresConsent = determineConsentRequired(principal, newClientRequest);
ClientData clientData = new ClientData(
newClientRequest.name(),
newClientRequest.redirectUri(),
newClientRequest.isPublic(),
newClientRequest.scopes(),
newClientRequest.contact());
newClientRequest.contact(),
requiresConsent);
final Client updatedClient = clientManagementService.updateClient(principal, id, clientData);
redirectAttributes.addFlashAttribute("message", "Client updated");
return "redirect:/admin/client/" + updatedClient.id();
}
private boolean determineConsentRequired(final Authentication principal, final NewClientRequest newClientRequest) {
if (hasDeveloperAccess(principal)) {
// Allow developers to decide if consent is required
return newClientRequest.requiresConsentBoolean();
} else {
// For everyone else it's always required
return true;
}
}
@ModelAttribute
public CsrfToken csrfToken(CsrfToken token) {
return token;
@ -162,4 +180,9 @@ public class ClientAdminController {
public AuthorizationServerSettings authorizationServerSettings() {
return authorizationServerSettings;
}
@ModelAttribute(name = "developerAccess")
public boolean hasDeveloperAccess(Authentication principal) {
return developerAccessCheck.test(principal);
}
}

@ -0,0 +1,9 @@
package se.su.dsv.oauth2.web.client;
import org.springframework.security.core.Authentication;
import java.util.function.Predicate;
@FunctionalInterface
public interface DeveloperAccessCheck extends Predicate<Authentication> {
}

@ -14,7 +14,8 @@ public record NewClientRequest(
String contact,
URI redirectUri,
String public_,
String scope)
String scope,
String requiresConsent)
{
public static NewClientRequest from(final Client client) {
return new NewClientRequest(
@ -22,7 +23,12 @@ public record NewClientRequest(
client.contact(),
URI.create(client.redirectUri()),
client.isPublic() ? "on" : null,
String.join("\r\n", client.scopes()));
String.join("\r\n", client.scopes()),
client.requiresConsent() ? "on" : null);
}
public static NewClientRequest empty() {
return new NewClientRequest(null, null, null, null, null, null);
}
public boolean isPublic() {
@ -40,4 +46,8 @@ public record NewClientRequest(
public String redirectUriString() {
return redirectUri == null ? "" : redirectUri.toString();
}
public boolean requiresConsentBoolean() {
return "on".equals(requiresConsent);
}
}

@ -0,0 +1,6 @@
ALTER TABLE `v2_client`
# Defaulting user consent to true so that future clients will require user consent
ADD COLUMN `requires_consent` BOOLEAN NOT NULL DEFAULT TRUE;
# Making sure current clients stay the same
UPDATE `v2_client` SET `requires_consent` = FALSE;

@ -7,6 +7,7 @@
@param Client client
@param String feedback
@param List<ObjectError> errors
@param boolean developerAccess
@template.base(title = "Edit client " + client.name(), content = @`
<nav>
@ -50,6 +51,16 @@
<label class="form-label" for="redirectUri">Redirect URI</label>
<input class="form-control" name="redirectUri" id="redirectUri" type="url" value="${newClient.redirectUriString()}">
</div>
@if (developerAccess)
<div class="mb-3 form-check">
<input class="form-check-input" type="checkbox" id="consent" name="requiresConsent" checked="${newClient.requiresConsentBoolean()}">
<label class="form-check-label" for="consent">Require user consent</label>
<br>
<small class="text-muted">
If checked, the user will be asked to consent to grant access to this client.
</small>
</div>
@endif
<div class="mb-3 form-check">
<input class="form-check-input" type="checkbox" id="public" name="public_" checked="${newClient.isPublic()}">
<label class="form-check-label" for="public">Public client</label>

@ -5,6 +5,7 @@
@param se.su.dsv.oauth2.web.client.NewClientRequest newClient
@param String feedback
@param List<ObjectError> errors
@param boolean developerAccess
@template.base(title = "Create new client", content = @`
<nav>
@ -59,6 +60,16 @@
<label class="form-label" for="redirectUri">Redirect URI</label>
<input class="form-control" name="redirectUri" id="redirectUri" type="url" value="${newClient.redirectUriString()}">
</div>
@if (developerAccess)
<div class="mb-3 form-check">
<input class="form-check-input" type="checkbox" id="consent" name="requiresConsent" checked="${newClient.requiresConsentBoolean()}">
<label class="form-check-label" for="consent">Require user consent</label>
<br>
<small class="text-muted">
If checked, the user will be asked to consent to grant access to this client.
</small>
</div>
@endif
<div class="mb-3 form-check">
<input class="form-check-input" type="checkbox" id="public" name="public_" checked="${newClient.isPublic()}">
<label class="form-check-label" for="public">Public client</label>

@ -67,6 +67,8 @@
<dt>Redirect URI</dt>
<dd>${client.redirectUri()}</dd>
@endif
<dt>Requires consent</dt>
<dd>${client.requiresConsent() ? "Yes" : "No"}</dd>
<dt>Public client</dt>
<dd>${client.isPublic() ? "Yes" : "No"}</dd>
<dt>Scopes</dt>

@ -42,7 +42,7 @@ public class PublicClientCodeFlowTest extends AbstractMetadataCodeFlowTest {
public void register_public_client() {
Set<String> scopes = Set.of("openid", "offline_access");
ClientData clientData = new ClientData("public-client", URI.create(REDIRECT_URI), true,
scopes, "admin@localhost");
scopes, "admin@localhost", false);
newClient = clientManagementService.createClient(() -> "admin", clientData);
}

@ -27,7 +27,7 @@ public class ResourceServerRegisteredClientTest extends AbstractMetadataCodeFlow
@Test
public void client_authenticates_then_resource_server_introspects() throws Exception {
ClientData application = new ClientData("client", URI.create("http://myapplication.local/oauth2"), false, Set.of(), "admin@localhost");
ClientData application = new ClientData("client", URI.create("http://myapplication.local/oauth2"), false, Set.of(), "admin@localhost", false);
ClientData resourceServer = new ClientData("resource-server", "admin@localhost");
NewClient application1 = clientManagementService.createClient(() -> "admin", application);
NewClient resourceServer1 = clientManagementService.createClient(() -> "admin", resourceServer);

@ -5,12 +5,18 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
import org.springframework.test.annotation.Rollback;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.transaction.annotation.Transactional;
import org.testcontainers.containers.MariaDBContainer;
import se.su.dsv.oauth2.admin.Client;
import se.su.dsv.oauth2.admin.ClientManagementService;
import java.util.List;
import java.util.Set;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
@ -20,11 +26,15 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
import static se.su.dsv.oauth2.ShibbolethRequestProcessor.remoteUser;
@SpringBootTest(properties = {
"se.su.dsv.oauth2." + ClientAdminControllerTest.ADMIN_ENTITLEMENT + "-entitlement=admin"
"se.su.dsv.oauth2.admin-entitlement=" + ClientAdminControllerTest.ADMIN_ENTITLEMENT,
"se.su.dsv.oauth2.developer-entitlement=" + ClientAdminControllerTest.DEVELOPER_ENTITLEMENT
})
@AutoConfigureMockMvc
@Transactional
@Rollback
public class ClientAdminControllerTest {
public static final String ADMIN_ENTITLEMENT = "admin";
public static final String DEVELOPER_ENTITLEMENT = "developer";
@ServiceConnection
static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11");
@ -32,6 +42,9 @@ public class ClientAdminControllerTest {
@Autowired
MockMvc mockMvc;
@Autowired
ClientManagementService clientManagementService;
@Test
public void register_new_client() throws Exception {
String name = "My client";
@ -62,4 +75,61 @@ public class ClientAdminControllerTest {
.andExpect(content().string(containsString(redirectUri)))
.andExpect(content().string(stringContainsInOrder(scopes)));
}
@Test
public void register_client_without_developer_access_always_require_consent() throws Exception {
String name = "My client";
String contactEmail = DEVELOPER_ENTITLEMENT + "@3rd-party.example";
String redirectUri = "https://3rd-party.example/oauth2/callback";
String principal = "third-party-" + DEVELOPER_ENTITLEMENT;
MvcResult creationResult = mockMvc.perform(post("/admin/client/new")
.with(csrf())
.with(remoteUser(principal)
.entitlement(ADMIN_ENTITLEMENT))
.formField("name", name)
.formField("contact", contactEmail)
.formField("redirectUri", redirectUri))
.andExpect(status().isFound())
.andExpect(flash().attribute("clientSecret", not(blankOrNullString())))
.andReturn();
String viewClientUrl = creationResult.getResponse().getRedirectedUrl();
assertNotNull(viewClientUrl);
List<Client> clients = clientManagementService.getClients(() -> principal);
assertThat(clients, hasSize(1));
Client client = clients.get(0);
assertThat(client.requiresConsent(), is(true));
}
@Test
public void register_client_with_developer_access_can_bypass_consent() throws Exception {
String name = "My client";
String contactEmail = DEVELOPER_ENTITLEMENT + "@3rd-party.example";
String redirectUri = "https://3rd-party.example/oauth2/callback";
String principal = "third-party-" + DEVELOPER_ENTITLEMENT;
MvcResult creationResult = mockMvc.perform(post("/admin/client/new")
.with(csrf())
.with(remoteUser(principal)
.entitlement(ADMIN_ENTITLEMENT)
.entitlement(DEVELOPER_ENTITLEMENT))
.formField("name", name)
.formField("contact", contactEmail)
.formField("redirectUri", redirectUri))
.andExpect(status().isFound())
.andExpect(flash().attribute("clientSecret", not(blankOrNullString())))
.andReturn();
String viewClientUrl = creationResult.getResponse().getRedirectedUrl();
assertNotNull(viewClientUrl);
List<Client> clients = clientManagementService.getClients(() -> principal);
assertThat(clients, hasSize(1));
Client client = clients.get(0);
assertThat(client.requiresConsent(), is(false));
}
}