root
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Remove OAuth2AuthorizationRequest when a distributed session is used 

Dirties the WebSession by putting the amended AUTHORIZATION_REQUEST map into
the WebSession even it was already in the map. This causes common SessionRepository
implementations like Redis to persist the updated attribute.

Fixes gh-7327

Author: Andreas Kluth <mail@andreaskluth.net>
This commit is contained in:
Andreas Kluth 2019-08-30 22:53:03 +02:00 committed by Joe Grandja
parent 26a65249f9
commit c46b224ec4
2 changed files with 44 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/WebSessionOAuth2ServerAuthorizationRequestRepository.java
View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2018 the original author or authors. * Copyright 2002-2019 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -85,6 +85,9 @@ public final class WebSessionOAuth2ServerAuthorizationRequestRepository
OAuth2AuthorizationRequest removedValue = stateToAuthzRequest.remove(state); OAuth2AuthorizationRequest removedValue = stateToAuthzRequest.remove(state);
if (stateToAuthzRequest.isEmpty()) { if (stateToAuthzRequest.isEmpty()) {
sessionAttrs.remove(this.sessionAttributeName); sessionAttrs.remove(this.sessionAttributeName);
} else if (removedValue != null) {
// gh-7327 Overwrite the existing Map to ensure the state is saved for distributed sessions
sessionAttrs.put(this.sessionAttributeName, stateToAuthzRequest);
} }
if (removedValue == null) { if (removedValue == null) {
sink.complete(); sink.complete();

72
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/WebSessionOAuth2ServerAuthorizationRequestRepositoryTests.java
View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2018 the original author or authors. * Copyright 2002-2019 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -63,7 +63,7 @@ public class WebSessionOAuth2ServerAuthorizationRequestRepositoryTests {
.queryParam(OAuth2ParameterNames.STATE, "state")); .queryParam(OAuth2ParameterNames.STATE, "state"));
@Test @Test
public void loadAuthorizatioNRequestWhenNullExchangeThenIllegalArgumentException() { public void loadAuthorizationRequestWhenNullExchangeThenIllegalArgumentException() {
this.exchange = null; this.exchange = null;
assertThatThrownBy(() -> this.repository.loadAuthorizationRequest(this.exchange)) assertThatThrownBy(() -> this.repository.loadAuthorizationRequest(this.exchange))
.isInstanceOf(IllegalArgumentException.class); .isInstanceOf(IllegalArgumentException.class);
@ -106,36 +106,6 @@ public class WebSessionOAuth2ServerAuthorizationRequestRepositoryTests {
.verifyComplete(); .verifyComplete();
} }
@Test
public void multipleSavedAuthorizationRequestAndRedisCookie() {
String oldState = "state0";
MockServerHttpRequest oldRequest = MockServerHttpRequest.get("/")
.queryParam(OAuth2ParameterNames.STATE, oldState).build();
OAuth2AuthorizationRequest oldAuthorizationRequest = OAuth2AuthorizationRequest.authorizationCode()
.authorizationUri("https://example.com/oauth2/authorize")
.clientId("client-id")
.redirectUri("http://localhost/client-1")
.state(oldState)
.build();
Map<String, Object> sessionAttrs = spy(new HashMap<>());
WebSession session = mock(WebSession.class);
when(session.getAttributes()).thenReturn(sessionAttrs);
WebSessionManager sessionManager = e -> Mono.just(session);
this.exchange = new DefaultServerWebExchange(this.exchange.getRequest(), new MockServerHttpResponse(), sessionManager,
ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver());
ServerWebExchange oldExchange = new DefaultServerWebExchange(oldRequest, new MockServerHttpResponse(), sessionManager,
ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver());
Mono<Void> saveAndSave = this.repository.saveAuthorizationRequest(oldAuthorizationRequest, oldExchange)
.then(this.repository.saveAuthorizationRequest(this.authorizationRequest, this.exchange));
StepVerifier.create(saveAndSave).verifyComplete();
verify(sessionAttrs, times(2)).put(any(), any());
}
@Test @Test
public void loadAuthorizationRequestWhenMultipleSavedThenAuthorizationRequest() { public void loadAuthorizationRequestWhenMultipleSavedThenAuthorizationRequest() {
String oldState = "state0"; String oldState = "state0";
@ -269,6 +239,44 @@ public class WebSessionOAuth2ServerAuthorizationRequestRepositoryTests {
.verifyComplete(); .verifyComplete();
} }
// gh-7327
@Test
public void removeAuthorizationRequestWhenMultipleThenRemovedAndSessionAttributeUpdated() {
String oldState = "state0";
MockServerHttpRequest oldRequest = MockServerHttpRequest.get("/")
.queryParam(OAuth2ParameterNames.STATE, oldState).build();
OAuth2AuthorizationRequest oldAuthorizationRequest = OAuth2AuthorizationRequest.authorizationCode()
.authorizationUri("https://example.com/oauth2/authorize")
.clientId("client-id")
.redirectUri("http://localhost/client-1")
.state(oldState)
.build();
Map<String, Object> sessionAttrs = spy(new HashMap<>());
WebSession session = mock(WebSession.class);
when(session.getAttributes()).thenReturn(sessionAttrs);
WebSessionManager sessionManager = e -> Mono.just(session);
this.exchange = new DefaultServerWebExchange(this.exchange.getRequest(), new MockServerHttpResponse(), sessionManager,
ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver());
ServerWebExchange oldExchange = new DefaultServerWebExchange(oldRequest, new MockServerHttpResponse(), sessionManager,
ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver());
Mono<OAuth2AuthorizationRequest> saveAndSaveAndRemove = this.repository.saveAuthorizationRequest(oldAuthorizationRequest, oldExchange)
.then(this.repository.saveAuthorizationRequest(this.authorizationRequest, this.exchange))
.then(this.repository.removeAuthorizationRequest(this.exchange));
StepVerifier.create(saveAndSaveAndRemove)
.expectNext(this.authorizationRequest)
.verifyComplete();
StepVerifier.create(this.repository.loadAuthorizationRequest(this.exchange))
.verifyComplete();
verify(sessionAttrs, times(3)).put(any(), any());
}
private void assertSessionStartedIs(boolean expected) { private void assertSessionStartedIs(boolean expected) {
Mono<Boolean> isStarted = this.exchange.getSession().map(WebSession::isStarted); Mono<Boolean> isStarted = this.exchange.getSession().map(WebSession::isStarted);
StepVerifier.create(isStarted) StepVerifier.create(isStarted)