Skip to content

Commit 5a4ada0

Browse files
rwinchrwinch
authored
Merge pull request #19004 from rwinch/CredentialRecordOwnerAuthorizationManager
Add CredentialRecordOwnerAuthorizationManager
2 parents ac63cf4 + a856baa commit 5a4ada0

6 files changed

Lines changed: 408 additions & 1 deletion

File tree

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurer.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.springframework.security.web.webauthn.authentication.PublicKeyCredentialRequestOptionsFilter;
3737
import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationFilter;
3838
import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationProvider;
39+
import org.springframework.security.web.webauthn.management.CredentialRecordOwnerAuthorizationManager;
3940
import org.springframework.security.web.webauthn.management.MapPublicKeyCredentialUserEntityRepository;
4041
import org.springframework.security.web.webauthn.management.MapUserCredentialRepository;
4142
import org.springframework.security.web.webauthn.management.PublicKeyCredentialUserEntityRepository;
@@ -166,6 +167,8 @@ public void configure(H http) throws Exception {
166167
new ProviderManager(new WebAuthnAuthenticationProvider(rpOperations, userDetailsService)));
167168
WebAuthnRegistrationFilter webAuthnRegistrationFilter = new WebAuthnRegistrationFilter(userCredentials,
168169
rpOperations);
170+
webAuthnRegistrationFilter.setDeleteCredentialAuthorizationManager(
171+
new CredentialRecordOwnerAuthorizationManager(userCredentials, userEntities));
169172
PublicKeyCredentialCreationOptionsFilter creationOptionsFilter = new PublicKeyCredentialCreationOptionsFilter(
170173
rpOperations);
171174
if (creationOptionsRepository != null) {

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurerTests.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,15 @@
4242
import org.springframework.security.web.FilterChainProxy;
4343
import org.springframework.security.web.SecurityFilterChain;
4444
import org.springframework.security.web.authentication.ui.DefaultResourcesFilter;
45+
import org.springframework.security.web.webauthn.api.Bytes;
46+
import org.springframework.security.web.webauthn.api.ImmutablePublicKeyCredentialUserEntity;
4547
import org.springframework.security.web.webauthn.api.PublicKeyCredentialCreationOptions;
48+
import org.springframework.security.web.webauthn.api.TestCredentialRecords;
4649
import org.springframework.security.web.webauthn.api.TestPublicKeyCredentialCreationOptions;
50+
import org.springframework.security.web.webauthn.management.MapPublicKeyCredentialUserEntityRepository;
51+
import org.springframework.security.web.webauthn.management.MapUserCredentialRepository;
52+
import org.springframework.security.web.webauthn.management.PublicKeyCredentialUserEntityRepository;
53+
import org.springframework.security.web.webauthn.management.UserCredentialRepository;
4754
import org.springframework.security.web.webauthn.management.WebAuthnRelyingPartyOperations;
4855
import org.springframework.security.web.webauthn.registration.HttpSessionPublicKeyCredentialCreationOptionsRepository;
4956
import org.springframework.test.web.servlet.MockMvc;
@@ -56,6 +63,7 @@
5663
import static org.mockito.Mockito.mock;
5764
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication;
5865
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
66+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
5967
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
6068
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
6169
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
@@ -247,6 +255,24 @@ public void webauthnWhenConfiguredMessageConverter() throws Exception {
247255
.andExpect(content().string(expectedBody));
248256
}
249257

258+
@Test
259+
void webauthnWhenDeleteAndCredentialBelongsToUserThenNoContent() throws Exception {
260+
this.spring.register(DeleteCredentialConfiguration.class).autowire();
261+
this.mvc
262+
.perform(delete("/webauthn/register/" + DeleteCredentialConfiguration.CREDENTIAL_ID_BASE64URL)
263+
.with(authentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"))))
264+
.andExpect(status().isNoContent());
265+
}
266+
267+
@Test
268+
void webauthnWhenDeleteAndCredentialBelongsToDifferentUserThenForbidden() throws Exception {
269+
this.spring.register(DeleteCredentialConfiguration.class).autowire();
270+
this.mvc
271+
.perform(delete("/webauthn/register/" + DeleteCredentialConfiguration.CREDENTIAL_ID_BASE64URL)
272+
.with(authentication(new TestingAuthenticationToken("other-user", "password", "ROLE_USER"))))
273+
.andExpect(status().isForbidden());
274+
}
275+
250276
@Configuration
251277
@EnableWebSecurity
252278
static class ConfigCredentialCreationOptionsRepository {
@@ -417,4 +443,47 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
417443

418444
}
419445

446+
@Configuration
447+
@EnableWebSecurity
448+
static class DeleteCredentialConfiguration {
449+
450+
static final String CREDENTIAL_ID_BASE64URL = "NauGCN7bZ5jEBwThcde51g";
451+
452+
static final Bytes USER_ENTITY_ID = Bytes.fromBase64("vKBFhsWT3gQnn-gHdT4VXIvjDkVXVYg5w8CLGHPunMM");
453+
454+
@Bean
455+
UserDetailsService userDetailsService() {
456+
return new InMemoryUserDetailsManager();
457+
}
458+
459+
@Bean
460+
WebAuthnRelyingPartyOperations webAuthnRelyingPartyOperations() {
461+
return mock(WebAuthnRelyingPartyOperations.class);
462+
}
463+
464+
@Bean
465+
UserCredentialRepository userCredentialRepository() {
466+
MapUserCredentialRepository repository = new MapUserCredentialRepository();
467+
repository.save(TestCredentialRecords.userCredential().build());
468+
return repository;
469+
}
470+
471+
@Bean
472+
PublicKeyCredentialUserEntityRepository userEntityRepository() {
473+
MapPublicKeyCredentialUserEntityRepository repository = new MapPublicKeyCredentialUserEntityRepository();
474+
repository.save(ImmutablePublicKeyCredentialUserEntity.builder()
475+
.name("user")
476+
.id(USER_ENTITY_ID)
477+
.displayName("User")
478+
.build());
479+
return repository;
480+
}
481+
482+
@Bean
483+
SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
484+
return http.csrf(AbstractHttpConfigurer::disable).webAuthn(Customizer.withDefaults()).build();
485+
}
486+
487+
}
488+
420489
}

‎web/src/main/java/org/springframework/security/web/webauthn/management/CredentialRecordOwnerAuthorizationManager.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.web.webauthn.management;
18+
19+
import java.util.function.Supplier;
20+
21+
import org.springframework.security.authorization.AuthenticatedAuthorizationManager;
22+
import org.springframework.security.authorization.AuthorizationDecision;
23+
import org.springframework.security.authorization.AuthorizationManager;
24+
import org.springframework.security.core.Authentication;
25+
import org.springframework.security.web.webauthn.api.Bytes;
26+
import org.springframework.security.web.webauthn.api.CredentialRecord;
27+
import org.springframework.security.web.webauthn.api.PublicKeyCredentialUserEntity;
28+
import org.springframework.util.Assert;
29+
30+
/**
31+
* An {@link AuthorizationManager} that grants access when the {@link CredentialRecord}
32+
* identified by the provided credential id is owned by the currently authenticated user.
33+
*
34+
* <p>
35+
* Per the <a href="https://www.w3.org/TR/webauthn-3/#credential-id">WebAuthn
36+
* specification</a>, a credential id must contain at least 16 bytes with at least 100
37+
* bits of entropy, making it practically unguessable. The specification also advises that
38+
* credential ids should be kept private, as exposing them can leak personally identifying
39+
* information (see
40+
* <a href="https://www.w3.org/TR/webauthn-3/#sctn-credential-id-privacy-leak">§ 14.6.3
41+
* Privacy leak via credential IDs</a>). This {@link AuthorizationManager} is therefore
42+
* intended as defense in depth: even if a credential id were somehow exposed, an
43+
* unauthorized user could not delete another user's credential.
44+
*
45+
* @author Rob Winch
46+
* @since 6.5.10
47+
*/
48+
public final class CredentialRecordOwnerAuthorizationManager implements AuthorizationManager<Bytes> {
49+
50+
private final AuthenticatedAuthorizationManager<Bytes> authenticatedAuthorizationManager = AuthenticatedAuthorizationManager
51+
.authenticated();
52+
53+
private final UserCredentialRepository userCredentials;
54+
55+
private final PublicKeyCredentialUserEntityRepository userEntities;
56+
57+
/**
58+
* Creates a new instance.
59+
* @param userCredentials the {@link UserCredentialRepository} to use
60+
* @param userEntities the {@link PublicKeyCredentialUserEntityRepository} to use
61+
*/
62+
public CredentialRecordOwnerAuthorizationManager(UserCredentialRepository userCredentials,
63+
PublicKeyCredentialUserEntityRepository userEntities) {
64+
Assert.notNull(userCredentials, "userCredentials cannot be null");
65+
Assert.notNull(userEntities, "userEntities cannot be null");
66+
this.userCredentials = userCredentials;
67+
this.userEntities = userEntities;
68+
}
69+
70+
@Override
71+
public AuthorizationDecision check(Supplier<Authentication> authentication, Bytes credentialId) {
72+
AuthorizationDecision decision = this.authenticatedAuthorizationManager.check(authentication, credentialId);
73+
if (!decision.isGranted()) {
74+
return decision;
75+
}
76+
Authentication auth = authentication.get();
77+
CredentialRecord credential = this.userCredentials.findByCredentialId(credentialId);
78+
if (credential == null) {
79+
return new AuthorizationDecision(false);
80+
}
81+
if (credential.getUserEntityUserId() == null) {
82+
return new AuthorizationDecision(false);
83+
}
84+
PublicKeyCredentialUserEntity userEntity = this.userEntities.findByUsername(auth.getName());
85+
if (userEntity == null) {
86+
return new AuthorizationDecision(false);
87+
}
88+
return new AuthorizationDecision(credential.getUserEntityUserId().equals(userEntity.getId()));
89+
}
90+
91+
}

‎web/src/main/java/org/springframework/security/web/webauthn/registration/WebAuthnRegistrationFilter.java‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.web.webauthn.registration;
1818

1919
import java.io.IOException;
20+
import java.util.function.Supplier;
2021

2122
import com.fasterxml.jackson.databind.json.JsonMapper;
2223
import jakarta.servlet.FilterChain;
@@ -34,6 +35,12 @@
3435
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
3536
import org.springframework.http.server.ServletServerHttpRequest;
3637
import org.springframework.http.server.ServletServerHttpResponse;
38+
import org.springframework.security.authorization.AuthorizationManager;
39+
import org.springframework.security.authorization.AuthorizationResult;
40+
import org.springframework.security.authorization.SingleResultAuthorizationManager;
41+
import org.springframework.security.core.Authentication;
42+
import org.springframework.security.core.context.SecurityContextHolder;
43+
import org.springframework.security.core.context.SecurityContextHolderStrategy;
3744
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
3845
import org.springframework.security.web.util.matcher.RequestMatcher;
3946
import org.springframework.security.web.webauthn.api.Bytes;
@@ -87,6 +94,9 @@ public class WebAuthnRegistrationFilter extends OncePerRequestFilter {
8794

8895
private final UserCredentialRepository userCredentials;
8996

97+
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
98+
.getContextHolderStrategy();
99+
90100
private HttpMessageConverter<Object> converter = new MappingJackson2HttpMessageConverter(
91101
JsonMapper.builder().addModule(new WebauthnJackson2Module()).build());
92102

@@ -98,6 +108,9 @@ public class WebAuthnRegistrationFilter extends OncePerRequestFilter {
98108
private RequestMatcher removeCredentialMatcher = PathPatternRequestMatcher.withDefaults()
99109
.matcher(HttpMethod.DELETE, "/webauthn/register/{id}");
100110

111+
private AuthorizationManager<Bytes> deleteCredentialAuthorizationManager = SingleResultAuthorizationManager
112+
.denyAll();
113+
101114
public WebAuthnRegistrationFilter(UserCredentialRepository userCredentials,
102115
WebAuthnRelyingPartyOperations rpOptions) {
103116
Assert.notNull(userCredentials, "userCredentials must not be null");
@@ -132,6 +145,42 @@ public void setRemoveCredentialMatcher(RequestMatcher removeCredentialMatcher) {
132145
this.removeCredentialMatcher = removeCredentialMatcher;
133146
}
134147

148+
/**
149+
* Sets the {@link AuthorizationManager} used to authorize the delete credential
150+
* operation. The object being authorized is the credential id as {@link Bytes}. By
151+
* default, all delete requests are denied.
152+
*
153+
* <p>
154+
* Per the <a href="https://www.w3.org/TR/webauthn-3/#credential-id">WebAuthn
155+
* specification</a>, a credential id must contain at least 16 bytes with at least 100
156+
* bits of entropy, making it practically unguessable. The specification also advises
157+
* that credential ids should be kept private, as exposing them can leak personally
158+
* identifying information (see
159+
* <a href="https://www.w3.org/TR/webauthn-3/#sctn-credential-id-privacy-leak">§
160+
* 14.6.3 Privacy leak via credential IDs</a>). This {@link AuthorizationManager} is
161+
* therefore intended as defense in depth: even if a credential id were somehow
162+
* exposed, an unauthorized user could not delete another user's credential.
163+
* @param deleteCredentialAuthorizationManager the {@link AuthorizationManager} to use
164+
* @since 6.5.10
165+
*/
166+
public void setDeleteCredentialAuthorizationManager(
167+
AuthorizationManager<Bytes> deleteCredentialAuthorizationManager) {
168+
Assert.notNull(deleteCredentialAuthorizationManager, "deleteCredentialAuthorizationManager cannot be null");
169+
this.deleteCredentialAuthorizationManager = deleteCredentialAuthorizationManager;
170+
}
171+
172+
/**
173+
* Sets the {@link SecurityContextHolderStrategy} to use. The default is
174+
* {@link SecurityContextHolder#getContextHolderStrategy()}.
175+
* @param securityContextHolderStrategy the {@link SecurityContextHolderStrategy} to
176+
* use
177+
* @since 6.5.10
178+
*/
179+
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) {
180+
Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null");
181+
this.securityContextHolderStrategy = securityContextHolderStrategy;
182+
}
183+
135184
@Override
136185
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
137186
throws ServletException, IOException {
@@ -203,7 +252,15 @@ private WebAuthnRegistrationRequest readRegistrationRequest(HttpServletRequest r
203252

204253
private void removeCredential(HttpServletRequest request, HttpServletResponse response, String id)
205254
throws IOException {
206-
this.userCredentials.delete(Bytes.fromBase64(id));
255+
Bytes credentialId = Bytes.fromBase64(id);
256+
Supplier<Authentication> authentication = () -> this.securityContextHolderStrategy.getContext()
257+
.getAuthentication();
258+
AuthorizationResult result = this.deleteCredentialAuthorizationManager.authorize(authentication, credentialId);
259+
if (result != null && !result.isGranted()) {
260+
response.setStatus(HttpStatus.FORBIDDEN.value());
261+
return;
262+
}
263+
this.userCredentials.delete(credentialId);
207264
response.setStatus(HttpStatus.NO_CONTENT.value());
208265
}
209266

0 commit comments

Comments
 (0)