From 7a55af246ea03dd9908be18bf9dd3bd351ae17f0 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 20 Dec 2018 14:53:52 -0700 Subject: [PATCH] Polish tests and javadoc When using AssertJ, it's easy to commit the following error assertThat(some boolean condition) The above actually does nothing. It at least needs to be assertThat(some boolean condition).isTrue() This commit refines some assertions that were missing a verify condition. Also, one Javadoc was just a little bit confusing, so this clarifies it. Issue: gh-6259 --- .../SessionManagementConfigurerServlet31Tests.java | 4 ++-- .../http/SessionManagementConfigServlet31Tests.java | 6 +++--- .../session/SessionFixationProtectionStrategy.java | 4 ++-- .../ChangeSessionIdAuthenticationStrategyTests.java | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java index c1c7e7f29e..1aae50d519 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java @@ -96,8 +96,8 @@ public class SessionManagementConfigurerServlet31Tests { springSecurityFilterChain.doFilter(request, response, chain); - assertThat(!request.getSession().getId().equals(id)); - assertThat(request.getSession().getAttribute("attribute1").equals("value1")); + assertThat(request.getSession().getId()).isNotEqualTo(id); + assertThat(request.getSession().getAttribute("attribute1")).isEqualTo("value1"); } @EnableWebSecurity diff --git a/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigServlet31Tests.java b/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigServlet31Tests.java index 08c694081c..6f2ee5b803 100644 --- a/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigServlet31Tests.java +++ b/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigServlet31Tests.java @@ -99,8 +99,8 @@ public class SessionManagementConfigServlet31Tests { springSecurityFilterChain.doFilter(request, response, chain); - assertThat(!request.getSession().getId().equals(id)); - assertThat(request.getSession().getAttribute("attribute1").equals("value1")); + assertThat(request.getSession().getId()).isNotEqualTo(id); + assertThat(request.getSession().getAttribute("attribute1")).isEqualTo("value1"); } @Test @@ -123,7 +123,7 @@ public class SessionManagementConfigServlet31Tests { springSecurityFilterChain.doFilter(request, response, chain); - assertThat(!request.getSession().getId().equals(id)); + assertThat(request.getSession().getId()).isNotEqualTo(id); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java index ad76962b6a..ab4cba057b 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java +++ b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java @@ -24,8 +24,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; /** - * The implementation of {@link SessionAuthenticationStrategy} when using < - * Servlet 3.1. + * Uses {@code HttpServletRequest.invalidate()} to protect against session fixation + * attacks. *

* Creates a new session for the newly authenticated user if they already have a session * (as a defence against session-fixation protection attacks), and copies their session diff --git a/web/src/test/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategyTests.java b/web/src/test/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategyTests.java index 2ed85347f1..d34be67e40 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategyTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategyTests.java @@ -15,12 +15,14 @@ */ package org.springframework.security.web.authentication.session; -import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.powermock.modules.junit4.PowerMockRunner; + import org.springframework.mock.web.MockHttpServletRequest; +import static org.assertj.core.api.Assertions.assertThat; + /** * @author Rob Winch * @@ -32,9 +34,7 @@ public class ChangeSessionIdAuthenticationStrategyTests { public void applySessionFixation() { MockHttpServletRequest request = new MockHttpServletRequest(); String id = request.getSession().getId(); - - new ChangeSessionIdAuthenticationStrategy().applySessionFixation(request); - - Assert.assertNotEquals(id, request.getSession().getId()); + new ChangeSessionIdAuthenticationStrategy().applySessionFixation(request); + assertThat(request.getSession().getId()).isNotEqualTo(id); } }