Skip to content

Commit 8c4cfe8

Browse files
rwinchrwinch
authored
Merge pull request #19006 from rwinch/main-CredentialRecordOwnerAuthorizationManager
Merge Add CredentialRecordOwnerAuthorizationManager
2 parents 875b076 + 9d047b6 commit 8c4cfe8

6 files changed

Lines changed: 412 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
@@ -39,6 +39,7 @@
3939
import org.springframework.security.web.webauthn.authentication.PublicKeyCredentialRequestOptionsFilter;
4040
import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationFilter;
4141
import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationProvider;
42+
import org.springframework.security.web.webauthn.management.CredentialRecordOwnerAuthorizationManager;
4243
import org.springframework.security.web.webauthn.management.MapPublicKeyCredentialUserEntityRepository;
4344
import org.springframework.security.web.webauthn.management.MapUserCredentialRepository;
4445
import org.springframework.security.web.webauthn.management.PublicKeyCredentialUserEntityRepository;
@@ -180,6 +181,8 @@ public void configure(H http) {
180181
webAuthnAuthnFilter = postProcess(webAuthnAuthnFilter);
181182
WebAuthnRegistrationFilter webAuthnRegistrationFilter = new WebAuthnRegistrationFilter(userCredentials,
182183
rpOperations);
184+
webAuthnRegistrationFilter.setDeleteCredentialAuthorizationManager(
185+
new CredentialRecordOwnerAuthorizationManager(userCredentials, userEntities));
183186
PublicKeyCredentialCreationOptionsFilter creationOptionsFilter = new PublicKeyCredentialCreationOptionsFilter(
184187
rpOperations);
185188
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
@@ -43,9 +43,16 @@
4343
import org.springframework.security.web.FilterChainProxy;
4444
import org.springframework.security.web.SecurityFilterChain;
4545
import org.springframework.security.web.authentication.ui.DefaultResourcesFilter;
46+
import org.springframework.security.web.webauthn.api.Bytes;
47+
import org.springframework.security.web.webauthn.api.ImmutablePublicKeyCredentialUserEntity;
4648
import org.springframework.security.web.webauthn.api.PublicKeyCredentialCreationOptions;
49+
import org.springframework.security.web.webauthn.api.TestCredentialRecords;
4750
import org.springframework.security.web.webauthn.api.TestPublicKeyCredentialCreationOptions;
4851
import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationFilter;
52+
import org.springframework.security.web.webauthn.management.MapPublicKeyCredentialUserEntityRepository;
53+
import org.springframework.security.web.webauthn.management.MapUserCredentialRepository;
54+
import org.springframework.security.web.webauthn.management.PublicKeyCredentialUserEntityRepository;
55+
import org.springframework.security.web.webauthn.management.UserCredentialRepository;
4956
import org.springframework.security.web.webauthn.management.WebAuthnRelyingPartyOperations;
5057
import org.springframework.security.web.webauthn.registration.HttpSessionPublicKeyCredentialCreationOptionsRepository;
5158
import org.springframework.test.web.servlet.MockMvc;
@@ -58,6 +65,7 @@
5865
import static org.mockito.Mockito.mock;
5966
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication;
6067
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
68+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
6169
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
6270
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
6371
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
@@ -257,6 +265,24 @@ public void webauthnWhenConfiguredMessageConverter() throws Exception {
257265
.andExpect(content().string(expectedBody));
258266
}
259267

268+
@Test
269+
void webauthnWhenDeleteAndCredentialBelongsToUserThenNoContent() throws Exception {
270+
this.spring.register(DeleteCredentialConfiguration.class).autowire();
271+
this.mvc
272+
.perform(delete("/webauthn/register/" + DeleteCredentialConfiguration.CREDENTIAL_ID_BASE64URL)
273+
.with(authentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"))))
274+
.andExpect(status().isNoContent());
275+
}
276+
277+
@Test
278+
void webauthnWhenDeleteAndCredentialBelongsToDifferentUserThenForbidden() throws Exception {
279+
this.spring.register(DeleteCredentialConfiguration.class).autowire();
280+
this.mvc
281+
.perform(delete("/webauthn/register/" + DeleteCredentialConfiguration.CREDENTIAL_ID_BASE64URL)
282+
.with(authentication(new TestingAuthenticationToken("other-user", "password", "ROLE_USER"))))
283+
.andExpect(status().isForbidden());
284+
}
285+
260286
@Configuration
261287
@EnableWebSecurity
262288
static class ConfigCredentialCreationOptionsRepository {
@@ -475,4 +501,47 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
475501

476502
}
477503

504+
@Configuration
505+
@EnableWebSecurity
506+
static class DeleteCredentialConfiguration {
507+
508+
static final String CREDENTIAL_ID_BASE64URL = "NauGCN7bZ5jEBwThcde51g";
509+
510+
static final Bytes USER_ENTITY_ID = Bytes.fromBase64("vKBFhsWT3gQnn-gHdT4VXIvjDkVXVYg5w8CLGHPunMM");
511+
512+
@Bean
513+
UserDetailsService userDetailsService() {
514+
return new InMemoryUserDetailsManager();
515+
}
516+
517+
@Bean
518+
WebAuthnRelyingPartyOperations webAuthnRelyingPartyOperations() {
519+
return mock(WebAuthnRelyingPartyOperations.class);
520+
}
521+
522+
@Bean
523+
UserCredentialRepository userCredentialRepository() {
524+
MapUserCredentialRepository repository = new MapUserCredentialRepository();
525+
repository.save(TestCredentialRecords.userCredential().build());
526+
return repository;
527+
}
528+
529+
@Bean
530+
PublicKeyCredentialUserEntityRepository userEntityRepository() {
531+
MapPublicKeyCredentialUserEntityRepository repository = new MapPublicKeyCredentialUserEntityRepository();
532+
repository.save(ImmutablePublicKeyCredentialUserEntity.builder()
533+
.name("user")
534+
.id(USER_ENTITY_ID)
535+
.displayName("User")
536+
.build());
537+
return repository;
538+
}
539+
540+
@Bean
541+
SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
542+
return http.csrf(AbstractHttpConfigurer::disable).webAuthn(Customizer.withDefaults()).build();
543+
}
544+
545+
}
546+
478547
}

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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.jspecify.annotations.Nullable;
22+
23+
import org.springframework.security.authorization.AuthenticatedAuthorizationManager;
24+
import org.springframework.security.authorization.AuthorizationDecision;
25+
import org.springframework.security.authorization.AuthorizationManager;
26+
import org.springframework.security.authorization.AuthorizationResult;
27+
import org.springframework.security.core.Authentication;
28+
import org.springframework.security.web.webauthn.api.Bytes;
29+
import org.springframework.security.web.webauthn.api.CredentialRecord;
30+
import org.springframework.security.web.webauthn.api.PublicKeyCredentialUserEntity;
31+
import org.springframework.util.Assert;
32+
33+
/**
34+
* An {@link AuthorizationManager} that grants access when the {@link CredentialRecord}
35+
* identified by the provided credential id is owned by the currently authenticated user.
36+
*
37+
* <p>
38+
* Per the <a href="https://www.w3.org/TR/webauthn-3/#credential-id">WebAuthn
39+
* specification</a>, a credential id must contain at least 16 bytes with at least 100
40+
* bits of entropy, making it practically unguessable. The specification also advises that
41+
* credential ids should be kept private, as exposing them can leak personally identifying
42+
* information (see
43+
* <a href="https://www.w3.org/TR/webauthn-3/#sctn-credential-id-privacy-leak">§ 14.6.3
44+
* Privacy leak via credential IDs</a>). This {@link AuthorizationManager} is therefore
45+
* intended as defense in depth: even if a credential id were somehow exposed, an
46+
* unauthorized user could not delete another user's credential.
47+
*
48+
* @author Rob Winch
49+
* @since 6.5.10
50+
*/
51+
public final class CredentialRecordOwnerAuthorizationManager implements AuthorizationManager<Bytes> {
52+
53+
private final AuthenticatedAuthorizationManager<Bytes> authenticatedAuthorizationManager = AuthenticatedAuthorizationManager
54+
.authenticated();
55+
56+
private final UserCredentialRepository userCredentials;
57+
58+
private final PublicKeyCredentialUserEntityRepository userEntities;
59+
60+
/**
61+
* Creates a new instance.
62+
* @param userCredentials the {@link UserCredentialRepository} to use
63+
* @param userEntities the {@link PublicKeyCredentialUserEntityRepository} to use
64+
*/
65+
public CredentialRecordOwnerAuthorizationManager(UserCredentialRepository userCredentials,
66+
PublicKeyCredentialUserEntityRepository userEntities) {
67+
Assert.notNull(userCredentials, "userCredentials cannot be null");
68+
Assert.notNull(userEntities, "userEntities cannot be null");
69+
this.userCredentials = userCredentials;
70+
this.userEntities = userEntities;
71+
}
72+
73+
@Override
74+
public AuthorizationResult authorize(Supplier<? extends @Nullable Authentication> authentication,
75+
Bytes credentialId) {
76+
AuthorizationResult decision = this.authenticatedAuthorizationManager.authorize(authentication, credentialId);
77+
if (!decision.isGranted()) {
78+
return decision;
79+
}
80+
Authentication auth = authentication.get();
81+
CredentialRecord credential = this.userCredentials.findByCredentialId(credentialId);
82+
if (credential == null) {
83+
return new AuthorizationDecision(false);
84+
}
85+
if (credential.getUserEntityUserId() == null) {
86+
return new AuthorizationDecision(false);
87+
}
88+
PublicKeyCredentialUserEntity userEntity = this.userEntities.findByUsername(auth.getName());
89+
if (userEntity == null) {
90+
return new AuthorizationDecision(false);
91+
}
92+
return new AuthorizationDecision(credential.getUserEntityUserId().equals(userEntity.getId()));
93+
}
94+
95+
}

‎webauthn/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 jakarta.servlet.FilterChain;
2223
import jakarta.servlet.ServletException;
@@ -35,6 +36,12 @@
3536
import org.springframework.http.converter.json.JacksonJsonHttpMessageConverter;
3637
import org.springframework.http.server.ServletServerHttpRequest;
3738
import org.springframework.http.server.ServletServerHttpResponse;
39+
import org.springframework.security.authorization.AuthorizationManager;
40+
import org.springframework.security.authorization.AuthorizationResult;
41+
import org.springframework.security.authorization.SingleResultAuthorizationManager;
42+
import org.springframework.security.core.Authentication;
43+
import org.springframework.security.core.context.SecurityContextHolder;
44+
import org.springframework.security.core.context.SecurityContextHolderStrategy;
3845
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
3946
import org.springframework.security.web.util.matcher.RequestMatcher;
4047
import org.springframework.security.web.webauthn.api.Bytes;
@@ -88,6 +95,9 @@ public class WebAuthnRegistrationFilter extends OncePerRequestFilter {
8895

8996
private final UserCredentialRepository userCredentials;
9097

98+
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
99+
.getContextHolderStrategy();
100+
91101
private HttpMessageConverter<Object> converter = new JacksonJsonHttpMessageConverter(
92102
JsonMapper.builder().addModule(new WebauthnJacksonModule()).build());
93103

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

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

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

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

0 commit comments

Comments
 (0)