Skip to content

Commit a856baa

Browse files
rwinchrwinch
committed
Add CredentialRecordOwnerAuthorizationManager
Add CredentialRecordOwnerAuthorizationManager that verifies the credential being deleted is owned by the currently authenticated user. Also add an AuthorizationManager<Bytes> to WebAuthnRegistrationFilter for the delete credential operation, defaulting to deny all, and wire it up in WebAuthnConfigurer. Per the WebAuthn specification [1], credential ids contain at least 16 bytes with at least 100 bits of entropy, making them practically unguessable. The specification also advises that credential ids should be kept private, as exposing them can leak personally identifying information [2]. The CredentialRecordOwnerAuthorizationManager serves as defense in depth: even if a credential id were somehow exposed, an unauthorized user could not delete another user's credential. [1] https://www.w3.org/TR/webauthn-3/#credential-id [2] https://www.w3.org/TR/webauthn-3/#sctn-credential-id-privacy-leak
1 parent 95b2cdf commit a856baa

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)