Don't allow users to edit their name if it comes from LDAP

If the LDAP server is supplying the full name for a user, always
prefer to use the LDAP server's data rather than letting the user
edit it in Gerrit Code Review.  This allows us to always update
the account anytime the name changes on the LDAP side, but we also
prevent users from spoofing themselves in Gerrit, instead they will
always show up with the name their organizational administrator wants
them to have.

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-08-19 09:43:13 -07:00
parent 59e09227e8
commit 3b39128525
10 changed files with 124 additions and 20 deletions

View File

@@ -76,6 +76,7 @@ class ContactPanelShort extends Composite {
nameTxt = new NpTextBox();
nameTxt.setVisibleLength(60);
nameTxt.setReadOnly(!canEditFullName());
emailPick = new ListBox();
@@ -95,7 +96,7 @@ class ContactPanelShort extends Composite {
});
final FlowPanel emailLine = new FlowPanel();
emailLine.add(emailPick);
if (Gerrit.getConfig().isAllowRegisterNewEmail()) {
if (canRegisterNewEmail()) {
emailLine.add(registerNewEmail);
}
@@ -137,6 +138,14 @@ class ContactPanelShort extends Composite {
});
}
private boolean canEditFullName() {
return Gerrit.getConfig().canEdit(Account.FieldName.FULL_NAME);
}
private boolean canRegisterNewEmail() {
return Gerrit.getConfig().canEdit(Account.FieldName.REGISTER_NEW_EMAIL);
}
void hideSaveButton() {
save.setVisible(false);
}
@@ -209,7 +218,7 @@ class ContactPanelShort extends Composite {
if (emailPick.getItemCount() > 0) {
emailPick.setVisible(true);
emailPick.setEnabled(true);
if (Gerrit.getConfig().isAllowRegisterNewEmail()) {
if (canRegisterNewEmail()) {
final String t = Util.C.buttonOpenRegisterNewEmail();
emailPick.addItem("... " + t + " ", t);
}
@@ -234,7 +243,7 @@ class ContactPanelShort extends Composite {
}
private void doRegisterNewEmail() {
if (!Gerrit.getConfig().isAllowRegisterNewEmail()) {
if (!canRegisterNewEmail()) {
return;
}
@@ -290,7 +299,7 @@ class ContactPanelShort extends Composite {
}
void doSave() {
String newName = nameTxt.getText();
String newName = canEditFullName() ? nameTxt.getText() : null;
if ("".equals(newName)) {
newName = null;
}

View File

@@ -95,8 +95,10 @@ class SshPanel extends Composite {
}
userNameTxt.addStyleName("gerrit-SshPanel-username");
userNameTxt.setVisibleLength(16);
userNameTxt.setReadOnly(!canEditSshUserName());
changeUserName = new Button(Util.C.buttonChangeSshUserName());
changeUserName.setVisible(canEditSshUserName());
changeUserName.setEnabled(false);
changeUserName.addClickHandler(new ClickHandler() {
@Override
@@ -209,6 +211,10 @@ class SshPanel extends Composite {
initWidget(body);
}
private boolean canEditSshUserName() {
return Gerrit.getConfig().canEdit(Account.FieldName.SSH_USER_NAME);
}
protected void row(final Grid info, final int row, final String name,
final Widget field) {
info.setText(row, labelIdx, name);
@@ -223,6 +229,10 @@ class SshPanel extends Composite {
}
void doChangeUserName() {
if(!canEditSshUserName()){
return;
}
String newName = userNameTxt.getText();
if ("".equals(newName)) {
newName = null;

View File

@@ -14,9 +14,12 @@
package com.google.gerrit.client.data;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.AuthType;
import com.google.gerrit.client.reviewdb.Project;
import java.util.Set;
public class GerritConfig implements Cloneable {
protected String canonicalUrl;
protected GitwebLink gitweb;
@@ -29,6 +32,7 @@ public class GerritConfig implements Cloneable {
protected String sshdAddress;
protected Project.NameKey wildProject;
protected ApprovalTypes approvalTypes;
protected Set<Account.FieldName> editableAccountFields;
public String getCanonicalUrl() {
return canonicalUrl;
@@ -70,14 +74,6 @@ public class GerritConfig implements Cloneable {
useContactInfo = r;
}
public boolean isAllowRegisterNewEmail() {
return allowRegisterNewEmail;
}
public void setAllowRegisterNewEmail(final boolean r) {
allowRegisterNewEmail = r;
}
public boolean isUseRepoDownload() {
return useRepoDownload;
}
@@ -120,4 +116,16 @@ public class GerritConfig implements Cloneable {
public void setApprovalTypes(final ApprovalTypes at) {
approvalTypes = at;
}
public boolean canEdit(final Account.FieldName f) {
return editableAccountFields.contains(f);
}
public Set<Account.FieldName> getEditableAccountFields() {
return editableAccountFields;
}
public void setEditableAccountFields(final Set<Account.FieldName> af) {
editableAccountFields = af;
}
}

View File

@@ -55,6 +55,10 @@ import java.sql.Timestamp;
* </ul>
*/
public final class Account {
public static enum FieldName {
FULL_NAME, SSH_USER_NAME, REGISTER_NEW_EMAIL;
}
/** Key local to Gerrit to identify a user. */
public static class Id extends IntKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L;

View File

@@ -120,6 +120,7 @@ public class AccountManager {
final AccountExternalId extId) throws OrmException, AccountException {
final Transaction txn = db.beginTransaction();
final Account account = db.accounts().get(extId.getAccountId());
boolean updateAccount = false;
if (account == null) {
throw new AccountException("Account has been deleted");
}
@@ -131,21 +132,42 @@ public class AccountManager {
final String oldEmail = extId.getEmailAddress();
if (newEmail != null && !newEmail.equals(oldEmail)) {
if (oldEmail != null && oldEmail.equals(account.getPreferredEmail())) {
updateAccount = true;
account.setPreferredEmail(newEmail);
db.accounts().update(Collections.singleton(account), txn);
}
extId.setEmailAddress(newEmail);
}
if (!realm.allowsEdit(Account.FieldName.FULL_NAME)
&& !eq(account.getFullName(), who.getDisplayName())) {
updateAccount = true;
account.setFullName(who.getDisplayName());
}
if (!realm.allowsEdit(Account.FieldName.SSH_USER_NAME)
&& !eq(account.getSshUserName(), who.getSshUserName())) {
updateAccount = true;
account.setSshUserName(who.getSshUserName());
}
extId.setLastUsedOn();
db.accountExternalIds().update(Collections.singleton(extId), txn);
if (updateAccount) {
db.accounts().update(Collections.singleton(account), txn);
}
txn.commit();
if (newEmail != null && !newEmail.equals(oldEmail)) {
byEmailCache.evict(oldEmail);
byEmailCache.evict(newEmail);
}
if (updateAccount) {
byIdCache.evict(account.getId());
}
}
private static boolean eq(final String a, final String b) {
return (a == null && b == null) || (a != null && a.equals(b));
}
private AuthResult create(final ReviewDb db, final AuthRequest who)

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.account;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.inject.Inject;
@@ -27,6 +28,11 @@ public final class DefaultRealm implements Realm {
this.emailExpander = emailExpander;
}
@Override
public boolean allowsEdit(final Account.FieldName field) {
return true;
}
@Override
public AuthRequest authenticate(final AuthRequest who) {
if (who.getEmailAddress() == null && who.getLocalUser() != null

View File

@@ -14,11 +14,15 @@
package com.google.gerrit.server.account;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.AccountGroup;
import java.util.Set;
public interface Realm {
/** Can the end-user modify this field of their own account? */
public boolean allowsEdit(Account.FieldName field);
public AuthRequest authenticate(AuthRequest who) throws AccountException;
public Set<AccountGroup.Id> groups(AccountState who);

View File

@@ -17,7 +17,9 @@ package com.google.gerrit.server.http;
import com.google.gerrit.client.data.ApprovalTypes;
import com.google.gerrit.client.data.GerritConfig;
import com.google.gerrit.client.data.GitwebLink;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -34,6 +36,8 @@ import org.spearce.jgit.lib.Config;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.HashSet;
import java.util.Set;
public class GerritConfigProvider implements Provider<GerritConfig> {
private static boolean isIPv6(final InetAddress ip) {
@@ -41,6 +45,7 @@ public class GerritConfigProvider implements Provider<GerritConfig> {
&& ip.getHostName().equals(ip.getHostAddress());
}
private final Realm realm;
private final Config cfg;
private final String canonicalWebUrl;
private final AuthConfig authConfig;
@@ -52,10 +57,11 @@ public class GerritConfigProvider implements Provider<GerritConfig> {
private final ContactStore contactStore;
@Inject
GerritConfigProvider(@GerritServerConfig final Config gsc,
GerritConfigProvider(final Realm r, @GerritServerConfig final Config gsc,
@CanonicalWebUrl @Nullable final String cwu, final AuthConfig ac,
@WildProjectName final Project.NameKey wp, final SshInfo si,
final ApprovalTypes at, final ContactStore cs) {
realm = r;
cfg = gsc;
canonicalWebUrl = cwu;
authConfig = ac;
@@ -79,12 +85,21 @@ public class GerritConfigProvider implements Provider<GerritConfig> {
config.setUseRepoDownload(cfg.getBoolean("repo", null,
"showdownloadcommand", false));
config.setUseContactInfo(contactStore != null && contactStore.isEnabled());
config.setAllowRegisterNewEmail(emailSender != null
&& emailSender.isEnabled());
config.setAuthType(authConfig.getLoginType());
config.setWildProject(wildProject);
config.setApprovalTypes(approvalTypes);
final Set<Account.FieldName> fields = new HashSet<Account.FieldName>();
for (final Account.FieldName n : Account.FieldName.values()) {
if (realm.allowsEdit(n)) {
fields.add(n);
}
}
if (emailSender != null && emailSender.isEnabled()) {
fields.add(Account.FieldName.REGISTER_NEW_EMAIL);
}
config.setEditableAccountFields(fields);
final String gitwebUrl = cfg.getString("gitweb", null, "url");
if (gitwebUrl != null) {
config.setGitwebLink(new GitwebLink(gitwebUrl));

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.ldap;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.AccountExternalId;
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.server.account.AccountException;
@@ -172,6 +173,20 @@ class LdapRealm implements Realm {
return v;
}
@Override
public boolean allowsEdit(final Account.FieldName field) {
switch (field) {
case FULL_NAME:
return accountDisplayName == null; // only if not obtained from LDAP
case SSH_USER_NAME:
return accountSshUserName == null; // only if not obtained from LDAP
default:
return true;
}
}
public AuthRequest authenticate(final AuthRequest who)
throws AccountException {
final String username = who.getLocalUser();

View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.contact.ContactStore;
import com.google.gerrit.server.mail.EmailException;
@@ -68,6 +69,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
private final Logger log = LoggerFactory.getLogger(getClass());
private final ContactStore contactStore;
private final AuthConfig authConfig;
private final Realm realm;
private final RegisterNewEmailSender.Factory registerNewEmailFactory;
private final SshKeyCache sshKeyCache;
private final AccountByEmailCache byEmailCache;
@@ -80,13 +82,15 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
@Inject
AccountSecurityImpl(final Provider<ReviewDb> schema,
final Provider<CurrentUser> currentUser, final ContactStore cs,
final AuthConfig ac, final RegisterNewEmailSender.Factory esf,
final SshKeyCache skc, final AccountByEmailCache abec,
final AccountCache uac, final AccountManager am,
final AuthConfig ac, final Realm r,
final RegisterNewEmailSender.Factory esf, final SshKeyCache skc,
final AccountByEmailCache abec, final AccountCache uac,
final AccountManager am,
final ExternalIdDetailFactory.Factory externalIdDetailFactory) {
super(schema, currentUser);
contactStore = cs;
authConfig = ac;
realm = r;
registerNewEmailFactory = esf;
sshKeyCache = skc;
byEmailCache = abec;
@@ -174,6 +178,11 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
@Override
public void changeSshUserName(final String newName,
final AsyncCallback<VoidResult> callback) {
if (!realm.allowsEdit(Account.FieldName.SSH_USER_NAME)) {
callback.onFailure(new NameAlreadyUsedException());
return;
}
run(callback, new Action<VoidResult>() {
@Override
public VoidResult run(ReviewDb db) throws OrmException, Failure {
@@ -274,7 +283,9 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
public Account run(ReviewDb db) throws OrmException, Failure {
final Account me = db.accounts().get(getAccountId());
final String oldEmail = me.getPreferredEmail();
me.setFullName(name != null && !name.isEmpty() ? name : null);
if (realm.allowsEdit(Account.FieldName.FULL_NAME)) {
me.setFullName(name != null && !name.isEmpty() ? name : null);
}
me.setPreferredEmail(emailAddr);
if (useContactInfo) {
if (ContactInformation.hasAddress(info)