From 5d9a32d1d223f5ee1b21d85b6e5cdb8360dc456f Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 27 Jun 2016 14:39:22 +0200 Subject: [PATCH] SONAR-7781 Password of external providers should not be stored in database --- ...58_remove_users_password_when_not_local.rb | 28 +++++ .../org/sonar/db/version/DatabaseVersion.java | 2 +- .../sonar/db/version/MigrationStepModule.java | 5 +- .../v60/RemoveUsersPasswordWhenNotLocal.java | 51 ++++++++ .../org/sonar/db/version/rows-h2.sql | 1 + .../db/version/MigrationStepModuleTest.java | 2 +- .../RemoveUsersPasswordWhenNotLocalTest.java | 114 ++++++++++++++++++ .../schema.sql | 15 +++ 8 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 server/sonar-web/src/main/webapp/WEB-INF/db/migrate/1258_remove_users_password_when_not_local.rb create mode 100644 sonar-db/src/main/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocal.java create mode 100644 sonar-db/src/test/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest.java create mode 100644 sonar-db/src/test/resources/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest/schema.sql diff --git a/server/sonar-web/src/main/webapp/WEB-INF/db/migrate/1258_remove_users_password_when_not_local.rb b/server/sonar-web/src/main/webapp/WEB-INF/db/migrate/1258_remove_users_password_when_not_local.rb new file mode 100644 index 00000000000..1d8f48a4d4a --- /dev/null +++ b/server/sonar-web/src/main/webapp/WEB-INF/db/migrate/1258_remove_users_password_when_not_local.rb @@ -0,0 +1,28 @@ +# +# SonarQube, open source software quality management tool. +# Copyright (C) 2008-2014 SonarSource +# mailto:contact AT sonarsource DOT com +# +# SonarQube 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. +# +# SonarQube 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. +# +# +# SonarQube 6.0 +# SONAR-7781 +# +class RemoveUsersPasswordWhenNotLocal < ActiveRecord::Migration + def self.up + execute_java_migration('org.sonar.db.version.v60.RemoveUsersPasswordWhenNotLocal') + end +end diff --git a/sonar-db/src/main/java/org/sonar/db/version/DatabaseVersion.java b/sonar-db/src/main/java/org/sonar/db/version/DatabaseVersion.java index 09e6fd5afd8..0099f81fdb8 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/DatabaseVersion.java +++ b/sonar-db/src/main/java/org/sonar/db/version/DatabaseVersion.java @@ -30,7 +30,7 @@ import org.sonar.db.MyBatis; public class DatabaseVersion { - public static final int LAST_VERSION = 1_257; + public static final int LAST_VERSION = 1_258; /** * The minimum supported version which can be upgraded. Lower diff --git a/sonar-db/src/main/java/org/sonar/db/version/MigrationStepModule.java b/sonar-db/src/main/java/org/sonar/db/version/MigrationStepModule.java index 5dd6b7e55e7..064eedfa295 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/MigrationStepModule.java +++ b/sonar-db/src/main/java/org/sonar/db/version/MigrationStepModule.java @@ -131,6 +131,7 @@ import org.sonar.db.version.v60.PopulateUuidColumnOnSnapshots; import org.sonar.db.version.v60.PopulateUuidColumnsOfProjects; import org.sonar.db.version.v60.PopulateUuidColumnsOfResourceIndex; import org.sonar.db.version.v60.PopulateUuidPathColumnOnProjects; +import org.sonar.db.version.v60.RemoveUsersPasswordWhenNotLocal; public class MigrationStepModule extends Module { @Override @@ -275,6 +276,8 @@ public class MigrationStepModule extends Module { // PROJECTS.UUID_PATH AddUuidPathColumnToProjects.class, PopulateUuidPathColumnOnProjects.class, - MakeUuidPathColumnNotNullOnProjects.class); + MakeUuidPathColumnNotNullOnProjects.class, + + RemoveUsersPasswordWhenNotLocal.class); } } diff --git a/sonar-db/src/main/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocal.java b/sonar-db/src/main/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocal.java new file mode 100644 index 00000000000..ddf36cbd6b9 --- /dev/null +++ b/sonar-db/src/main/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocal.java @@ -0,0 +1,51 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact 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.version.v60; + +import java.sql.SQLException; +import org.sonar.api.utils.System2; +import org.sonar.db.Database; +import org.sonar.db.version.BaseDataChange; +import org.sonar.db.version.MassUpdate; + +public class RemoveUsersPasswordWhenNotLocal extends BaseDataChange { + + private final System2 system2; + + public RemoveUsersPasswordWhenNotLocal(Database db, System2 system2) { + super(db); + this.system2 = system2; + } + + @Override + public void execute(Context context) throws SQLException { + long now = system2.now(); + MassUpdate massUpdate = context.prepareMassUpdate(); + massUpdate.select("SELECT u.id FROM users u WHERE (u.crypted_password IS NOT NULL OR u.salt IS NOT NULL) AND u.user_local=?").setBoolean(1, false); + massUpdate.update("UPDATE users SET crypted_password=null, salt=null, updated_at=? WHERE id=?"); + massUpdate.rowPluralName("none local users with password"); + massUpdate.execute((row, update) -> { + update.setLong(1, now); + update.setLong(2, row.getLong(1)); + return true; + }); + } +} diff --git a/sonar-db/src/main/resources/org/sonar/db/version/rows-h2.sql b/sonar-db/src/main/resources/org/sonar/db/version/rows-h2.sql index ce75f9372b6..207cb3d1b81 100644 --- a/sonar-db/src/main/resources/org/sonar/db/version/rows-h2.sql +++ b/sonar-db/src/main/resources/org/sonar/db/version/rows-h2.sql @@ -464,6 +464,7 @@ INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1254'); INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1255'); INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1256'); INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1257'); +INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1258'); INSERT INTO USERS(ID, LOGIN, NAME, EMAIL, EXTERNAL_IDENTITY, EXTERNAL_IDENTITY_PROVIDER, USER_LOCAL, CRYPTED_PASSWORD, SALT, CREATED_AT, UPDATED_AT) VALUES (1, 'admin', 'Administrator', '', 'admin', 'sonarqube', true, 'a373a0e667abb2604c1fd571eb4ad47fe8cc0878', '48bc4b0d93179b5103fd3885ea9119498e9d161b', '1418215735482', '1418215735482'); ALTER TABLE USERS ALTER COLUMN ID RESTART WITH 2; diff --git a/sonar-db/src/test/java/org/sonar/db/version/MigrationStepModuleTest.java b/sonar-db/src/test/java/org/sonar/db/version/MigrationStepModuleTest.java index a4d75a54649..6c85da727e3 100644 --- a/sonar-db/src/test/java/org/sonar/db/version/MigrationStepModuleTest.java +++ b/sonar-db/src/test/java/org/sonar/db/version/MigrationStepModuleTest.java @@ -29,6 +29,6 @@ public class MigrationStepModuleTest { public void verify_count_of_added_MigrationStep_types() { ComponentContainer container = new ComponentContainer(); new MigrationStepModule().configure(container); - assertThat(container.size()).isEqualTo(113); + assertThat(container.size()).isEqualTo(114); } } diff --git a/sonar-db/src/test/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest.java b/sonar-db/src/test/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest.java new file mode 100644 index 00000000000..dc1744fdb76 --- /dev/null +++ b/sonar-db/src/test/java/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest.java @@ -0,0 +1,114 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact 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.version.v60; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.utils.System2; +import org.sonar.db.DbTester; +import org.sonar.db.version.MigrationStep; + +public class RemoveUsersPasswordWhenNotLocalTest { + + static long PAST_DATE = 1_000_000_000L; + static long NOW = 2_000_000_000L; + + System2 system2 = mock(System2.class); + + @Rule + public DbTester db = DbTester.createForSchema(System2.INSTANCE, RemoveUsersPasswordWhenNotLocalTest.class, "schema.sql"); + + private MigrationStep migration = new RemoveUsersPasswordWhenNotLocal(db.database(), system2); + + @Before + public void setUp() throws Exception { + when(system2.now()).thenReturn(NOW); + } + + @Test + public void update_not_local_user() throws Exception { + insertUserWithPassword("john", false); + + migration.execute(); + + List> rows = db.select("SELECT CRYPTED_PASSWORD,SALT,UPDATED_AT FROM users"); + assertThat(rows).hasSize(1); + assertThat(rows.get(0).get("CRYPTED_PASSWORD")).isNull(); + assertThat(rows.get(0).get("SALT")).isNull(); + assertThat(rows.get(0).get("UPDATED_AT")).isEqualTo(NOW); + } + + @Test + public void ignore_local_user() throws Exception { + insertUserWithPassword("john", true); + + migration.execute(); + + List> rows = db.select("SELECT CRYPTED_PASSWORD,SALT,UPDATED_AT FROM users"); + assertThat(rows).hasSize(1); + assertThat(rows.get(0).get("CRYPTED_PASSWORD")).isNotNull(); + assertThat(rows.get(0).get("SALT")).isNotNull(); + assertThat(rows.get(0).get("UPDATED_AT")).isEqualTo(PAST_DATE); + } + + @Test + public void ignore_already_migrated_user() throws Exception { + insertUserWithoutPasword("john", false); + + migration.execute(); + + List> rows = db.select("SELECT CRYPTED_PASSWORD,SALT,UPDATED_AT FROM users"); + assertThat(rows).hasSize(1); + assertThat(rows.get(0).get("CRYPTED_PASSWORD")).isNull(); + assertThat(rows.get(0).get("SALT")).isNull(); + assertThat(rows.get(0).get("UPDATED_AT")).isEqualTo(PAST_DATE); + } + + private void insertUserWithPassword(String login, boolean isLocal) { + db.executeInsert( + "users", + "LOGIN", login, + "NAME", login, + "USER_LOCAL", Boolean.toString(isLocal), + "CRYPTED_PASSWORD", "crypted_" + login, + "SALT", "salt" + login, + "CREATED_AT", Long.toString(PAST_DATE), + "UPDATED_AT", Long.toString(PAST_DATE)); + } + + private void insertUserWithoutPasword(String login, boolean isLocal) { + db.executeInsert( + "users", + "LOGIN", login, + "NAME", login, + "USER_LOCAL", Boolean.toString(isLocal), + "CREATED_AT", Long.toString(PAST_DATE), + "UPDATED_AT", Long.toString(PAST_DATE)); + } + +} diff --git a/sonar-db/src/test/resources/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest/schema.sql b/sonar-db/src/test/resources/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest/schema.sql new file mode 100644 index 00000000000..cae775f31cb --- /dev/null +++ b/sonar-db/src/test/resources/org/sonar/db/version/v60/RemoveUsersPasswordWhenNotLocalTest/schema.sql @@ -0,0 +1,15 @@ +CREATE TABLE "USERS" ( + "ID" INTEGER NOT NULL GENERATED BY DEFAULT AS IDENTITY (START WITH 1, INCREMENT BY 1), + "LOGIN" VARCHAR(255), + "NAME" VARCHAR(200), + "EMAIL" VARCHAR(100), + "CRYPTED_PASSWORD" VARCHAR(40), + "SALT" VARCHAR(40), + "ACTIVE" BOOLEAN DEFAULT TRUE, + "SCM_ACCOUNTS" VARCHAR(4000), + "EXTERNAL_IDENTITY" VARCHAR(255), + "EXTERNAL_IDENTITY_PROVIDER" VARCHAR(100), + "USER_LOCAL" BOOLEAN, + "CREATED_AT" BIGINT, + "UPDATED_AT" BIGINT +); -- 2.39.5