From: Eric Hartmann Date: Mon, 16 Oct 2017 09:21:32 +0000 (+0200) Subject: SONAR-9527 Implement cleanup of deactivated users X-Git-Tag: 6.7-RC1~193 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=76558f8bb1c7c7fff227e343d764656e97094453;p=sonarqube.git SONAR-9527 Implement cleanup of deactivated users --- diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsers.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsers.java new file mode 100644 index 00000000000..5462cc5abb0 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsers.java @@ -0,0 +1,67 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 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.v67; + +import java.sql.SQLException; +import org.sonar.api.utils.System2; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.step.DataChange; +import org.sonar.server.platform.db.migration.step.MassUpdate; + +public class CleanupDisabledUsers extends DataChange { + + private final System2 system2; + private final String falseValue; + + public CleanupDisabledUsers(Database db, System2 system2) { + super(db); + this.falseValue = db.getDialect().getFalseSqlValue(); + this.system2 = system2; + } + + @Override + protected void execute(Context context) throws SQLException { + long now = system2.now(); + + MassUpdate massUpdate = context.prepareMassUpdate(); + massUpdate.select("select u.id from users u " + + "where u.active = " + falseValue + " and " + + "( email is not null or " + + " crypted_password is not null or " + + " salt is not null or " + + " external_identity is not null or " + + " external_identity_provider is not null ) "); + massUpdate.update("update users set " + + " email = null, " + + " crypted_password = null, " + + " salt = null, " + + " external_identity = null, " + + " external_identity_provider = null, " + + " updated_at = ? " + + " where id = ?"); + massUpdate.rowPluralName("deactivated users"); + massUpdate.execute((row, update) -> { + update.setLong(1, now); + update.setLong(2, row.getLong(1)); + return true; + }); + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67.java index 475e3df7db0..0a367054e18 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67.java @@ -29,6 +29,7 @@ public class DbVersion67 implements DbVersion { .add(1830, "Copy deprecated server ID", CopyDeprecatedServerId.class) .add(1831, "Add webhook_deliveries.analysis_uuid", AddAnalysisUuidToWebhookDeliveries.class) .add(1832, "Create table ANALYSIS_PROPERTIES", CreateTableAnalysisProperties.class) + .add(1833, "Cleanup disabled users", CleanupDisabledUsers.class) ; } } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest.java new file mode 100644 index 00000000000..b682dab14b7 --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest.java @@ -0,0 +1,228 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 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.v67; + +import com.google.common.collect.ImmutableList; +import java.sql.SQLException; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Random; +import java.util.stream.Collectors; +import javax.annotation.Nullable; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; +import org.sonar.db.CoreDbTester; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; +import static org.assertj.core.api.Assertions.assertThat; + +public class CleanupDisabledUsersTest { + + private final static long PAST = 100_000_000_000L; + private final static long NOW = 500_000_000_000L; + private final static Random RANDOM = new Random(); + private final static String SELECT_USERS = "select name, scm_accounts, user_local, login, crypted_password, salt, email, external_identity, external_identity_provider, active, is_root, onboarded, created_at, updated_at from users"; + + private System2 system2 = new TestSystem2().setNow(NOW); + + @Rule + public CoreDbTester db = CoreDbTester.createForSchema(CleanupDisabledUsersTest.class, "users.sql"); + + private CleanupDisabledUsers underTest = new CleanupDisabledUsers(db.database(), system2); + + @Test + public void do_nothing_when_no_data() throws SQLException { + assertThat(db.countRowsOfTable("USERS")).isEqualTo(0); + underTest.execute(); + assertThat(db.countRowsOfTable("USERS")).isEqualTo(0); + } + + @Test + public void execute_must_update_database() throws SQLException { + final List users = generateAndInsertUsers(); + + underTest.execute(); + + assertDatabaseContainsExactly(applyCleanupOnUsers(users)); + } + + @Test + public void migration_is_reentrant() throws SQLException { + final List users = generateAndInsertUsers(); + + underTest.execute(); + underTest.execute(); + + assertDatabaseContainsExactly(applyCleanupOnUsers(users)); + } + + private List generateAndInsertUsers() { + List users = ImmutableList.of( + new User("user1", null, null, null, null, null, false), + new User("user2", randomAlphanumeric(10), null, null, null, null, false), + new User("user3", null, randomAlphanumeric(10), null, null, null, false), + new User("user4", null, null, randomAlphanumeric(10), null, null, false), + new User("user5", null, null, null, randomAlphanumeric(10), null, false), + new User("user6", null, null, null, null, randomAlphanumeric(10), false), + new User("user7", randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), false), + new User("user8", randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), true), + new User("user9", randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), randomAlphanumeric(10), true) + ); + + users.forEach(User::insert); + return users; + } + + private List applyCleanupOnUsers(List users) { + return users.stream().map( + u -> { + User cleanedupUser = u.clone(); + + // If a user is active => no change + if (cleanedupUser.active) { + return cleanedupUser; + } + + // updated_at field will only be updated if there is a real change so if at least one field is not null + if (cleanedupUser.cryptedPassword != null || cleanedupUser.salt != null || cleanedupUser.email != null || + cleanedupUser.externalIdentityProvider != null || cleanedupUser.externalIdentity != null) { + cleanedupUser.updatedAt = NOW; + } + + // Cleanup fields, all those fields must be null + cleanedupUser.cryptedPassword = null; + cleanedupUser.salt = null; + cleanedupUser.email = null; + cleanedupUser.externalIdentityProvider = null; + cleanedupUser.externalIdentity = null; + + return cleanedupUser; + }).collect(Collectors.toList()); + } + + private void assertDatabaseContainsExactly(List expectedUsers) { + assertThat(db.select(SELECT_USERS)).isEqualTo( + expectedUsers.stream().map(User::toMap).collect(Collectors.toList()) + ); + } + + private class User { + private String login; + private String cryptedPassword; + private String salt; + private String email; + private String externalIdentity; + private String externalIdentityProvider; + private String name; + private String scmAccounts; + private boolean userLocal; + private boolean active; + private boolean isRoot; + private boolean onBoarded; + private long updatedAt; + private long createdAt; + + private User(String login, @Nullable String cryptedPassword, @Nullable String salt, @Nullable String email, + @Nullable String externalIdentity, @Nullable String externalIdentityProvider, boolean active) { + this.login = login; + this.cryptedPassword = cryptedPassword; + this.salt = salt; + this.email = email; + this.externalIdentity = externalIdentity; + this.externalIdentityProvider = externalIdentityProvider; + this.active = active; + this.isRoot = RANDOM.nextBoolean(); + this.onBoarded = RANDOM.nextBoolean(); + this.userLocal = RANDOM.nextBoolean(); + this.scmAccounts = randomAlphanumeric(1500); + this.name = randomAlphanumeric(200); + this.updatedAt = PAST; + this.createdAt = PAST; + } + + private void insert() { + db.executeInsert("USERS", toMap()); + } + + private Map toMap() { + HashMap map = new HashMap<>(); + map.put("LOGIN", login); + map.put("IS_ROOT", isRoot); + map.put("ONBOARDED", onBoarded); + map.put("ACTIVE", active); + map.put("CREATED_AT", createdAt); + map.put("UPDATED_AT", updatedAt); + map.put("CRYPTED_PASSWORD", cryptedPassword); + map.put("SALT", salt); + map.put("EMAIL", email); + map.put("EXTERNAL_IDENTITY", externalIdentity); + map.put("EXTERNAL_IDENTITY_PROVIDER", externalIdentityProvider); + map.put("NAME", name); + map.put("SCM_ACCOUNTS", scmAccounts); + map.put("USER_LOCAL", userLocal); + + return Collections.unmodifiableMap(map); + } + + @Override + public User clone() { + User user = new User(this.login, this.cryptedPassword, this.salt, this.email, this.externalIdentity, this.externalIdentityProvider, this.active); + + user.name = this.name; + user.scmAccounts = this.scmAccounts; + user.userLocal = this.userLocal; + user.onBoarded = this.onBoarded; + user.isRoot = this.isRoot; + + return user; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof User)) { + return false; + } + User user = (User) o; + return active == user.active && + isRoot == user.isRoot && + onBoarded == user.onBoarded && + Objects.equals(login, user.login) && + Objects.equals(cryptedPassword, user.cryptedPassword) && + Objects.equals(salt, user.salt) && + Objects.equals(email, user.email) && + Objects.equals(externalIdentity, user.externalIdentity) && + Objects.equals(externalIdentityProvider, user.externalIdentityProvider); + } + + @Override + public int hashCode() { + return Objects.hash(login, cryptedPassword, salt, email, externalIdentity, externalIdentityProvider, active, isRoot, onBoarded); + } + } +} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67Test.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67Test.java index 4fef78d7e20..2d59608cc87 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67Test.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v67/DbVersion67Test.java @@ -36,7 +36,7 @@ public class DbVersion67Test { @Test public void verify_migration_count() { - verifyMigrationCount(underTest, 3); + verifyMigrationCount(underTest, 4); } } diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest/users.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest/users.sql new file mode 100644 index 00000000000..8d6baaacafa --- /dev/null +++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v67/CleanupDisabledUsersTest/users.sql @@ -0,0 +1,19 @@ +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), + "IS_ROOT" BOOLEAN NOT NULL, + "USER_LOCAL" BOOLEAN, + "ONBOARDED" BOOLEAN NOT NULL, + "CREATED_AT" BIGINT, + "UPDATED_AT" BIGINT +); +CREATE UNIQUE INDEX "USERS_LOGIN" ON "USERS" ("LOGIN"); +CREATE INDEX "USERS_UPDATED_AT" ON "USERS" ("UPDATED_AT");