From bb547bf5159ef7ca598791815cd52dec5cfebd13 Mon Sep 17 00:00:00 2001 From: blabadi Date: Mon, 28 Feb 2022 19:10:56 -0500 Subject: [PATCH 1/3] fix OAuth2RequestResolver to only check on request that match the oauth2 login flow. --- docker-compose-all.yml | 2 +- .../ego/security/OAuth2RequestResolver.java | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docker-compose-all.yml b/docker-compose-all.yml index 92f80a67..1b433257 100644 --- a/docker-compose-all.yml +++ b/docker-compose-all.yml @@ -11,7 +11,7 @@ services: REACT_APP_EGO_CLIENT_ID: ego-ui api: # change the image tag to the target image as needed - image: overture/ego:4c1969bf + image: overture/ego:5.2.0 environment: SERVER_PORT: 8081 SPRING_DATASOURCE_URL: jdbc:postgresql://postgres:5432/ego?stringtype=unspecified diff --git a/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java b/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java index be39bb7a..8658a6a8 100644 --- a/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java +++ b/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java @@ -8,6 +8,7 @@ import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver; import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestResolver; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.util.StringUtils; import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.context.request.ServletRequestAttributes; @@ -20,19 +21,27 @@ *

intended to replace {@see OAuth2ClientResources} */ public class OAuth2RequestResolver implements OAuth2AuthorizationRequestResolver { + private final AntPathRequestMatcher authorizationRequestMatcher; private DefaultOAuth2AuthorizationRequestResolver resolver; - + private static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"; public OAuth2RequestResolver( ClientRegistrationRepository clientRegistrationRepository, String authorizationRequestBaseUri) { this.resolver = new DefaultOAuth2AuthorizationRequestResolver( clientRegistrationRepository, authorizationRequestBaseUri); + this.authorizationRequestMatcher = new AntPathRequestMatcher( + authorizationRequestBaseUri + "/{" + REGISTRATION_ID_URI_VARIABLE_NAME + "}"); } @SneakyThrows @Override public OAuth2AuthorizationRequest resolve(HttpServletRequest request) { + // check if the request is an oauth2 login request first + String registrationId = this.resolveRegistrationId(request); + if (registrationId == null) { + return this.resolver.resolve(request); + } val uri = new URI(request.getRequestURI() + "?" + request.getQueryString()); val attr = (ServletRequestAttributes) RequestContextHolder.currentRequestAttributes(); val session = attr.getRequest().getSession(true); @@ -58,4 +67,12 @@ public OAuth2AuthorizationRequest resolve(HttpServletRequest request) { public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String registrationId) { return this.resolve(request, registrationId); } + + private String resolveRegistrationId(HttpServletRequest request) { + if (this.authorizationRequestMatcher.matches(request)) { + return this.authorizationRequestMatcher.matcher(request).getVariables() + .get(REGISTRATION_ID_URI_VARIABLE_NAME); + } + return null; + } } From 6512a8b26387ddc2f0d425aed7a1c2a95d0434f5 Mon Sep 17 00:00:00 2001 From: blabadi Date: Mon, 28 Feb 2022 19:35:52 -0500 Subject: [PATCH 2/3] fix refresh token test cookies null check --- .../java/bio/overture/ego/controller/RefreshTokenTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java b/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java index 9ee827e2..b520fab4 100644 --- a/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java +++ b/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java @@ -172,6 +172,9 @@ public void deleteRefresh_missingRefreshToken_Unauthorized() { private void assertNoRefreshIdCookie(StringResponseOption response) { val cookies = response.getResponse().getHeaders().get("Set-Cookie"); + if (Objects.isNull(cookies)) { + return; + } Objects.requireNonNull(cookies) .forEach( c -> { From ad2ad28683541b6a08cdc7cf9c722eb0400f93f0 Mon Sep 17 00:00:00 2001 From: blabadi Date: Tue, 1 Mar 2022 13:02:09 -0500 Subject: [PATCH 3/3] code styling --- .../ego/security/OAuth2RequestResolver.java | 16 +++++++++++----- .../ego/controller/RefreshTokenTest.java | 3 ++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java b/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java index 8658a6a8..b553fe5c 100644 --- a/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java +++ b/src/main/java/bio/overture/ego/security/OAuth2RequestResolver.java @@ -14,6 +14,8 @@ import org.springframework.web.context.request.ServletRequestAttributes; import org.springframework.web.util.UriComponentsBuilder; +import static java.util.Objects.isNull; + /** * Custom request resolver to capture request info before sending it to oauth2 providers and store * them in the current request session @@ -24,22 +26,24 @@ public class OAuth2RequestResolver implements OAuth2AuthorizationRequestResolver private final AntPathRequestMatcher authorizationRequestMatcher; private DefaultOAuth2AuthorizationRequestResolver resolver; private static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"; + public OAuth2RequestResolver( ClientRegistrationRepository clientRegistrationRepository, String authorizationRequestBaseUri) { this.resolver = new DefaultOAuth2AuthorizationRequestResolver( clientRegistrationRepository, authorizationRequestBaseUri); - this.authorizationRequestMatcher = new AntPathRequestMatcher( - authorizationRequestBaseUri + "/{" + REGISTRATION_ID_URI_VARIABLE_NAME + "}"); + this.authorizationRequestMatcher = + new AntPathRequestMatcher( + authorizationRequestBaseUri + "/{" + REGISTRATION_ID_URI_VARIABLE_NAME + "}"); } @SneakyThrows @Override public OAuth2AuthorizationRequest resolve(HttpServletRequest request) { // check if the request is an oauth2 login request first - String registrationId = this.resolveRegistrationId(request); - if (registrationId == null) { + val registrationId = this.resolveRegistrationId(request); + if (isNull(registrationId)) { return this.resolver.resolve(request); } val uri = new URI(request.getRequestURI() + "?" + request.getQueryString()); @@ -70,7 +74,9 @@ public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String reg private String resolveRegistrationId(HttpServletRequest request) { if (this.authorizationRequestMatcher.matches(request)) { - return this.authorizationRequestMatcher.matcher(request).getVariables() + return this.authorizationRequestMatcher + .matcher(request) + .getVariables() .get(REGISTRATION_ID_URI_VARIABLE_NAME); } return null; diff --git a/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java b/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java index b520fab4..aba8ea5d 100644 --- a/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java +++ b/src/test/java/bio/overture/ego/controller/RefreshTokenTest.java @@ -1,6 +1,7 @@ package bio.overture.ego.controller; import static bio.overture.ego.model.enums.JavaFields.REFRESH_ID; +import static java.util.Objects.isNull; import static org.junit.Assert.*; import static org.springframework.http.HttpHeaders.AUTHORIZATION; import static org.springframework.http.HttpStatus.*; @@ -172,7 +173,7 @@ public void deleteRefresh_missingRefreshToken_Unauthorized() { private void assertNoRefreshIdCookie(StringResponseOption response) { val cookies = response.getResponse().getHeaders().get("Set-Cookie"); - if (Objects.isNull(cookies)) { + if (isNull(cookies)) { return; } Objects.requireNonNull(cookies)