opendev/gerrit
Code Issues Proposed changes

Cleanup display of external ids in the user settings

Browse Source 
Both Google Accounts and Yahoo! IDs are very long base 64 encoded
strings of binary data.  There is nothing in there that a user can
decipher on their own, so there is no value in showing the string
to them as-is.  Instead we reformat it to a cleaner value that has
just the name of the identity provider.  Most other OpenID sites
include your username with them as part of the URL string, so we
leave those alone.

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-08-08 14:53:09 -07:00
parent 98ccac8c45
commit 31e55395a0
7 changed files with 62 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.client.account;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.SignInDialog;
import com.google.gerrit.client.openid.OpenIdUtil;
import com.google.gerrit.client.reviewdb.AccountExternalId;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.FancyFlexTable;
@@ -28,6 +29,8 @@ import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -146,6 +149,17 @@ class ExternalIdPanel extends Composite {
}
void display(final List<AccountExternalId> result) {
Collections.sort(result, new Comparator<AccountExternalId>() {
@Override
public int compare(AccountExternalId a, AccountExternalId b) {
return emailOf(a).compareTo(emailOf(b));
}
private String emailOf(final AccountExternalId a) {
return a.getEmailAddress() != null ? a.getEmailAddress() : "";
}
});
while (1 < table.getRowCount())
table.removeRow(table.getRowCount() - 1);
@@ -174,18 +188,14 @@ class ExternalIdPanel extends Composite {
table.insertRow(row);
applyDataRowStyle(row);
if (k.canUserDelete()) {
table.setWidget(row, 1, new CheckBox());
} else {
table.setHTML(row, 1, "&nbsp;");
}
if (k.getLastUsedOn() != null) {
table.setText(row, 2, FormatUtil.mediumFormat(k.getLastUsedOn()));
} else {
table.setHTML(row, 2, "&nbsp;");
table.setText(row, 2, "");
}
if (k.isTrusted()) {
table.setHTML(row, 3, "&nbsp;");
table.setText(row, 3, "");
} else {
table.setText(row, 3, Util.C.untrustedProvider());
fmt.addStyleName(row, 3, "gerrit-Identity-UntrustedExternalId");
@@ -193,9 +203,9 @@ class ExternalIdPanel extends Composite {
if (k.getEmailAddress() != null && k.getEmailAddress().length() > 0) {
table.setText(row, 4, k.getEmailAddress());
} else {
table.setHTML(row, 4, "&nbsp;");
table.setText(row, 4, "");
}
table.setText(row, 5, k.getExternalId());
table.setText(row, 5, describe(k));
fmt.addStyleName(row, 1, S_ICON_CELL);
fmt.addStyleName(row, 2, S_DATA_CELL);
@@ -206,5 +216,32 @@ class ExternalIdPanel extends Composite {
setRowItem(row, k);
}
private String describe(final AccountExternalId k) {
final String id = k.getExternalId();
if (k.isScheme(AccountExternalId.SCHEME_GERRIT)) {
// A local user identity should just be itself.
//
return id.substring(AccountExternalId.SCHEME_GERRIT.length());
} else if (k.isScheme(AccountExternalId.SCHEME_MAILTO)) {
// Describe a mailto address as just its email address, which
// is already shown in the email address field.
//
return "";
} else if (k.isScheme(OpenIdUtil.URL_GOOGLE)) {
return OpenIdUtil.C.nameGoogle();
} else if (k.isScheme(OpenIdUtil.URL_YAHOO)) {
return OpenIdUtil.C.nameYahoo();
} else if (k.isScheme(AccountExternalId.LEGACY_GAE)) {
return OpenIdUtil.C.nameGoogle() + " (Imported from Google AppEngine)";
}
return id;
}
}
}

31
src/main/java/com/google/gerrit/client/reviewdb/AccountExternalId.java
View File

@@ -23,6 +23,10 @@ import java.util.Collection;
/** Association of an external account identifier to a local {@link Account}. */
public final class AccountExternalId {
public static final String SCHEME_GERRIT = "gerrit:";
public static final String SCHEME_MAILTO = "mailto:";
public static final String LEGACY_GAE = "Google Account ";
public static class Key extends StringKey<Account.Id> {
private static final long serialVersionUID = 1L;
@@ -74,7 +78,7 @@ public final class AccountExternalId {
continue;
}
if (e.getExternalId().startsWith("mailto:")) {
if (e.getExternalId().startsWith(SCHEME_MAILTO)) {
// Don't ever consider an email address as a "recent login"
//
continue;
@@ -141,28 +145,9 @@ public final class AccountExternalId {
lastUsedOn = new Timestamp(System.currentTimeMillis());
}
public boolean canUserDelete() {
switch (Gerrit.getConfig().getLoginType()) {
case OPENID:
if (getExternalId().startsWith("Google Account ")) {
// Don't allow users to delete legacy google account tokens.
// Administrators will do it when cleaning the database.
//
return false;
}
break;
case HTTP:
if (getExternalId().startsWith("gerrit:")) {
// Don't allow users to delete a gerrit: token, as this is
// a Gerrit generated value for single-sign-on configurations
// not using OpenID.
//
return false;
}
break;
}
return true;
public boolean isScheme(final String scheme) {
final String id = getExternalId();
return id != null && id.startsWith(scheme);
}
public boolean isTrusted() {

3
src/main/java/com/google/gerrit/server/EncryptedContactStore.java
View File

@@ -260,8 +260,7 @@ class EncryptedContactStore implements ContactStore {
}
oistr.append(e.getEmailAddress());
}
if (e.getExternalId() != null && e.getExternalId().length() > 0
&& !e.getExternalId().startsWith("mailto:")) {
if (e.isScheme(AccountExternalId.SCHEME_MAILTO)) {
if (oistr.length() > 0) {
oistr.append(' ');
}

4
src/main/java/com/google/gerrit/server/config/AuthConfig.java
View File

@@ -156,7 +156,7 @@ public class AuthConfig {
}
private boolean isTrusted(final AccountExternalId id) {
if (id.getExternalId().startsWith("Google Account ")) {
if (id.isScheme(AccountExternalId.LEGACY_GAE)) {
// Assume this is a trusted token, its a legacy import from
// a fairly well respected provider and only takes effect if
// the administrator has the import still enabled
@@ -164,7 +164,7 @@ public class AuthConfig {
return isAllowGoogleAccountUpgrade();
}
if (id.getExternalId().startsWith("mailto:")) {
if (id.isScheme(AccountExternalId.SCHEME_MAILTO)) {
// mailto identities are created by sending a unique validation
// token to the address and asking them to come back to the site
// with that token.

2
src/main/java/com/google/gerrit/server/http/GerritCall.java
View File

@@ -176,7 +176,7 @@ public class GerritCall extends ActiveCall {
try {
final ReviewDb db = schema.open();
try {
final String eid = "gerrit:" + user;
final String eid = AccountExternalId.SCHEME_GERRIT + user;
final List<AccountExternalId> matches =
db.accountExternalIds().byExternal(eid).toList();

2
src/main/java/com/google/gerrit/server/openid/OpenIdServiceImpl.java
View File

@@ -548,7 +548,7 @@ class OpenIdServiceImpl implements OpenIdService {
//
final List<AccountExternalId> m = new ArrayList<AccountExternalId>();
for (final AccountExternalId e : extAccess.byEmailAddress(email)) {
if (e.getExternalId().equals("Google Account " + email)) {
if (e.getExternalId().equals(AccountExternalId.LEGACY_GAE + email)) {
m.add(e);
}
}

6
src/main/java/com/google/gerrit/server/rpc/account/AccountSecurityImpl.java
View File

@@ -243,7 +243,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
//
continue;
} else if (e.canUserDelete()) {
} else {
toDelete.add(e);
removed.add(e.getKey());
}
@@ -373,8 +373,8 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
try {
final AccountExternalId id =
new AccountExternalId(new AccountExternalId.Key(me, "mailto:"
+ newEmail));
new AccountExternalId(new AccountExternalId.Key(me,
AccountExternalId.SCHEME_MAILTO + newEmail));
id.setEmailAddress(newEmail);
db.accountExternalIds().insert(Collections.singleton(id));
} catch (OrmDuplicateKeyException e) {