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

7 files changed, 188 insertions, 22 deletions

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<PropertyDefinition> 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);
   }
 }