From 7a582c3b6ead96a0e790f74e3fdee2dd46de3617 Mon Sep 17 00:00:00 2001 From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com> Date: Sun, 20 Nov 2022 19:50:48 +0200 Subject: [PATCH] Fix removal of expired cookies Cookies that expired during server downtime failed to be removed from the database. This occurred because the cookie removal method did a lookup to the in-memory active cookies, but the startup method does not load expired cookies to memory. Because the expired cookies were never loaded to memory this did not pose a security vulnerability. Fixed by always deleting a cookie from database if requested. --- .../auth/ActiveCookieExpiryCleanupTask.java | 15 ++++++++++++--- .../webserver/auth/ActiveCookieStore.java | 10 ++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieExpiryCleanupTask.java b/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieExpiryCleanupTask.java index 86d835353..d1e97bac2 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieExpiryCleanupTask.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieExpiryCleanupTask.java @@ -17,6 +17,8 @@ package com.djrapitops.plan.delivery.webserver.auth; import com.djrapitops.plan.TaskSystem; +import com.djrapitops.plan.delivery.formatting.Formatter; +import com.djrapitops.plan.delivery.formatting.Formatters; import com.djrapitops.plan.settings.config.PlanConfig; import com.djrapitops.plan.settings.config.paths.PluginSettings; import dagger.Lazy; @@ -40,13 +42,20 @@ public class ActiveCookieExpiryCleanupTask extends TaskSystem.Task { private final PluginLogger logger; private final Map expiryDates; + private final Formatter dateFormatter; @Inject - public ActiveCookieExpiryCleanupTask(PlanConfig config, Lazy activeCookieStore, PluginLogger logger) { + public ActiveCookieExpiryCleanupTask( + PlanConfig config, + Lazy activeCookieStore, + Formatters formatters, + PluginLogger logger + ) { this.config = config; this.activeCookieStore = activeCookieStore; this.logger = logger; this.expiryDates = new ConcurrentHashMap<>(); + dateFormatter = formatters.secondLong(); } @Override @@ -74,7 +83,7 @@ public class ActiveCookieExpiryCleanupTask extends TaskSystem.Task { activeCookieStore.get().removeCookie(cookie); expiryDates.remove(cookie); if (config.isTrue(PluginSettings.DEV_MODE)) { - logger.info("Cookie " + cookie + " has expired: " + time); + logger.info("Cookie " + cookie + " has expired: " + dateFormatter.apply(time)); } } } @@ -82,7 +91,7 @@ public class ActiveCookieExpiryCleanupTask extends TaskSystem.Task { public void addExpiry(String cookie, Long time) { expiryDates.put(cookie, time); if (config.isTrue(PluginSettings.DEV_MODE)) { - logger.info("Cookie " + cookie + " will expire " + time); + logger.info("Cookie " + cookie + " will expire " + dateFormatter.apply(time)); } } } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieStore.java b/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieStore.java index 781c5c42a..3110b2aa9 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieStore.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/auth/ActiveCookieStore.java @@ -125,12 +125,10 @@ public class ActiveCookieStore implements SubSystem { } public void removeCookie(String cookie) { - Optional foundUser = checkCookie(cookie); - if (foundUser.isPresent()) { - USERS_BY_COOKIE.remove(cookie); - deleteCookieByUser(foundUser.get().getUsername()); - deleteCookie(cookie); - } + checkCookie(cookie).map(User::getUsername) + .ifPresent(this::deleteCookieByUser); + USERS_BY_COOKIE.remove(cookie); + deleteCookie(cookie); } private void deleteCookie(String cookie) {