From 8da71269177ba578085fab8affaa25fd125330f0 Mon Sep 17 00:00:00 2001 From: Giulio Ruggeri Date: Fri, 15 Nov 2019 18:46:27 +0100 Subject: [PATCH 1/2] Fix issue #466 Fixed line separator issue --- .../saml/SAMLAuthenticationProvider.java | 2 +- .../security/saml/SAMLProcessingFilter.java | 17 +++++++++++++++++ .../saml/SAMLAuthenticationProviderTest.java | 5 ++++- .../security/saml/SAMLProcessingFilterTest.java | 4 +++- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java b/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java index d68adc8ba..389e60db5 100644 --- a/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java @@ -121,7 +121,7 @@ public Authentication authenticate(Authentication authentication) throws Authent SAMLCredential authenticationCredential = excludeCredential ? null : credential; ExpiringUsernameAuthenticationToken result = new ExpiringUsernameAuthenticationToken(expiration, principal, authenticationCredential, entitlements); - result.setDetails(userDetails); + result.setDetails(authentication.getDetails()); samlLogger.log(SAMLConstants.AUTH_N_RESPONSE, SAMLConstants.SUCCESS, context, result, null); diff --git a/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java b/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java index f583d057e..4813c31e0 100644 --- a/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java @@ -89,6 +89,10 @@ public Authentication attemptAuthentication(HttpServletRequest request, HttpServ context.setLocalEntityEndpoint(SAMLUtil.getEndpoint(context.getLocalEntityRoleMetadata().getEndpoints(), context.getInboundSAMLBinding(), context.getInboundMessageTransport(), uriComparator)); SAMLAuthenticationToken token = new SAMLAuthenticationToken(context); + + // Allow subclasses to set the "details" property + setDetails(request, token); + return getAuthenticationManager().authenticate(token); } catch (SAMLException e) { @@ -107,6 +111,19 @@ public Authentication attemptAuthentication(HttpServletRequest request, HttpServ } + /** + * Provided so that subclasses may configure what is put into the authentication + * request's details property. + * + * @param request that an authentication request is being created for + * @param authRequest the authentication request object that should have its details + * set + */ + protected void setDetails(HttpServletRequest request, + SAMLAuthenticationToken authRequest) { + authRequest.setDetails(authenticationDetailsSource.buildDetails(request)); + } + /** * Name of the profile this used for authentication. * diff --git a/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java b/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java index 3e1616708..344435e0e 100644 --- a/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java +++ b/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java @@ -120,6 +120,8 @@ public void testAuthenticateUserDetails() throws Exception { provider.setUserDetails(details); SAMLAuthenticationToken token = new SAMLAuthenticationToken(context); + Object authenticationDetails = new Object(); + token.setDetails(authenticationDetails); SAMLCredential result = new SAMLCredential(nameID, assertion, "IDP", "localSP"); expect(consumer.processAuthenticationResponse(context)).andReturn(result); @@ -135,6 +137,7 @@ public void testAuthenticateUserDetails() throws Exception { assertEquals(user, authentication.getPrincipal()); assertEquals(user.getUsername(), authentication.getName()); assertNotNull(authentication.getDetails()); + assertEquals(authenticationDetails, authentication.getDetails()); assertEquals(2, authentication.getAuthorities().size()); assertTrue(authentication.getAuthorities().contains(new SimpleGrantedAuthority("role1"))); assertTrue(authentication.getAuthorities().contains(new SimpleGrantedAuthority("role2"))); @@ -175,4 +178,4 @@ private void verifyMock() { verify(nameID); verify(assertion); } -} \ No newline at end of file +} diff --git a/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java b/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java index 0e94eaedd..5c8fc42a2 100644 --- a/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java +++ b/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java @@ -190,6 +190,8 @@ public void testCorrectPass() throws Exception { final Capture context = new Capture(); expect(request.getRequestURL()).andReturn(new StringBuffer("http://localhost:8081/spring-security-saml2-webapp/saml/SSO")); expect(request.getQueryString()).andReturn(null); + expect(request.getRemoteAddr()).andReturn(null); + expect(request.getSession(false)).andReturn(null); expect(processor.retrieveMessage(capture(context))).andAnswer(new IAnswer() { public SAMLMessageContext answer() throws Throwable { context.getValue().setInboundSAMLBinding(org.opensaml.common.xml.SAMLConstants.SAML2_POST_BINDING_URI); @@ -230,4 +232,4 @@ private void verifyMock() { verify(processor); verify(session); } -} \ No newline at end of file +} From 3f4a4329751e836471877867ee3be4e032d91f37 Mon Sep 17 00:00:00 2001 From: Giulio Ruggeri Date: Fri, 15 Nov 2019 18:46:27 +0100 Subject: [PATCH 2/2] Fix issue #466 Fixed line separator issue --- .../saml/SAMLAuthenticationProvider.java | 2 +- .../security/saml/SAMLProcessingFilter.java | 17 +++++++++++++++++ .../saml/SAMLAuthenticationProviderTest.java | 6 +++++- .../security/saml/SAMLProcessingFilterTest.java | 5 ++++- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java b/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java index b31c94226..6ba2dff3b 100644 --- a/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/saml/SAMLAuthenticationProvider.java @@ -121,7 +121,7 @@ public Authentication authenticate(Authentication authentication) throws Authent SAMLCredential authenticationCredential = excludeCredential ? null : credential; ExpiringUsernameAuthenticationToken result = new ExpiringUsernameAuthenticationToken(expiration, principal, authenticationCredential, entitlements); - result.setDetails(userDetails); + result.setDetails(authentication.getDetails()); samlLogger.log(SAMLConstants.AUTH_N_RESPONSE, SAMLConstants.SUCCESS, context, result, null); diff --git a/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java b/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java index 9c8fd03e8..67441f124 100644 --- a/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/saml/SAMLProcessingFilter.java @@ -89,6 +89,10 @@ public Authentication attemptAuthentication(HttpServletRequest request, HttpServ context.setLocalEntityEndpoint(SAMLUtil.getEndpoint(context.getLocalEntityRoleMetadata().getEndpoints(), context.getInboundSAMLBinding(), context.getInboundMessageTransport(), uriComparator)); SAMLAuthenticationToken token = new SAMLAuthenticationToken(context); + + // Allow subclasses to set the "details" property + setDetails(request, token); + return getAuthenticationManager().authenticate(token); } catch (SAMLException e) { @@ -107,6 +111,19 @@ public Authentication attemptAuthentication(HttpServletRequest request, HttpServ } + /** + * Provided so that subclasses may configure what is put into the authentication + * request's details property. + * + * @param request that an authentication request is being created for + * @param authRequest the authentication request object that should have its details + * set + */ + protected void setDetails(HttpServletRequest request, + SAMLAuthenticationToken authRequest) { + authRequest.setDetails(authenticationDetailsSource.buildDetails(request)); + } + /** * Name of the profile this used for authentication. * diff --git a/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java b/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java index 46c4e0120..275f74fc1 100644 --- a/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java +++ b/core/src/test/java/org/springframework/security/saml/SAMLAuthenticationProviderTest.java @@ -120,6 +120,8 @@ public void testAuthenticateUserDetails() throws Exception { provider.setUserDetails(details); SAMLAuthenticationToken token = new SAMLAuthenticationToken(context); + Object authenticationDetails = new Object(); + token.setDetails(authenticationDetails); SAMLCredential result = new SAMLCredential(nameID, assertion, "IDP", "localSP"); expect(consumer.processAuthenticationResponse(context)).andReturn(result); @@ -135,6 +137,7 @@ public void testAuthenticateUserDetails() throws Exception { assertEquals(user, authentication.getPrincipal()); assertEquals(user.getUsername(), authentication.getName()); assertNotNull(authentication.getDetails()); + assertEquals(authenticationDetails, authentication.getDetails()); assertEquals(2, authentication.getAuthorities().size()); assertTrue(authentication.getAuthorities().contains(new SimpleGrantedAuthority("role1"))); assertTrue(authentication.getAuthorities().contains(new SimpleGrantedAuthority("role2"))); @@ -175,4 +178,5 @@ private void verifyMock() { verify(nameID); verify(assertion); } -} \ No newline at end of file +} + diff --git a/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java b/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java index e71792232..e999f2368 100644 --- a/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java +++ b/core/src/test/java/org/springframework/security/saml/SAMLProcessingFilterTest.java @@ -190,6 +190,8 @@ public void testCorrectPass() throws Exception { final Capture context = new Capture(); expect(request.getRequestURL()).andReturn(new StringBuffer("http://localhost:8081/spring-security-saml2-webapp/saml/SSO")); expect(request.getQueryString()).andReturn(null); + expect(request.getRemoteAddr()).andReturn(null); + expect(request.getSession(false)).andReturn(null); expect(processor.retrieveMessage(capture(context))).andAnswer(new IAnswer() { public SAMLMessageContext answer() throws Throwable { context.getValue().setInboundSAMLBinding(org.opensaml.common.xml.SAMLConstants.SAML2_POST_BINDING_URI); @@ -230,4 +232,5 @@ private void verifyMock() { verify(processor); verify(session); } -} \ No newline at end of file +} +