From 8c7e9ded9ad3f8f9aca79558320f319d229c547c Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 15 Jun 2020 18:19:02 +0200 Subject: [PATCH] SONAR-13327 Fix SSF-107 * SONAR-13327 Create 'SAML_MESSAGE_IDS' table and DAO * SONAR-13327 Check SAML Message id not already exist during auth * SONAR-13327 Clean expired SAML Message ids daily --- server/sonar-auth-saml/build.gradle | 2 + .../sonar/auth/saml/SamlIdentityProvider.java | 5 +- .../sonar/auth/saml/SamlMessageIdChecker.java | 59 +++++++++++ .../java/org/sonar/auth/saml/SamlModule.java | 1 + .../auth/saml/SamlIdentityProviderTest.java | 51 ++++++---- .../auth/saml/SamlMessageIdCheckerTest.java | 90 +++++++++++++++++ .../org/sonar/auth/saml/SamlModuleTest.java | 2 +- .../java/org/sonar/db/version/SqTables.java | 1 + .../src/main/java/org/sonar/db/DaoModule.java | 2 + .../src/main/java/org/sonar/db/DbClient.java | 7 ++ .../src/main/java/org/sonar/db/MyBatis.java | 2 + .../org/sonar/db/user/SamlMessageIdDao.java | 57 +++++++++++ .../org/sonar/db/user/SamlMessageIdDto.java | 75 ++++++++++++++ .../sonar/db/user/SamlMessageIdMapper.java | 34 +++++++ .../org/sonar/db/user/SamlMessageIdMapper.xml | 40 ++++++++ server/sonar-db-dao/src/schema/schema-sq.ddl | 9 ++ .../sonar/db/user/SamlMessageIdDaoTest.java | 98 +++++++++++++++++++ .../v84/CreateSamlMessageIdsTable.java | 71 ++++++++++++++ .../db/migration/version/v84/DbVersion84.java | 1 + .../v84/CreateSamlMessageIdsTableTest.java | 58 +++++++++++ .../authentication/AuthenticationModule.java | 8 +- ...eaner.java => ExpiredSessionsCleaner.java} | 36 ++++--- ...xpiredSessionsCleanerExecutorService.java} | 2 +- ...edSessionsCleanerExecutorServiceImpl.java} | 8 +- ...t.java => ExpiredSessionsCleanerTest.java} | 33 +++++-- 25 files changed, 697 insertions(+), 55 deletions(-) create mode 100644 server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlMessageIdChecker.java create mode 100644 server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlMessageIdCheckerTest.java create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDao.java create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDto.java create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdMapper.java create mode 100644 server/sonar-db-dao/src/main/resources/org/sonar/db/user/SamlMessageIdMapper.xml create mode 100644 server/sonar-db-dao/src/test/java/org/sonar/db/user/SamlMessageIdDaoTest.java create mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTable.java create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTableTest.java rename server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/{SessionTokensCleaner.java => ExpiredSessionsCleaner.java} (61%) rename server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/{SessionTokensCleanerExecutorService.java => ExpiredSessionsCleanerExecutorService.java} (91%) rename server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/{SessionTokensCleanerExecutorServiceImpl.java => ExpiredSessionsCleanerExecutorServiceImpl.java} (86%) rename server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/{SessionTokensCleanerTest.java => ExpiredSessionsCleanerTest.java} (76%) diff --git a/server/sonar-auth-saml/build.gradle b/server/sonar-auth-saml/build.gradle index 5651516b7b6..dd9d2341cb1 100644 --- a/server/sonar-auth-saml/build.gradle +++ b/server/sonar-auth-saml/build.gradle @@ -17,10 +17,12 @@ dependencies { compileOnly 'com.google.code.findbugs:jsr305' compileOnly 'com.squareup.okhttp3:okhttp' compileOnly 'javax.servlet:javax.servlet-api' + compileOnly project(':server:sonar-db-dao') compileOnly project(':sonar-core') testCompile 'com.tngtech.java:junit-dataprovider' testCompile 'junit:junit' testCompile 'org.assertj:assertj-core' testCompile 'org.mockito:mockito-core' + testCompile testFixtures(project(':server:sonar-db-dao')) } diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java index 692e0250669..f8033bbfc56 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java @@ -56,9 +56,11 @@ public class SamlIdentityProvider implements OAuth2IdentityProvider { private static final String STATE_REQUEST_PARAMETER = "RelayState"; private final SamlSettings samlSettings; + private final SamlMessageIdChecker samlMessageIdChecker; - public SamlIdentityProvider(SamlSettings samlSettings) { + public SamlIdentityProvider(SamlSettings samlSettings, SamlMessageIdChecker samlMessageIdChecker) { this.samlSettings = samlSettings; + this.samlMessageIdChecker = samlMessageIdChecker; } @Override @@ -107,6 +109,7 @@ public class SamlIdentityProvider implements OAuth2IdentityProvider { LOGGER.trace("Name ID : {}", auth.getNameId()); checkAuthentication(auth); + samlMessageIdChecker.check(auth); LOGGER.trace("Attributes received : {}", auth.getAttributes()); String login = getNonNullFirstAttribute(auth, samlSettings.getUserLogin()); diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlMessageIdChecker.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlMessageIdChecker.java new file mode 100644 index 00000000000..f913b3d6f37 --- /dev/null +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlMessageIdChecker.java @@ -0,0 +1,59 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.auth.saml; + +import com.onelogin.saml2.Auth; +import org.joda.time.Instant; +import org.sonar.api.server.ServerSide; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.user.SamlMessageIdDto; + +import static java.util.Objects.requireNonNull; + +@ServerSide +public class SamlMessageIdChecker { + + private final DbClient dbClient; + + public SamlMessageIdChecker(DbClient dbClient) { + this.dbClient = dbClient; + } + + public void check(Auth auth) { + String messageId = requireNonNull(auth.getLastMessageId(), "Message ID is missing"); + Instant lastAssertionNotOnOrAfter = auth.getLastAssertionNotOnOrAfter().stream() + .sorted() + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("Missing NotOnOrAfter element")); + try (DbSession dbSession = dbClient.openSession(false)) { + dbClient.samlMessageIdDao().selectByMessageId(dbSession, messageId) + .ifPresent(m -> { + throw new IllegalArgumentException("This message has already been processed"); + }); + dbClient.samlMessageIdDao().insert(dbSession, new SamlMessageIdDto() + .setMessageId(messageId) + .setExpirationDate(lastAssertionNotOnOrAfter.getMillis())); + dbSession.commit(); + } + } + +} diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlModule.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlModule.java index 63f9732870d..aecf19200ea 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlModule.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlModule.java @@ -29,6 +29,7 @@ public class SamlModule extends Module { protected void configureModule() { add( SamlIdentityProvider.class, + SamlMessageIdChecker.class, SamlSettings.class); List definitions = SamlSettings.definitions(); add(definitions.toArray(new Object[definitions.size()])); diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java index 58f3e87a0ab..3bd7cd2b85c 100644 --- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java @@ -30,15 +30,16 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.server.authentication.UserIdentity; import org.sonar.api.utils.System2; +import org.sonar.db.DbTester; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -47,11 +48,13 @@ import static org.mockito.Mockito.when; public class SamlIdentityProviderTest { @Rule - public ExpectedException expectedException = ExpectedException.none(); + public DbTester db = DbTester.create(); private MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE, SamlSettings.definitions())); - private SamlIdentityProvider underTest = new SamlIdentityProvider(new SamlSettings(settings.asConfig())); + private SamlMessageIdChecker samlMessageIdChecker = mock(SamlMessageIdChecker.class); + + private SamlIdentityProvider underTest = new SamlIdentityProvider(new SamlSettings(settings.asConfig()), new SamlMessageIdChecker(db.getDbClient())); @Test public void check_fields() { @@ -98,10 +101,9 @@ public class SamlIdentityProviderTest { settings.setProperty("sonar.auth.saml.loginUrl", "invalid"); DumbInitContext context = new DumbInitContext(); - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("Fail to create Auth"); - - underTest.init(context); + assertThatThrownBy(() -> underTest.init(context)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Fail to create Auth"); } @Test @@ -159,10 +161,10 @@ public class SamlIdentityProviderTest { setSettings(true); DumbCallbackContext callbackContext = new DumbCallbackContext("encoded_response_without_login.txt"); - expectedException.expect(NullPointerException.class); - expectedException.expectMessage("login is missing"); + assertThatThrownBy(() -> underTest.callback(callbackContext)) + .isInstanceOf(NullPointerException.class) + .hasMessage("login is missing"); - underTest.callback(callbackContext); } @Test @@ -170,10 +172,9 @@ public class SamlIdentityProviderTest { setSettings(true); DumbCallbackContext callbackContext = new DumbCallbackContext("encoded_response_without_name.txt"); - expectedException.expect(NullPointerException.class); - expectedException.expectMessage("name is missing"); - - underTest.callback(callbackContext); + assertThatThrownBy(() -> underTest.callback(callbackContext)) + .isInstanceOf(NullPointerException.class) + .hasMessage("name is missing"); } @Test @@ -182,10 +183,9 @@ public class SamlIdentityProviderTest { settings.setProperty("sonar.auth.saml.certificate.secured", "invalid"); DumbCallbackContext callbackContext = new DumbCallbackContext("encoded_full_response.txt"); - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("Fail to create Auth"); - - underTest.callback(callbackContext); + assertThatThrownBy(() -> underTest.callback(callbackContext)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Fail to create Auth"); } @Test @@ -218,10 +218,21 @@ public class SamlIdentityProviderTest { "-----END CERTIFICATE-----\n"); DumbCallbackContext callbackContext = new DumbCallbackContext("encoded_full_response.txt"); - expectedException.expect(UnauthorizedException.class); - expectedException.expectMessage("Signature validation failed. SAML Response rejected"); + assertThatThrownBy(() -> underTest.callback(callbackContext)) + .isInstanceOf(UnauthorizedException.class) + .hasMessage("Signature validation failed. SAML Response rejected"); + } + + @Test + public void fail_callback_when_message_was_already_sent() { + setSettings(true); + DumbCallbackContext callbackContext = new DumbCallbackContext("encoded_minimal_response.txt"); underTest.callback(callbackContext); + + assertThatThrownBy(() -> underTest.callback(callbackContext)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("This message has already been processed"); } private void setSettings(boolean enabled) { diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlMessageIdCheckerTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlMessageIdCheckerTest.java new file mode 100644 index 00000000000..f5657459c0f --- /dev/null +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlMessageIdCheckerTest.java @@ -0,0 +1,90 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.auth.saml; + +import com.google.common.collect.ImmutableList; +import com.onelogin.saml2.Auth; +import java.util.Arrays; +import org.joda.time.Instant; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; +import org.sonar.db.user.SamlMessageIdDto; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SamlMessageIdCheckerTest { + + @Rule + public DbTester db = DbTester.create(); + + private DbSession dbSession = db.getSession(); + + private Auth auth = mock(Auth.class); + + private SamlMessageIdChecker underTest = new SamlMessageIdChecker(db.getDbClient()); + + @Test + public void check_do_not_fail_when_message_id_is_new_and_insert_saml_message_in_db() { + db.getDbClient().samlMessageIdDao().insert(dbSession, new SamlMessageIdDto().setMessageId("MESSAGE_1").setExpirationDate(1_000_000_000L)); + db.commit(); + when(auth.getLastMessageId()).thenReturn("MESSAGE_2"); + when(auth.getLastAssertionNotOnOrAfter()).thenReturn(ImmutableList.of(Instant.ofEpochMilli(10_000_000_000L))); + + assertThatCode(() -> underTest.check(auth)).doesNotThrowAnyException(); + + SamlMessageIdDto result = db.getDbClient().samlMessageIdDao().selectByMessageId(dbSession, "MESSAGE_2").get(); + assertThat(result.getMessageId()).isEqualTo("MESSAGE_2"); + assertThat(result.getExpirationDate()).isEqualTo(10_000_000_000L); + } + + @Test + public void check_fails_when_message_id_already_exist() { + db.getDbClient().samlMessageIdDao().insert(dbSession, new SamlMessageIdDto().setMessageId("MESSAGE_1").setExpirationDate(1_000_000_000L)); + db.commit(); + when(auth.getLastMessageId()).thenReturn("MESSAGE_1"); + when(auth.getLastAssertionNotOnOrAfter()).thenReturn(ImmutableList.of(Instant.ofEpochMilli(10_000_000_000L))); + + assertThatThrownBy(() -> underTest.check(auth)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("This message has already been processed"); + } + + @Test + public void check_insert_message_id_using_oldest_NotOnOrAfter_value() { + db.getDbClient().samlMessageIdDao().insert(dbSession, new SamlMessageIdDto().setMessageId("MESSAGE_1").setExpirationDate(1_000_000_000L)); + db.commit(); + when(auth.getLastMessageId()).thenReturn("MESSAGE_2"); + when(auth.getLastAssertionNotOnOrAfter()) + .thenReturn(Arrays.asList(Instant.ofEpochMilli(10_000_000_000L), Instant.ofEpochMilli(30_000_000_000L), Instant.ofEpochMilli(20_000_000_000L))); + + assertThatCode(() -> underTest.check(auth)).doesNotThrowAnyException(); + + SamlMessageIdDto result = db.getDbClient().samlMessageIdDao().selectByMessageId(dbSession, "MESSAGE_2").get(); + assertThat(result.getMessageId()).isEqualTo("MESSAGE_2"); + assertThat(result.getExpirationDate()).isEqualTo(10_000_000_000L); + } +} diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlModuleTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlModuleTest.java index 2add1d96f88..01aeba26b72 100644 --- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlModuleTest.java +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlModuleTest.java @@ -31,6 +31,6 @@ public class SamlModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new SamlModule().configure(container); - assertThat(container.size()).isEqualTo(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 12); + assertThat(container.size()).isGreaterThan(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER); } } diff --git a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java index 5c899412dca..b99c49f6536 100644 --- a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java +++ b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java @@ -106,6 +106,7 @@ public final class SqTables { "qprofile_edit_users", "quality_gates", "quality_gate_conditions", + "saml_message_ids", "rules", "rules_metadata", "rules_parameters", diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/DaoModule.java b/server/sonar-db-dao/src/main/java/org/sonar/db/DaoModule.java index bba7ecd2731..aa3ffa7667d 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/DaoModule.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/DaoModule.java @@ -84,6 +84,7 @@ import org.sonar.db.source.FileSourceDao; import org.sonar.db.user.GroupDao; import org.sonar.db.user.GroupMembershipDao; import org.sonar.db.user.RoleDao; +import org.sonar.db.user.SamlMessageIdDao; import org.sonar.db.user.SessionTokensDao; import org.sonar.db.user.UserDao; import org.sonar.db.user.UserGroupDao; @@ -155,6 +156,7 @@ public class DaoModule extends Module { RoleDao.class, RuleDao.class, RuleRepositoryDao.class, + SamlMessageIdDao.class, SnapshotDao.class, SchemaMigrationDao.class, SessionTokensDao.class, diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/DbClient.java b/server/sonar-db-dao/src/main/java/org/sonar/db/DbClient.java index 2d3f79aad57..24718f69174 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/DbClient.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/DbClient.java @@ -82,6 +82,7 @@ import org.sonar.db.source.FileSourceDao; import org.sonar.db.user.GroupDao; import org.sonar.db.user.GroupMembershipDao; import org.sonar.db.user.RoleDao; +import org.sonar.db.user.SamlMessageIdDao; import org.sonar.db.user.SessionTokensDao; import org.sonar.db.user.UserDao; import org.sonar.db.user.UserGroupDao; @@ -164,6 +165,7 @@ public class DbClient { private final NewCodePeriodDao newCodePeriodDao; private final ProjectDao projectDao; private final SessionTokensDao sessionTokensDao; + private final SamlMessageIdDao samlMessageIdDao; public DbClient(Database database, MyBatis myBatis, DBSessions dbSessions, Dao... daos) { this.database = database; @@ -242,6 +244,7 @@ public class DbClient { newCodePeriodDao = getDao(map, NewCodePeriodDao.class); projectDao = getDao(map, ProjectDao.class); sessionTokensDao = getDao(map, SessionTokensDao.class); + samlMessageIdDao = getDao(map, SamlMessageIdDao.class); } public DbSession openSession(boolean batch) { @@ -534,4 +537,8 @@ public class DbClient { return sessionTokensDao; } + public SamlMessageIdDao samlMessageIdDao() { + return samlMessageIdDao; + } + } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java b/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java index ad5d5310b2f..8d31e0f9735 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java @@ -140,6 +140,7 @@ import org.sonar.db.user.GroupMapper; import org.sonar.db.user.GroupMembershipDto; import org.sonar.db.user.GroupMembershipMapper; import org.sonar.db.user.RoleMapper; +import org.sonar.db.user.SamlMessageIdMapper; import org.sonar.db.user.SessionTokenMapper; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserGroupDto; @@ -286,6 +287,7 @@ public class MyBatis implements Startable { RoleMapper.class, RuleMapper.class, RuleRepositoryMapper.class, + SamlMessageIdMapper.class, SchemaMigrationMapper.class, SessionTokenMapper.class, SnapshotMapper.class, diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDao.java new file mode 100644 index 00000000000..3cb6729c871 --- /dev/null +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDao.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.user; + +import java.util.Optional; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.Dao; +import org.sonar.db.DbSession; + +public class SamlMessageIdDao implements Dao { + + private final System2 system2; + private final UuidFactory uuidFactory; + + public SamlMessageIdDao(System2 system2, UuidFactory uuidFactory) { + this.system2 = system2; + this.uuidFactory = uuidFactory; + } + + public Optional selectByMessageId(DbSession session, String messageId) { + return Optional.ofNullable(mapper(session).selectByMessageId(messageId)); + } + + public SamlMessageIdDto insert(DbSession session, SamlMessageIdDto dto) { + long now = system2.now(); + mapper(session).insert(dto + .setUuid(uuidFactory.create()) + .setCreatedAt(now)); + return dto; + } + + public int deleteExpired(DbSession dbSession) { + return mapper(dbSession).deleteExpired(system2.now()); + } + + private static SamlMessageIdMapper mapper(DbSession session) { + return session.getMapper(SamlMessageIdMapper.class); + } +} diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDto.java new file mode 100644 index 00000000000..89b00ab4a86 --- /dev/null +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdDto.java @@ -0,0 +1,75 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.user; + +public class SamlMessageIdDto { + + private String uuid; + + /** + * Message ID from the SAML response received during authentication. + */ + private String messageId; + + /** + * Expiration date is coming from the NotOnOrAfter attribute of the SAML response. + * + * A row that contained an expired date can be safely deleted from database. + */ + private long expirationDate; + + private long createdAt; + + public String getUuid() { + return uuid; + } + + SamlMessageIdDto setUuid(String uuid) { + this.uuid = uuid; + return this; + } + + public String getMessageId() { + return messageId; + } + + public SamlMessageIdDto setMessageId(String messageId) { + this.messageId = messageId; + return this; + } + + public long getExpirationDate() { + return expirationDate; + } + + public SamlMessageIdDto setExpirationDate(long expirationDate) { + this.expirationDate = expirationDate; + return this; + } + + public long getCreatedAt() { + return createdAt; + } + + SamlMessageIdDto setCreatedAt(long createdAt) { + this.createdAt = createdAt; + return this; + } +} diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdMapper.java new file mode 100644 index 00000000000..0cd1a0a0186 --- /dev/null +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/SamlMessageIdMapper.java @@ -0,0 +1,34 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.db.user; + +import javax.annotation.CheckForNull; +import org.apache.ibatis.annotations.Param; + +public interface SamlMessageIdMapper { + + @CheckForNull + SamlMessageIdDto selectByMessageId(String messageId); + + void insert(@Param("dto") SamlMessageIdDto dto); + + int deleteExpired(@Param("now") long now); + +} diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/SamlMessageIdMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/SamlMessageIdMapper.xml new file mode 100644 index 00000000000..5e090f3bd1b --- /dev/null +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/SamlMessageIdMapper.xml @@ -0,0 +1,40 @@ + + + + + + + smi.uuid as uuid, + smi.message_id as "messageId", + smi.expiration_date as "expirationDate", + smi.created_at as "createdAt" + + + + + + insert into saml_message_ids + ( + uuid, + message_id, + expiration_date, + created_at + ) + values ( + #{dto.uuid, jdbcType=VARCHAR}, + #{dto.messageId, jdbcType=VARCHAR}, + #{dto.expirationDate, jdbcType=BIGINT}, + #{dto.createdAt, jdbcType=BIGINT} + ) + + + + delete from saml_message_ids where expiration_date < #{now, jdbcType=BIGINT} + + + diff --git a/server/sonar-db-dao/src/schema/schema-sq.ddl b/server/sonar-db-dao/src/schema/schema-sq.ddl index 1bc82047b1e..106647743ed 100644 --- a/server/sonar-db-dao/src/schema/schema-sq.ddl +++ b/server/sonar-db-dao/src/schema/schema-sq.ddl @@ -871,6 +871,15 @@ CREATE TABLE "RULES_PROFILES"( ); ALTER TABLE "RULES_PROFILES" ADD CONSTRAINT "PK_RULES_PROFILES" PRIMARY KEY("UUID"); +CREATE TABLE "SAML_MESSAGE_IDS"( + "UUID" VARCHAR(40) NOT NULL, + "MESSAGE_ID" VARCHAR(255) NOT NULL, + "EXPIRATION_DATE" BIGINT NOT NULL, + "CREATED_AT" BIGINT NOT NULL +); +ALTER TABLE "SAML_MESSAGE_IDS" ADD CONSTRAINT "PK_SAML_MESSAGE_IDS" PRIMARY KEY("UUID"); +CREATE UNIQUE INDEX "SAML_MESSAGE_IDS_UNIQUE" ON "SAML_MESSAGE_IDS"("MESSAGE_ID"); + CREATE TABLE "SESSION_TOKENS"( "UUID" VARCHAR(40) NOT NULL, "USER_UUID" VARCHAR(255) NOT NULL, diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/user/SamlMessageIdDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/user/SamlMessageIdDaoTest.java new file mode 100644 index 00000000000..f7dd0b6920f --- /dev/null +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/user/SamlMessageIdDaoTest.java @@ -0,0 +1,98 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.db.user; + +import java.util.Optional; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.impl.utils.TestSystem2; +import org.sonar.core.util.SequenceUuidFactory; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; + +import static org.assertj.core.api.Assertions.assertThat; + +public class SamlMessageIdDaoTest { + + private static final long NOW = 1_000_000_000L; + + private TestSystem2 system2 = new TestSystem2().setNow(NOW); + @Rule + public DbTester db = DbTester.create(system2); + + private DbSession dbSession = db.getSession(); + private UuidFactory uuidFactory = new SequenceUuidFactory(); + + private SamlMessageIdDao underTest = new SamlMessageIdDao(system2, uuidFactory); + + @Test + public void selectByMessageId() { + SamlMessageIdDto dto = new SamlMessageIdDto() + .setMessageId("ABCD") + .setExpirationDate(15_000_000_000L); + underTest.insert(dbSession, dto); + + Optional result = underTest.selectByMessageId(dbSession, dto.getMessageId()); + + assertThat(result).isPresent(); + assertThat(result.get().getMessageId()).isEqualTo("ABCD"); + assertThat(result.get().getExpirationDate()).isEqualTo(15_000_000_000L); + assertThat(result.get().getCreatedAt()).isEqualTo(NOW); + } + + @Test + public void uuid_created_at_and_updated_at_are_ignored_during_insert() { + SamlMessageIdDto dto = new SamlMessageIdDto() + .setMessageId("ABCD") + .setExpirationDate(15_000_000_000L) + // Following fields should be ignored + .setUuid("SHOULD_NOT_BE_USED") + .setCreatedAt(8_000_000_000L); + underTest.insert(dbSession, dto); + + Optional result = underTest.selectByMessageId(dbSession, dto.getMessageId()); + + assertThat(result).isPresent(); + assertThat(result.get().getUuid()).isNotEqualTo("SHOULD_NOT_BE_USED"); + assertThat(result.get().getCreatedAt()).isEqualTo(NOW); + } + + @Test + public void deleteExpired() { + SamlMessageIdDto expiredSamlMessageId1 = underTest.insert(dbSession, new SamlMessageIdDto() + .setMessageId("MESSAGE_1") + .setExpirationDate(NOW - 2_000_000_000L)); + SamlMessageIdDto expiredSamlMessageId2 = underTest.insert(dbSession, new SamlMessageIdDto() + .setMessageId("MESSAGE_2") + .setExpirationDate(NOW - 2_000_000_000L)); + SamlMessageIdDto validSamlMessageId = underTest.insert(dbSession, new SamlMessageIdDto() + .setMessageId("MESSAGE_3") + .setExpirationDate(NOW + 1_000_000_000L)); + + int result = underTest.deleteExpired(dbSession); + + assertThat(underTest.selectByMessageId(dbSession, expiredSamlMessageId1.getMessageId())).isNotPresent(); + assertThat(underTest.selectByMessageId(dbSession, expiredSamlMessageId2.getMessageId())).isNotPresent(); + assertThat(underTest.selectByMessageId(dbSession, validSamlMessageId.getMessageId())).isPresent(); + assertThat(result).isEqualTo(2); + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTable.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTable.java new file mode 100644 index 00000000000..cca7776d599 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTable.java @@ -0,0 +1,71 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.platform.db.migration.version.v84; + +import java.sql.SQLException; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.def.VarcharColumnDef; +import org.sonar.server.platform.db.migration.sql.CreateIndexBuilder; +import org.sonar.server.platform.db.migration.sql.CreateTableBuilder; +import org.sonar.server.platform.db.migration.step.DdlChange; + +import static org.sonar.server.platform.db.migration.def.BigIntegerColumnDef.newBigIntegerColumnDefBuilder; +import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.UUID_SIZE; + +public class CreateSamlMessageIdsTable extends DdlChange { + + private static final String TABLE_NAME = "saml_message_ids"; + private static final VarcharColumnDef MESSAGE_ID_COLUMN = VarcharColumnDef.newVarcharColumnDefBuilder() + .setColumnName("message_id") + .setLimit(255) + .setIsNullable(false) + .build(); + + public CreateSamlMessageIdsTable(Database db) { + super(db); + } + + @Override + public void execute(Context context) throws SQLException { + context.execute(new CreateTableBuilder(getDialect(), TABLE_NAME) + .addPkColumn(VarcharColumnDef.newVarcharColumnDefBuilder() + .setColumnName("uuid") + .setLimit(UUID_SIZE) + .setIsNullable(false) + .build()) + .addColumn(MESSAGE_ID_COLUMN) + .addColumn(newBigIntegerColumnDefBuilder() + .setColumnName("expiration_date") + .setIsNullable(false) + .build()) + .addColumn(newBigIntegerColumnDefBuilder() + .setColumnName("created_at") + .setIsNullable(false) + .build()) + .build()); + + context.execute(new CreateIndexBuilder() + .setTable(TABLE_NAME) + .setName("saml_message_ids_unique") + .addColumn(MESSAGE_ID_COLUMN) + .setUnique(true) + .build()); + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java index 1d4eac7aed2..811bb596e04 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java @@ -782,6 +782,7 @@ public class DbVersion84 implements DbVersion { .add(3800, "Remove favourites for components with qualifiers 'DIR', 'FIL', 'UTS'", RemoveFilesFavouritesFromProperties.class) .add(3801, "Create 'SESSION_TOKENS' table", CreateSessionTokensTable.class) + .add(3802, "Create 'SAML_MESSAGE_IDS' table", CreateSamlMessageIdsTable.class) ; } } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTableTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTableTest.java new file mode 100644 index 00000000000..86c5ccebf8a --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/CreateSamlMessageIdsTableTest.java @@ -0,0 +1,58 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.platform.db.migration.version.v84; + +import java.sql.SQLException; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.db.CoreDbTester; + +import static java.sql.Types.BIGINT; +import static java.sql.Types.VARCHAR; + +public class CreateSamlMessageIdsTableTest { + + private static final String TABLE_NAME = "saml_message_ids"; + + @Rule + public CoreDbTester dbTester = CoreDbTester.createEmpty(); + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private CreateSamlMessageIdsTable underTest = new CreateSamlMessageIdsTable(dbTester.database()); + + @Test + public void table_has_been_created() throws SQLException { + underTest.execute(); + + dbTester.assertTableExists(TABLE_NAME); + dbTester.assertPrimaryKey(TABLE_NAME, "pk_saml_message_ids", "uuid"); + dbTester.assertUniqueIndex(TABLE_NAME, "saml_message_ids_unique", "message_id"); + + dbTester.assertColumnDefinition(TABLE_NAME, "uuid", VARCHAR, 40, false); + dbTester.assertColumnDefinition(TABLE_NAME, "message_id", VARCHAR, 255, false); + dbTester.assertColumnDefinition(TABLE_NAME, "expiration_date", BIGINT, 20, false); + dbTester.assertColumnDefinition(TABLE_NAME, "created_at", BIGINT, 20, false); + } + +} diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java index c2619579720..18681bee6a0 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java @@ -21,8 +21,8 @@ package org.sonar.server.authentication; import org.sonar.core.platform.Module; import org.sonar.server.authentication.event.AuthenticationEventImpl; -import org.sonar.server.authentication.purge.SessionTokensCleaner; -import org.sonar.server.authentication.purge.SessionTokensCleanerExecutorServiceImpl; +import org.sonar.server.authentication.purge.ExpiredSessionsCleaner; +import org.sonar.server.authentication.purge.ExpiredSessionsCleanerExecutorServiceImpl; public class AuthenticationModule extends Module { @Override @@ -45,8 +45,8 @@ public class AuthenticationModule extends Module { OAuth2ContextFactory.class, OAuthCsrfVerifier.class, RequestAuthenticatorImpl.class, - SessionTokensCleaner.class, - SessionTokensCleanerExecutorServiceImpl.class, + ExpiredSessionsCleaner.class, + ExpiredSessionsCleanerExecutorServiceImpl.class, UserLastConnectionDatesUpdaterImpl.class, UserRegistrarImpl.class, UserSessionInitializer.class); diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleaner.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleaner.java similarity index 61% rename from server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleaner.java rename to server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleaner.java index d5764bf3fd8..0c4bb61ce2c 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleaner.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleaner.java @@ -22,50 +22,58 @@ package org.sonar.server.authentication.purge; import java.util.concurrent.TimeUnit; import org.sonar.api.Startable; -import org.sonar.api.config.Configuration; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.server.util.GlobalLockManager; -public class SessionTokensCleaner implements Startable { +public class ExpiredSessionsCleaner implements Startable { - private static final Logger LOG = Loggers.get(SessionTokensCleaner.class); + private static final Logger LOG = Loggers.get(ExpiredSessionsCleaner.class); - private static final String PURGE_DELAY_CONFIGURATION = "sonar.authentication.session.tokens.purge.delay"; - private static final long DEFAULT_PURGE_DELAY_IN_SECONDS = 24 * 60 * 60L; + private static final long PERIOD_IN_SECONDS = 24 * 60 * 60L; private static final String LOCK_NAME = "SessionCleaner"; - private final SessionTokensCleanerExecutorService executorService; + private final ExpiredSessionsCleanerExecutorService executorService; private final DbClient dbClient; - private final Configuration configuration; private final GlobalLockManager lockManager; - public SessionTokensCleaner(SessionTokensCleanerExecutorService executorService, DbClient dbClient, Configuration configuration, GlobalLockManager lockManager) { + public ExpiredSessionsCleaner(ExpiredSessionsCleanerExecutorService executorService, DbClient dbClient, GlobalLockManager lockManager) { this.executorService = executorService; this.dbClient = dbClient; - this.configuration = configuration; this.lockManager = lockManager; } @Override public void start() { - this.executorService.scheduleAtFixedRate(this::executePurge, 0, configuration.getLong(PURGE_DELAY_CONFIGURATION).orElse(DEFAULT_PURGE_DELAY_IN_SECONDS), TimeUnit.SECONDS); + this.executorService.scheduleAtFixedRate(this::executePurge, 0, PERIOD_IN_SECONDS, TimeUnit.SECONDS); } private void executePurge() { if (!lockManager.tryLock(LOCK_NAME)) { return; } - LOG.debug("Start of cleaning expired session tokens"); try (DbSession dbSession = dbClient.openSession(false)) { - int deletedSessionTokens = dbClient.sessionTokensDao().deleteExpired(dbSession); - dbSession.commit(); - LOG.info("Purge of expired session tokens has removed {} elements", deletedSessionTokens); + cleanExpiredSessionTokens(dbSession); + cleanExpiredSamlMessageIds(dbSession); } } + private void cleanExpiredSessionTokens(DbSession dbSession) { + LOG.debug("Start of cleaning expired session tokens"); + int deletedSessionTokens = dbClient.sessionTokensDao().deleteExpired(dbSession); + dbSession.commit(); + LOG.info("Purge of expired session tokens has removed {} elements", deletedSessionTokens); + } + + private void cleanExpiredSamlMessageIds(DbSession dbSession) { + LOG.debug("Start of cleaning expired SAML message IDs"); + int deleted = dbClient.samlMessageIdDao().deleteExpired(dbSession); + dbSession.commit(); + LOG.info("Purge of expired SAML message ids has removed {} elements", deleted); + } + @Override public void stop() { // nothing to do diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorService.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorService.java similarity index 91% rename from server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorService.java rename to server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorService.java index 551363c6944..3ecd80025c9 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorService.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorService.java @@ -23,5 +23,5 @@ import java.util.concurrent.ScheduledExecutorService; import org.sonar.api.server.ServerSide; @ServerSide -public interface SessionTokensCleanerExecutorService extends ScheduledExecutorService { +public interface ExpiredSessionsCleanerExecutorService extends ScheduledExecutorService { } diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorServiceImpl.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorServiceImpl.java similarity index 86% rename from server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorServiceImpl.java rename to server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorServiceImpl.java index 3a0bbadb7a6..b97d66c3690 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/SessionTokensCleanerExecutorServiceImpl.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerExecutorServiceImpl.java @@ -25,15 +25,15 @@ import org.sonar.server.util.AbstractStoppableScheduledExecutorServiceImpl; import static java.lang.Thread.MIN_PRIORITY; -public class SessionTokensCleanerExecutorServiceImpl +public class ExpiredSessionsCleanerExecutorServiceImpl extends AbstractStoppableScheduledExecutorServiceImpl - implements SessionTokensCleanerExecutorService { + implements ExpiredSessionsCleanerExecutorService { - public SessionTokensCleanerExecutorServiceImpl() { + public ExpiredSessionsCleanerExecutorServiceImpl() { super( Executors.newSingleThreadScheduledExecutor(r -> { Thread thread = Executors.defaultThreadFactory().newThread(r); - thread.setName("SessionTokensCleaner-%d"); + thread.setName("ExpiredSessionsCleaner-%d"); thread.setPriority(MIN_PRIORITY); thread.setDaemon(false); return thread; diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/SessionTokensCleanerTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerTest.java similarity index 76% rename from server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/SessionTokensCleanerTest.java rename to server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerTest.java index 08a28f7f5df..df04c523a22 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/SessionTokensCleanerTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/purge/ExpiredSessionsCleanerTest.java @@ -26,13 +26,12 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.config.Configuration; -import org.sonar.api.config.internal.MapSettings; import org.sonar.api.impl.utils.TestSystem2; import org.sonar.api.utils.log.LogAndArguments; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.db.DbTester; +import org.sonar.db.user.SamlMessageIdDto; import org.sonar.db.user.SessionTokenDto; import org.sonar.db.user.UserDto; import org.sonar.server.util.AbstractStoppableExecutorService; @@ -43,7 +42,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class SessionTokensCleanerTest { +public class ExpiredSessionsCleanerTest { private static final long NOW = 1_000_000_000L; @@ -55,18 +54,15 @@ public class SessionTokensCleanerTest { private GlobalLockManager lockManager = mock(GlobalLockManager.class); - private final MapSettings settings = new MapSettings(); - private final Configuration configuration = settings.asConfig(); - private SyncSessionTokensCleanerExecutorService executorService = new SyncSessionTokensCleanerExecutorService(); - private SessionTokensCleaner underTest = new SessionTokensCleaner(executorService, db.getDbClient(), configuration, lockManager); + private ExpiredSessionsCleaner underTest = new ExpiredSessionsCleaner(executorService, db.getDbClient(), lockManager); @Test public void purge_expired_session_tokens() { when(lockManager.tryLock(anyString())).thenReturn(true); UserDto user = db.users().insertUser(); - SessionTokenDto validSessionToken = db.users().insertSessionToken(user); + SessionTokenDto validSessionToken = db.users().insertSessionToken(user, st -> st.setExpirationDate(NOW + 1_000_000L)); SessionTokenDto expiredSessionToken = db.users().insertSessionToken(user, st -> st.setExpirationDate(NOW - 1_000_000L)); underTest.start(); @@ -76,7 +72,24 @@ public class SessionTokensCleanerTest { assertThat(db.getDbClient().sessionTokensDao().selectByUuid(db.getSession(), expiredSessionToken.getUuid())).isNotPresent(); assertThat(logTester.getLogs(LoggerLevel.INFO)) .extracting(LogAndArguments::getFormattedMsg) - .containsOnly("Purge of expired session tokens has removed 1 elements"); + .contains("Purge of expired session tokens has removed 1 elements"); + } + + @Test + public void purge_expired_saml_message_ids() { + when(lockManager.tryLock(anyString())).thenReturn(true); + db.getDbClient().samlMessageIdDao().insert(db.getSession(), new SamlMessageIdDto().setMessageId("MESSAGE_1").setExpirationDate(NOW + 1_000_000L)); + db.getDbClient().samlMessageIdDao().insert(db.getSession(), new SamlMessageIdDto().setMessageId("MESSAGE_2").setExpirationDate(NOW - 1_000_000L)); + db.commit(); + underTest.start(); + + executorService.runCommand(); + + assertThat(db.getDbClient().samlMessageIdDao().selectByMessageId(db.getSession(), "MESSAGE_1")).isPresent(); + assertThat(db.getDbClient().samlMessageIdDao().selectByMessageId(db.getSession(), "MESSAGE_2")).isNotPresent(); + assertThat(logTester.getLogs(LoggerLevel.INFO)) + .extracting(LogAndArguments::getFormattedMsg) + .contains("Purge of expired SAML message ids has removed 1 elements"); } @Test @@ -90,7 +103,7 @@ public class SessionTokensCleanerTest { assertThat(db.getDbClient().sessionTokensDao().selectByUuid(db.getSession(), expiredSessionToken.getUuid())).isPresent(); } - private static class SyncSessionTokensCleanerExecutorService extends AbstractStoppableExecutorService implements SessionTokensCleanerExecutorService { + private static class SyncSessionTokensCleanerExecutorService extends AbstractStoppableExecutorService implements ExpiredSessionsCleanerExecutorService { private Runnable command; -- 2.39.5