From af80b6530c21c59877947f839587e005fed39c67 Mon Sep 17 00:00:00 2001 From: Greg Turnquist Date: Wed, 9 Oct 2019 09:05:09 -0500 Subject: [PATCH] Track change of MongoSession's id to properly delete. When a session is made invalid and changed to a new one, the old one must be deleted from MongoDB at the next save(). Resolves #116. --- pom.xml | 39 ++++ .../session/data/mongo/MongoSession.java | 25 ++- .../mongo/ReactiveMongoSessionRepository.java | 10 +- .../MongoDbLogoutVerificationTest.java | 196 ++++++++++++++++++ src/test/resources/logback.xml | 17 ++ 5 files changed, 284 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/springframework/session/data/mongo/integration/MongoDbLogoutVerificationTest.java create mode 100644 src/test/resources/logback.xml diff --git a/pom.xml b/pom.xml index df2c240a..1cd3990c 100644 --- a/pom.xml +++ b/pom.xml @@ -525,6 +525,24 @@ test + + org.springframework + spring-webflux + test + + + + org.springframework.security + spring-security-config + test + + + + org.springframework.security + spring-security-web + test + + de.flapdoodle.embed de.flapdoodle.embed.mongo @@ -575,6 +593,27 @@ test + + org.hamcrest + hamcrest + 2.1 + test + + + + ch.qos.logback + logback-classic + 1.2.3 + test + + + + ch.qos.logback + logback-core + 1.2.3 + test + + org.mockito mockito-core diff --git a/src/main/java/org/springframework/session/data/mongo/MongoSession.java b/src/main/java/org/springframework/session/data/mongo/MongoSession.java index 520c1894..9fbced35 100644 --- a/src/main/java/org/springframework/session/data/mongo/MongoSession.java +++ b/src/main/java/org/springframework/session/data/mongo/MongoSession.java @@ -43,6 +43,7 @@ public class MongoSession implements Session { private static final char DOT_COVER_CHAR = ''; private String id; + private String originalSessionId; private long createdMillis = System.currentTimeMillis(); private long accessedMillis; private long intervalSeconds; @@ -60,6 +61,7 @@ public MongoSession(long maxInactiveIntervalInSeconds) { public MongoSession(String id, long maxInactiveIntervalInSeconds) { this.id = id; + this.originalSessionId = id; this.intervalSeconds = maxInactiveIntervalInSeconds; setLastAccessedTime(Instant.ofEpochMilli(this.createdMillis)); } @@ -72,6 +74,7 @@ static String uncoverDot(String attributeName) { return attributeName.replace(DOT_COVER_CHAR, '.'); } + @Override public String changeSessionId() { String changedId = UUID.randomUUID().toString(); @@ -85,10 +88,12 @@ public T getAttribute(String attributeName) { return (T) this.attrs.get(coverDot(attributeName)); } + @Override public Set getAttributeNames() { return this.attrs.keySet().stream().map(MongoSession::uncoverDot).collect(Collectors.toSet()); } + @Override public void setAttribute(String attributeName, Object attributeValue) { if (attributeValue == null) { @@ -98,10 +103,12 @@ public void setAttribute(String attributeName, Object attributeValue) { } } + @Override public void removeAttribute(String attributeName) { this.attrs.remove(coverDot(attributeName)); } + @Override public Instant getCreationTime() { return Instant.ofEpochMilli(this.createdMillis); } @@ -110,24 +117,29 @@ public void setCreationTime(long created) { this.createdMillis = created; } + @Override public Instant getLastAccessedTime() { return Instant.ofEpochMilli(this.accessedMillis); } + @Override public void setLastAccessedTime(Instant lastAccessedTime) { this.accessedMillis = lastAccessedTime.toEpochMilli(); this.expireAt = Date.from(lastAccessedTime.plus(Duration.ofSeconds(this.intervalSeconds))); } + @Override public Duration getMaxInactiveInterval() { return Duration.ofSeconds(this.intervalSeconds); } + @Override public void setMaxInactiveInterval(Duration interval) { this.intervalSeconds = interval.getSeconds(); } + @Override public boolean isExpired() { return this.intervalSeconds >= 0 && new Date().after(this.expireAt); } @@ -140,14 +152,15 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; MongoSession that = (MongoSession) o; - return Objects.equals(id, that.id); + return Objects.equals(this.id, that.id); } @Override public int hashCode() { - return Objects.hash(id); + return Objects.hash(this.id); } + @Override public String getId() { return this.id; } @@ -159,4 +172,12 @@ public Date getExpireAt() { public void setExpireAt(final Date expireAt) { this.expireAt = expireAt; } + + boolean hasChangedSessionId() { + return !getId().equals(this.originalSessionId); + } + + String getOriginalSessionId() { + return this.originalSessionId; + } } diff --git a/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java b/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java index d87e5449..bf576228 100644 --- a/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java +++ b/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java @@ -15,6 +15,8 @@ */ package org.springframework.session.data.mongo; +import static org.springframework.data.mongodb.core.query.Criteria.*; +import static org.springframework.data.mongodb.core.query.Query.*; import static org.springframework.session.data.mongo.MongoSessionUtils.*; import java.time.Duration; @@ -94,7 +96,13 @@ public Mono save(MongoSession session) { DBObject dbObject = convertToDBObject(this.mongoSessionConverter, session); if (dbObject != null) { - return this.mongoOperations.save(dbObject, this.collectionName).then(); + if (session.hasChangedSessionId()) { + return this.mongoOperations.findAndRemove(query(where("_id").is(session.getOriginalSessionId())), MongoSession.class, this.collectionName) + .then(this.mongoOperations.save(dbObject, this.collectionName)) + .then(); + } else { + return this.mongoOperations.save(dbObject, this.collectionName).then(); + } } else { return Mono.empty(); } diff --git a/src/test/java/org/springframework/session/data/mongo/integration/MongoDbLogoutVerificationTest.java b/src/test/java/org/springframework/session/data/mongo/integration/MongoDbLogoutVerificationTest.java new file mode 100644 index 00000000..0c7b4300 --- /dev/null +++ b/src/test/java/org/springframework/session/data/mongo/integration/MongoDbLogoutVerificationTest.java @@ -0,0 +1,196 @@ +/* + * Copyright 2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.session.data.mongo.integration; + +import static org.assertj.core.api.AssertionsForClassTypes.*; + +import java.io.IOException; +import java.net.URI; + +import de.flapdoodle.embed.mongo.MongodExecutable; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import reactor.test.StepVerifier; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.mongodb.core.ReactiveMongoOperations; +import org.springframework.data.mongodb.core.ReactiveMongoTemplate; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurity; +import org.springframework.security.config.web.server.ServerHttpSecurity; +import org.springframework.security.core.userdetails.MapReactiveUserDetailsService; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.web.server.SecurityWebFilterChain; +import org.springframework.session.data.mongo.config.annotation.web.reactive.EnableMongoWebSession; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.web.reactive.server.FluxExchangeResult; +import org.springframework.test.web.reactive.server.WebTestClient; +import org.springframework.util.SocketUtils; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.config.EnableWebFlux; +import org.springframework.web.reactive.function.BodyInserters; + +import com.mongodb.reactivestreams.client.MongoClient; +import com.mongodb.reactivestreams.client.MongoClients; + +/** + * @author Greg Turnquist + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration +public class MongoDbLogoutVerificationTest { + + @Autowired ApplicationContext ctx; + + WebTestClient client; + + @BeforeEach + void setUp() { + this.client = WebTestClient.bindToApplicationContext(this.ctx).build(); + } + + @Test + void logoutShouldDeleteOldSessionIdFromMongoDB() { + + // 1. `curl -i -v -X POST --data "username=admin&password=password" localhost:8080/login` - Save SESSION cookie and + // use it it nex step as {cookie-value-1} + + FluxExchangeResult loginResult = this.client.post().uri("/login") + .contentType(MediaType.APPLICATION_FORM_URLENCODED) // + .body(BodyInserters // + .fromFormData("username", "admin") // + .with("password", "password")) // + .exchange() // + .returnResult(String.class); + + assertThat(loginResult.getResponseHeaders().getLocation()).isEqualTo(URI.create("/")); + + String originalSessionId = loginResult.getResponseCookies().getFirst("SESSION").getValue(); + + // 2. `curl -i -L -v -X GET --cookie "SESSION=48eb6ab2-2c08-43b7-a303-46099bfef231" localhost:8080/hello` - response + // status will be 200, body will be "HelloWorld" + + this.client.get().uri("/hello") // + .cookie("SESSION", originalSessionId) // + .exchange() // + .expectStatus().isOk() // + .returnResult(String.class).getResponseBody() // + .as(StepVerifier::create) // + .expectNext("HelloWorld") // + .verifyComplete(); + + // 3. `curl -i -L -v -X POST --cookie "SESSION=48eb6ab2-2c08-43b7-a303-46099bfef231" localhost:8080/logout` - Save + // SESSION cookie and use it it nex step as {cookie-value-2} + + String newSessionId = this.client.post().uri("/logout") // + .cookie("SESSION", originalSessionId) // + .exchange() // + .expectStatus().isFound() // + .returnResult(String.class) + .getResponseCookies().getFirst("SESSION").getValue(); + + assertThat(newSessionId).isNotEqualTo(originalSessionId); + + // 4. `curl -i -L -v -X GET --cookie "SESSION=3b20200c-cf5e-4529-b3af-3c37ed365f5a" localhost:8080/hello` - response + // status will be 302, body will be empty + + this.client.get().uri("/hello") // + .cookie("SESSION", newSessionId) // + .exchange() // + .expectStatus().isFound() // + .expectHeader().value(HttpHeaders.LOCATION, value -> assertThat(value).isEqualTo("/login")); + + // 5. `curl -i -L -v -X GET --cookie "SESSION=48eb6ab2-2c08-43b7-a303-46099bfef231" localhost:8080/hello` - response + // status will be 200, body will be "HelloWorld", but it should be the same as step 4 + + this.client.get().uri("/hello") // + .cookie("SESSION", originalSessionId) // + .exchange() // + .expectStatus().isFound() // + .expectHeader().value(HttpHeaders.LOCATION, value -> assertThat(value).isEqualTo("/login")); + } + + @RestController + static class TestController { + + @GetMapping("/hello") + public ResponseEntity hello() { + return ResponseEntity.ok("HelloWorld"); + } + + } + + @EnableWebFluxSecurity + static class SecurityConfig { + + @Bean + public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) { + + return http // + .logout()// + /**/.and() // + .formLogin() // + /**/.and() // + .csrf().disable() // + .authorizeExchange() // + .anyExchange().authenticated() // + /**/.and() // + .build(); + } + + @Bean + public MapReactiveUserDetailsService userDetailsService() { + + return new MapReactiveUserDetailsService(User.withDefaultPasswordEncoder() // + .username("admin") // + .password("password") // + .roles("USER,ADMIN") // + .build()); + } + } + + @Configuration + @EnableWebFlux + @EnableMongoWebSession + static class Config { + + private int embeddedMongoPort = SocketUtils.findAvailableTcpPort(); + + @Bean(initMethod = "start", destroyMethod = "stop") + public MongodExecutable embeddedMongoServer() throws IOException { + return MongoITestUtils.embeddedMongoServer(this.embeddedMongoPort); + } + + @Bean + public ReactiveMongoOperations mongoOperations(MongodExecutable embeddedMongoServer) { + + MongoClient mongo = MongoClients.create("mongodb://localhost:" + this.embeddedMongoPort); + return new ReactiveMongoTemplate(mongo, "test"); + } + + @Bean + TestController controller() { + return new TestController(); + } + } +} diff --git a/src/test/resources/logback.xml b/src/test/resources/logback.xml new file mode 100644 index 00000000..3c33cc09 --- /dev/null +++ b/src/test/resources/logback.xml @@ -0,0 +1,17 @@ + + + + %d{HH:mm:ss.SSS} [%8.-8thread] %-5level %logger{36} - %msg%n + + + + + + + + + + + + + \ No newline at end of file