From 36cbdfe0135ecbd6480c1163f9b456b2f7976456 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 23 Jul 2018 12:28:48 -0400 Subject: [PATCH] Fix NPE when null Authentication in authorization_code grant Fixes gh-5560 --- .../OAuth2AuthorizationCodeGrantFilter.java | 3 +- ...uth2AuthorizationCodeGrantFilterTests.java | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilter.java index 90497366de..841e02a391 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilter.java @@ -192,10 +192,11 @@ public class OAuth2AuthorizationCodeGrantFilter extends OncePerRequestFilter { } Authentication currentAuthentication = SecurityContextHolder.getContext().getAuthentication(); + String principalName = currentAuthentication != null ? currentAuthentication.getName() : "anonymousUser"; OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient( authenticationResult.getClientRegistration(), - currentAuthentication.getName(), + principalName, authenticationResult.getAccessToken(), authenticationResult.getRefreshToken()); diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilterTests.java index e9b615e98f..fa834f919e 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilterTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationCodeGrantFilterTests.java @@ -338,6 +338,44 @@ public class OAuth2AuthorizationCodeGrantFilterTests { assertThat(authorizedClients.values().iterator().next()).isSameAs(authorizedClient); } + @Test + public void doFilterWhenAuthorizationResponseSuccessAndAnonymousAccessNullAuthenticationThenAuthorizedClientSavedToHttpSession() throws Exception { + SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); + SecurityContextHolder.setContext(securityContext); // null Authentication + + String requestUri = "/callback/client-1"; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + request.addParameter(OAuth2ParameterNames.CODE, "code"); + request.addParameter(OAuth2ParameterNames.STATE, "state"); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.setUpAuthorizationRequest(request, response, this.registration1); + this.setUpAuthenticationResult(this.registration1); + + this.filter.doFilter(request, response, filterChain); + + OAuth2AuthorizedClient authorizedClient = this.authorizedClientRepository.loadAuthorizedClient( + this.registration1.getRegistrationId(), null, request); + assertThat(authorizedClient).isNotNull(); + + assertThat(authorizedClient.getClientRegistration()).isEqualTo(this.registration1); + assertThat(authorizedClient.getPrincipalName()).isEqualTo("anonymousUser"); + assertThat(authorizedClient.getAccessToken()).isNotNull(); + + HttpSession session = request.getSession(false); + assertThat(session).isNotNull(); + + @SuppressWarnings("unchecked") + Map authorizedClients = (Map) + session.getAttribute(HttpSessionOAuth2AuthorizedClientRepository.class.getName() + ".AUTHORIZED_CLIENTS"); + assertThat(authorizedClients).isNotEmpty(); + assertThat(authorizedClients).hasSize(1); + assertThat(authorizedClients.values().iterator().next()).isSameAs(authorizedClient); + } + private void setUpAuthorizationRequest(HttpServletRequest request, HttpServletResponse response, ClientRegistration registration) { Map additionalParameters = new HashMap<>();