From 3f8385ba1e0b1f67966bfd0d51af4cb52235cfb9 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Mon, 9 Aug 2010 17:41:31 -0600 Subject: [PATCH] Add ability to deactivate a user when they leave the project. Add a inactive column to the Account object. Use the inactive status to disable the user's web and ssh logins, sending emails to the user on behalf of gerrit, adding the user as a reviewer or to a group, and making the user appear in the "add reviewer" and group "add member" auto completion boxes. Bug: issue 503 Change-Id: Ib002788ebf8204dfea608d9f5ac3a5cdff20f817 --- .../gerrit/common/data/ReviewerResult.java | 3 +++ .../gerrit/common/data/SuggestService.java | 2 +- .../errors/InactiveAccountException.java | 26 +++++++++++++++++++ .../google/gerrit/client/GerritConstants.java | 2 ++ .../gerrit/client/GerritConstants.properties | 2 ++ .../gerrit/client/changes/ApprovalTable.java | 4 +++ .../gerrit/client/changes/ChangeMessages.java | 1 + .../client/changes/ChangeMessages.properties | 1 + .../gerrit/client/rpc/GerritCallback.java | 9 +++++++ .../client/ui/AccountSuggestOracle.java | 3 ++- .../gerrit/httpd/ProjectDigestFilter.java | 2 +- .../gerrit/httpd/rpc/SuggestServiceImpl.java | 18 +++++++++---- .../rpc/account/GroupAdminServiceImpl.java | 4 +++ .../gerrit/httpd/rpc/patch/AddReviewer.java | 5 ++++ .../com/google/gerrit/reviewdb/Account.java | 12 +++++++++ .../gerrit/server/account/AccountManager.java | 13 +++++++--- .../gerrit/server/mail/OutgoingEmail.java | 2 +- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_41.java | 25 ++++++++++++++++++ .../gerrit/sshd/DatabasePubKeyAuth.java | 5 ++++ 20 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_41.java diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerResult.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerResult.java index 19772fcf49..678ec79a7e 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerResult.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerResult.java @@ -50,6 +50,9 @@ public class ReviewerResult { /** Name supplied does not match to a registered account. */ ACCOUNT_NOT_FOUND, + /** The account is inactive. */ + ACCOUNT_INACTIVE, + /** The account is not permitted to see the change. */ CHANGE_NOT_VISIBLE, diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/SuggestService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/SuggestService.java index 164df439fb..9dae169830 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/SuggestService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/SuggestService.java @@ -28,7 +28,7 @@ public interface SuggestService extends RemoteJsonService { void suggestProjectNameKey(String query, int limit, AsyncCallback> callback); - void suggestAccount(String query, int limit, + void suggestAccount(String query, Boolean enabled, int limit, AsyncCallback> callback); void suggestAccountGroup(String query, int limit, diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java b/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java new file mode 100644 index 0000000000..6ae5eb6d5d --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java @@ -0,0 +1,26 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.errors; + +/** Error indicating the account is currently inactive. */ +public class InactiveAccountException extends Exception { + private static final long serialVersionUID = 1L; + + public static final String MESSAGE = "Account Inactive: "; + + public InactiveAccountException(String who) { + super(MESSAGE + who); + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java index 3f263d23fd..b32a32d061 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java @@ -46,6 +46,8 @@ public interface GerritConstants extends Constants { String nameAlreadyUsedBody(); String noSuchAccountTitle(); + String inactiveAccountBody(); + String menuAll(); String menuAllOpen(); String menuAllMerged(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties index 83a06e47de..33630150d3 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties @@ -29,6 +29,8 @@ notFoundBody = The page you requested was not found. nameAlreadyUsedBody = The name is already in use. noSuchAccountTitle = Code Review - Unknown User +inactiveAccountBody = This user is currently inactive. + menuAll = All menuAllOpen = Open menuAllMerged = Merged diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java index f850a33eda..23dd635719 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java @@ -188,6 +188,10 @@ public class ApprovalTable extends Composite { r.append(Util.M.accountNotFound(e.getName())); break; + case ACCOUNT_INACTIVE: + r.append(Util.M.accountInactive(e.getName())); + break; + case CHANGE_NOT_VISIBLE: r.append(Util.M.changeNotVisibleTo(e.getName())); break; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java index 51e2bc5896..fd7c363159 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java @@ -47,6 +47,7 @@ public interface ChangeMessages extends Messages { String changeQueryPageTitle(String query); String accountNotFound(String who); + String accountInactive(String who); String changeNotVisibleTo(String who); String anonymousDownload(String protocol); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties index 5d2d3a5014..80f9dabc9e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties @@ -28,6 +28,7 @@ changeQueryWindowTitle = {0} changeQueryPageTitle = Search for {0} accountNotFound = {0} is not a registered user. +accountInactive = {0} is not an active user. changeNotVisibleTo = {0} cannot access the change. anonymousDownload = Anonymous {0} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java index fa028d75ba..f29742ad92 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java @@ -17,6 +17,7 @@ package com.google.gerrit.client.rpc; import com.google.gerrit.client.ErrorDialog; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.NotSignedInDialog; +import com.google.gerrit.common.errors.InactiveAccountException; import com.google.gerrit.common.errors.NameAlreadyUsedException; import com.google.gerrit.common.errors.NoSuchAccountException; import com.google.gerrit.common.errors.NoSuchEntityException; @@ -37,6 +38,9 @@ public abstract class GerritCallback implements AsyncCallback { } else if (isNoSuchEntity(caught)) { new ErrorDialog(Gerrit.C.notFoundBody()).center(); + } else if (isInactiveAccount(caught)) { + new ErrorDialog(Gerrit.C.inactiveAccountBody()).center(); + } else if (isNoSuchAccount(caught)) { final String msg = caught.getMessage(); final String who = msg.substring(NoSuchAccountException.MESSAGE.length()); @@ -71,6 +75,11 @@ public abstract class GerritCallback implements AsyncCallback { && caught.getMessage().equals(NoSuchEntityException.MESSAGE); } + protected static boolean isInactiveAccount(final Throwable caught) { + return caught instanceof RemoteJsonException + && caught.getMessage().startsWith(InactiveAccountException.MESSAGE); + } + private static boolean isNoSuchAccount(final Throwable caught) { return caught instanceof RemoteJsonException && caught.getMessage().startsWith(NoSuchAccountException.MESSAGE); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java index 441878f41a..bcf6438ec8 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java @@ -30,7 +30,8 @@ public class AccountSuggestOracle extends HighlightSuggestOracle { public void onRequestSuggestions(final Request req, final Callback callback) { RpcStatus.hide(new Runnable() { public void run() { - SuggestUtil.SVC.suggestAccount(req.getQuery(), req.getLimit(), + SuggestUtil.SVC.suggestAccount(req.getQuery(), Boolean.TRUE, + req.getLimit(), new GerritCallback>() { public void onSuccess(final List result) { final ArrayList r = diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectDigestFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectDigestFilter.java index 9d9daec2f2..f67f12fd37 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectDigestFilter.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectDigestFilter.java @@ -133,7 +133,7 @@ class ProjectDigestFilter implements Filter { } final AccountState who = accountCache.getByUsername(username); - if (who == null) { + if (who == null || ! who.getAccount().isActive()) { rsp.sendError(SC_UNAUTHORIZED); return false; } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java index f9721f8647..864c550ded 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java @@ -33,6 +33,7 @@ import com.google.inject.Provider; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; class SuggestServiceImpl extends BaseServiceImplementation implements SuggestService { @@ -74,8 +75,8 @@ class SuggestServiceImpl extends BaseServiceImplementation implements }); } - public void suggestAccount(final String query, final int limit, - final AsyncCallback> callback) { + public void suggestAccount(final String query, final Boolean active, + final int limit, final AsyncCallback> callback) { run(callback, new Action>() { public List run(final ReviewDb db) throws OrmException { final String a = query; @@ -86,12 +87,12 @@ class SuggestServiceImpl extends BaseServiceImplementation implements final LinkedHashMap r = new LinkedHashMap(); for (final Account p : db.accounts().suggestByFullName(a, b, n)) { - r.put(p.getId(), new AccountInfo(p)); + addSuggestion(r, p, new AccountInfo(p), active); } if (r.size() < n) { for (final Account p : db.accounts().suggestByPreferredEmail(a, b, n - r.size())) { - r.put(p.getId(), new AccountInfo(p)); + addSuggestion(r, p, new AccountInfo(p), active); } } if (r.size() < n) { @@ -101,7 +102,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements final Account p = accountCache.get(e.getAccountId()).getAccount(); final AccountInfo info = new AccountInfo(p); info.setPreferredEmail(e.getEmailAddress()); - r.put(e.getAccountId(), info); + addSuggestion(r, p, info, active); } } } @@ -110,6 +111,13 @@ class SuggestServiceImpl extends BaseServiceImplementation implements }); } + private void addSuggestion(Map map, Account account, AccountInfo info, + Boolean active) { + if (active == null || active == account.isActive()) { + map.put(account.getId(), info); + } + } + public void suggestAccountGroup(final String query, final int limit, final AsyncCallback> callback) { run(callback, new Action>() { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java index 4e4f82e7f8..870d77ce91 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java @@ -16,6 +16,7 @@ package com.google.gerrit.httpd.rpc.account; import com.google.gerrit.common.data.GroupAdminService; import com.google.gerrit.common.data.GroupDetail; +import com.google.gerrit.common.errors.InactiveAccountException; import com.google.gerrit.common.errors.NameAlreadyUsedException; import com.google.gerrit.common.errors.NoSuchAccountException; import com.google.gerrit.common.errors.NoSuchEntityException; @@ -221,6 +222,9 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements } final Account a = findAccount(nameOrEmail); + if (!a.isActive()) { + throw new Failure(new InactiveAccountException(a.getFullName())); + } if (!control.canAdd(a.getId())) { throw new Failure(new NoSuchEntityException()); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewer.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewer.java index ae36388dc9..9d508e2fcf 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewer.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewer.java @@ -94,6 +94,11 @@ class AddReviewer extends Handler { ReviewerResult.Error.Type.ACCOUNT_NOT_FOUND, nameOrEmail)); continue; } + if (!account.isActive()) { + result.addError(new ReviewerResult.Error( + ReviewerResult.Error.Type.ACCOUNT_INACTIVE, nameOrEmail)); + continue; + } final IdentifiedUser user = identifiedUserFactory.create(account.getId()); if (!control.forUser(user).isVisible()) { diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Account.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Account.java index f428a22790..43b7b17ba7 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Account.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Account.java @@ -129,6 +129,10 @@ public final class Account { @Column(id = 6, name = Column.NONE) protected AccountGeneralPreferences generalPreferences; + /** Is this user active */ + @Column(id = 7) + protected boolean inactive; + /** computed the username selected from the identities. */ protected String userName; @@ -198,6 +202,14 @@ public final class Account { contactFiledOn = new Timestamp(System.currentTimeMillis()); } + public boolean isActive() { + return ! inactive; + } + + public void setActive(boolean active) { + inactive = ! active; + } + /** @return the computed user name for this account */ public String getUserName() { return userName; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index 24ddd27f39..83a5eedf56 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -100,7 +100,7 @@ public class AccountManager { * @param who identity of the user, with any details we received about them. * @return the result of authenticating the user. * @throws AccountException the account does not exist, and cannot be created, - * or exists, but cannot be located. + * or exists, but cannot be located, or is inactive. */ public AuthResult authenticate(AuthRequest who) throws AccountException { who = realm.authenticate(who); @@ -114,9 +114,14 @@ public class AccountManager { // return create(db, who); - } else { - // Account exists, return the identity to the caller. - // + } else { // Account exists + + Account act = db.accounts().get(id.getAccountId()); + if (act == null || !act.isActive()) { + throw new AccountException("Authentication error, account inactive"); + } + + // return the identity to the caller. update(db, who, id); return new AuthResult(id.getAccountId(), key, false); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java index fe666367d3..451d097a4a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java @@ -331,7 +331,7 @@ public abstract class OutgoingEmail { private Address toAddress(final Account.Id id) { final Account a = args.accountCache.get(id).getAccount(); final String e = a.getPreferredEmail(); - if (e == null) { + if (!a.isActive() || e == null) { return null; } return new Address(a.getFullName(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 2f8d4ca86b..78edbc32d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - private static final Class C = Schema_40.class; + private static final Class C = Schema_41.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_41.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_41.java new file mode 100644 index 0000000000..508db43665 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_41.java @@ -0,0 +1,25 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.schema; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +public class Schema_41 extends SchemaVersion { + @Inject + Schema_41(Provider prior) { + super(prior); + } +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java index 6c96b702f1..977d20914f 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java @@ -135,6 +135,11 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator { } } + if (!createUser(sd, key).getAccount().isActive()) { + sd.authenticationError(username, "inactive-account"); + return false; + } + return success(username, session, sd, createUser(sd, key)); }