Permit Realm to supply email address verification on the fly
A Realm might have more data about a user's account in the backend database. If email address information is available, the realm could look this up on the fly to verify incoming commits. Add a new hasEmailAddress() method to check individual emails and cache results inside of the IdentifiedUser object for the duration of the current request/operation scope. Also add a method to lookup as many possible email variations as possible, for reporting in error messages or GET /emails API. Change-Id: I13fec7eb00b1bd6ddcb74323ba7205ee1b2ee42f
This commit is contained in:
parent
a33543d7ea
commit
773f8f1469
|
@ -26,8 +26,10 @@ import com.google.gerrit.server.IdentifiedUser;
|
|||
import com.google.gerrit.server.account.AccountByEmailCacheImpl;
|
||||
import com.google.gerrit.server.account.AccountCacheImpl;
|
||||
import com.google.gerrit.server.account.CapabilityControl;
|
||||
import com.google.gerrit.server.account.FakeRealm;
|
||||
import com.google.gerrit.server.account.GroupCacheImpl;
|
||||
import com.google.gerrit.server.account.GroupIncludeCacheImpl;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.cache.CacheRemovalListener;
|
||||
import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
|
||||
import com.google.gerrit.server.change.ChangeKindCacheImpl;
|
||||
|
@ -96,6 +98,7 @@ public class BatchProgramModule extends FactoryModule {
|
|||
.toProvider(CanonicalWebUrlProvider.class);
|
||||
bind(Boolean.class).annotatedWith(DisableReverseDnsLookup.class)
|
||||
.toProvider(DisableReverseDnsLookupProvider.class).in(SINGLETON);
|
||||
bind(Realm.class).to(FakeRealm.class);
|
||||
bind(IdentifiedUser.class)
|
||||
.toProvider(Providers.<IdentifiedUser> of(null));
|
||||
bind(ReplacePatchSetSender.Factory.class).toProvider(
|
||||
|
|
|
@ -30,6 +30,7 @@ import com.google.gerrit.server.account.CapabilityControl;
|
|||
import com.google.gerrit.server.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.GroupMembership;
|
||||
import com.google.gerrit.server.account.ListGroupMembership;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.config.AnonymousCowardName;
|
||||
import com.google.gerrit.server.config.AuthConfig;
|
||||
import com.google.gerrit.server.config.CanonicalWebUrl;
|
||||
|
@ -67,6 +68,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
public static class GenericFactory {
|
||||
private final CapabilityControl.Factory capabilityControlFactory;
|
||||
private final AuthConfig authConfig;
|
||||
private final Realm realm;
|
||||
private final String anonymousCowardName;
|
||||
private final Provider<String> canonicalUrl;
|
||||
private final AccountCache accountCache;
|
||||
|
@ -77,6 +79,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
public GenericFactory(
|
||||
@Nullable CapabilityControl.Factory capabilityControlFactory,
|
||||
AuthConfig authConfig,
|
||||
Realm realm,
|
||||
@AnonymousCowardName String anonymousCowardName,
|
||||
@CanonicalWebUrl Provider<String> canonicalUrl,
|
||||
@DisableReverseDnsLookup Boolean disableReverseDnsLookup,
|
||||
|
@ -84,6 +87,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
GroupBackend groupBackend) {
|
||||
this.capabilityControlFactory = capabilityControlFactory;
|
||||
this.authConfig = authConfig;
|
||||
this.realm = realm;
|
||||
this.anonymousCowardName = anonymousCowardName;
|
||||
this.canonicalUrl = canonicalUrl;
|
||||
this.accountCache = accountCache;
|
||||
|
@ -96,20 +100,20 @@ public class IdentifiedUser extends CurrentUser {
|
|||
}
|
||||
|
||||
public IdentifiedUser create(Provider<ReviewDb> db, Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig,
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
|
||||
anonymousCowardName, canonicalUrl, accountCache, groupBackend,
|
||||
disableReverseDnsLookup, null, db, id, null);
|
||||
}
|
||||
|
||||
public IdentifiedUser create(SocketAddress remotePeer, Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig,
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
|
||||
anonymousCowardName, canonicalUrl, accountCache, groupBackend,
|
||||
disableReverseDnsLookup, Providers.of(remotePeer), null, id, null);
|
||||
}
|
||||
|
||||
public CurrentUser runAs(SocketAddress remotePeer, Account.Id id,
|
||||
@Nullable CurrentUser caller) {
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig,
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
|
||||
anonymousCowardName, canonicalUrl, accountCache, groupBackend,
|
||||
disableReverseDnsLookup, Providers.of(remotePeer), null, id, caller);
|
||||
}
|
||||
|
@ -125,6 +129,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
public static class RequestFactory {
|
||||
private final CapabilityControl.Factory capabilityControlFactory;
|
||||
private final AuthConfig authConfig;
|
||||
private final Realm realm;
|
||||
private final String anonymousCowardName;
|
||||
private final Provider<String> canonicalUrl;
|
||||
private final AccountCache accountCache;
|
||||
|
@ -138,6 +143,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
RequestFactory(
|
||||
CapabilityControl.Factory capabilityControlFactory,
|
||||
final AuthConfig authConfig,
|
||||
Realm realm,
|
||||
final @AnonymousCowardName String anonymousCowardName,
|
||||
final @CanonicalWebUrl Provider<String> canonicalUrl,
|
||||
final AccountCache accountCache,
|
||||
|
@ -148,6 +154,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
final Provider<ReviewDb> dbProvider) {
|
||||
this.capabilityControlFactory = capabilityControlFactory;
|
||||
this.authConfig = authConfig;
|
||||
this.realm = realm;
|
||||
this.anonymousCowardName = anonymousCowardName;
|
||||
this.canonicalUrl = canonicalUrl;
|
||||
this.accountCache = accountCache;
|
||||
|
@ -159,13 +166,13 @@ public class IdentifiedUser extends CurrentUser {
|
|||
}
|
||||
|
||||
public IdentifiedUser create(Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig,
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
|
||||
anonymousCowardName, canonicalUrl, accountCache, groupBackend,
|
||||
disableReverseDnsLookup, remotePeerProvider, dbProvider, id, null);
|
||||
}
|
||||
|
||||
public IdentifiedUser runAs(Account.Id id, CurrentUser caller) {
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig,
|
||||
return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
|
||||
anonymousCowardName, canonicalUrl, accountCache, groupBackend,
|
||||
disableReverseDnsLookup, remotePeerProvider, dbProvider, id, caller);
|
||||
}
|
||||
|
@ -182,9 +189,11 @@ public class IdentifiedUser extends CurrentUser {
|
|||
private final Provider<String> canonicalUrl;
|
||||
private final AccountCache accountCache;
|
||||
private final AuthConfig authConfig;
|
||||
private final Realm realm;
|
||||
private final GroupBackend groupBackend;
|
||||
private final String anonymousCowardName;
|
||||
private final Boolean disableReverseDnsLookup;
|
||||
private final Set<String> validEmails = Sets.newHashSetWithExpectedSize(4);
|
||||
|
||||
@Nullable
|
||||
private final Provider<SocketAddress> remotePeerProvider;
|
||||
|
@ -195,17 +204,18 @@ public class IdentifiedUser extends CurrentUser {
|
|||
private final Account.Id accountId;
|
||||
|
||||
private AccountState state;
|
||||
private Set<String> emailAddresses;
|
||||
private boolean loadedAllEmails;
|
||||
private Set<String> invalidEmails;
|
||||
private GroupMembership effectiveGroups;
|
||||
private Set<Change.Id> starredChanges;
|
||||
private ResultSet<StarredChange> starredQuery;
|
||||
private Collection<AccountProjectWatch> notificationFilters;
|
||||
private CurrentUser realUser;
|
||||
|
||||
|
||||
private IdentifiedUser(
|
||||
CapabilityControl.Factory capabilityControlFactory,
|
||||
final AuthConfig authConfig,
|
||||
Realm realm,
|
||||
final String anonymousCowardName,
|
||||
final Provider<String> canonicalUrl,
|
||||
final AccountCache accountCache,
|
||||
|
@ -220,6 +230,7 @@ public class IdentifiedUser extends CurrentUser {
|
|||
this.accountCache = accountCache;
|
||||
this.groupBackend = groupBackend;
|
||||
this.authConfig = authConfig;
|
||||
this.realm = realm;
|
||||
this.anonymousCowardName = anonymousCowardName;
|
||||
this.disableReverseDnsLookup = disableReverseDnsLookup;
|
||||
this.remotePeerProvider = remotePeerProvider;
|
||||
|
@ -271,14 +282,26 @@ public class IdentifiedUser extends CurrentUser {
|
|||
}
|
||||
|
||||
public boolean hasEmailAddress(String email) {
|
||||
return getEmailAddresses().contains(email);
|
||||
if (validEmails.contains(email)) {
|
||||
return true;
|
||||
} else if (invalidEmails != null && invalidEmails.contains(email)) {
|
||||
return false;
|
||||
} else if (realm.hasEmailAddress(this, email)) {
|
||||
validEmails.add(email);
|
||||
return true;
|
||||
} else if (invalidEmails == null) {
|
||||
invalidEmails = Sets.newHashSetWithExpectedSize(4);
|
||||
}
|
||||
invalidEmails.add(email);
|
||||
return false;
|
||||
}
|
||||
|
||||
public Set<String> getEmailAddresses() {
|
||||
if (emailAddresses == null) {
|
||||
emailAddresses = state().getEmailAddresses();
|
||||
if (!loadedAllEmails) {
|
||||
validEmails.addAll(realm.getEmailAddresses(this));
|
||||
loadedAllEmails = true;
|
||||
}
|
||||
return emailAddresses;
|
||||
return validEmails;
|
||||
}
|
||||
|
||||
public String getName() {
|
||||
|
|
|
@ -0,0 +1,49 @@
|
|||
// Copyright (C) 2014 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.account;
|
||||
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.common.collect.Sets;
|
||||
import com.google.gerrit.reviewdb.client.AccountExternalId;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
|
||||
import java.util.Collection;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
|
||||
/** Basic implementation of {@link Realm}. */
|
||||
public abstract class AbstractRealm implements Realm {
|
||||
@Override
|
||||
public boolean hasEmailAddress(IdentifiedUser user, String email) {
|
||||
for (AccountExternalId ext : user.state().getExternalIds()) {
|
||||
if (Objects.equals(ext.getEmailAddress(), email)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<String> getEmailAddresses(IdentifiedUser user) {
|
||||
Collection<AccountExternalId> ids = user.state().getExternalIds();
|
||||
Set<String> emails = Sets.newHashSetWithExpectedSize(ids.size());
|
||||
for (AccountExternalId ext : ids) {
|
||||
if (!Strings.isNullOrEmpty(ext.getEmailAddress())) {
|
||||
emails.add(ext.getEmailAddress());
|
||||
}
|
||||
}
|
||||
return emails;
|
||||
}
|
||||
}
|
|
@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
|
|||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
|
||||
import java.util.Collection;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
public class AccountState {
|
||||
|
@ -64,25 +63,6 @@ public class AccountState {
|
|||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* All email addresses registered to this account.
|
||||
* <p>
|
||||
* Gerrit is "reasonably certain" that the returned email addresses actually
|
||||
* belong to the user of the account. Some emails may have been obtained from
|
||||
* the authentication provider, which in the case of OpenID may be trusting
|
||||
* the provider to have validated the address. Other emails may have been
|
||||
* validated by Gerrit directly.
|
||||
*/
|
||||
public Set<String> getEmailAddresses() {
|
||||
final Set<String> emails = new HashSet<>();
|
||||
for (final AccountExternalId e : externalIds) {
|
||||
if (e.getEmailAddress() != null && !e.getEmailAddress().isEmpty()) {
|
||||
emails.add(e.getEmailAddress());
|
||||
}
|
||||
}
|
||||
return emails;
|
||||
}
|
||||
|
||||
/** The external identities that identify the account holder. */
|
||||
public Collection<AccountExternalId> getExternalIds() {
|
||||
return externalIds;
|
||||
|
|
|
@ -25,7 +25,7 @@ import com.google.inject.Singleton;
|
|||
import java.util.Set;
|
||||
|
||||
@Singleton
|
||||
public class DefaultRealm implements Realm {
|
||||
public class DefaultRealm extends AbstractRealm {
|
||||
private final EmailExpander emailExpander;
|
||||
private final AccountByEmailCache byEmail;
|
||||
private final AuthConfig authConfig;
|
||||
|
|
|
@ -12,16 +12,14 @@
|
|||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.testutil;
|
||||
package com.google.gerrit.server.account;
|
||||
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.Account.FieldName;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.account.AuthRequest;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
|
||||
/** Fake implementation of {@link Realm} for testing. */
|
||||
public class FakeRealm implements Realm {
|
||||
/** Fake implementation of {@link Realm} that does not communicate. */
|
||||
public class FakeRealm extends AbstractRealm {
|
||||
@Override
|
||||
public boolean allowsEdit(FieldName field) {
|
||||
return false;
|
|
@ -16,6 +16,9 @@ package com.google.gerrit.server.account;
|
|||
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
|
||||
import java.util.Set;
|
||||
|
||||
public interface Realm {
|
||||
/** Can the end-user modify this field of their own account? */
|
||||
|
@ -31,6 +34,12 @@ public interface Realm {
|
|||
|
||||
public void onCreateAccount(AuthRequest who, Account account);
|
||||
|
||||
/** @return true if the user has the given email address. */
|
||||
public boolean hasEmailAddress(IdentifiedUser who, String email);
|
||||
|
||||
/** @return all known email addresses for the identified user. */
|
||||
public Set<String> getEmailAddresses(IdentifiedUser who);
|
||||
|
||||
/**
|
||||
* Locate an account whose local username is the given account name.
|
||||
* <p>
|
||||
|
|
|
@ -26,10 +26,10 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
|
|||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.AuthType;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.account.AbstractRealm;
|
||||
import com.google.gerrit.server.account.AccountException;
|
||||
import com.google.gerrit.server.account.AuthRequest;
|
||||
import com.google.gerrit.server.account.EmailExpander;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.auth.AuthenticationUnavailableException;
|
||||
import com.google.gerrit.server.config.AuthConfig;
|
||||
import com.google.gerrit.server.config.GerritServerConfig;
|
||||
|
@ -58,7 +58,7 @@ import javax.naming.directory.DirContext;
|
|||
import javax.security.auth.login.LoginException;
|
||||
|
||||
@Singleton
|
||||
public class LdapRealm implements Realm {
|
||||
public class LdapRealm extends AbstractRealm {
|
||||
static final Logger log = LoggerFactory.getLogger(LdapRealm.class);
|
||||
static final String LDAP = "com.sun.jndi.ldap.LdapCtxFactory";
|
||||
static final String USERNAME = "username";
|
||||
|
|
|
@ -14,11 +14,11 @@
|
|||
|
||||
package com.google.gerrit.rules;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.gerrit.common.data.Permission.LABEL;
|
||||
import static com.google.gerrit.server.project.Util.allow;
|
||||
import static com.google.gerrit.server.project.Util.category;
|
||||
import static com.google.gerrit.server.project.Util.value;
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import com.google.gerrit.common.TimeUtil;
|
||||
|
|
|
@ -52,7 +52,9 @@ import com.google.gerrit.server.PatchLineCommentsUtil;
|
|||
import com.google.gerrit.server.account.AccountCache;
|
||||
import com.google.gerrit.server.account.AccountLoader;
|
||||
import com.google.gerrit.server.account.CapabilityControl;
|
||||
import com.google.gerrit.server.account.FakeRealm;
|
||||
import com.google.gerrit.server.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.config.AllUsersNameProvider;
|
||||
import com.google.gerrit.server.config.AnonymousCowardName;
|
||||
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
|
||||
|
@ -171,6 +173,7 @@ public class CommentsTest {
|
|||
bind(draftViewsType).toInstance(draftViews);
|
||||
bind(AccountLoader.Factory.class).toInstance(alf);
|
||||
bind(ReviewDb.class).toInstance(db);
|
||||
bind(Realm.class).to(FakeRealm.class);
|
||||
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
|
||||
bind(ProjectCache.class).toProvider(Providers.<ProjectCache> of(null));
|
||||
install(new GitModule());
|
||||
|
|
|
@ -33,6 +33,7 @@ import com.google.gerrit.server.GerritPersonIdent;
|
|||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.AccountCache;
|
||||
import com.google.gerrit.server.account.CapabilityControl;
|
||||
import com.google.gerrit.server.account.FakeRealm;
|
||||
import com.google.gerrit.server.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.config.AllUsersNameProvider;
|
||||
|
@ -48,7 +49,6 @@ import com.google.gerrit.server.git.GitRepositoryManager;
|
|||
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||
import com.google.gerrit.server.project.ProjectCache;
|
||||
import com.google.gerrit.testutil.FakeAccountCache;
|
||||
import com.google.gerrit.testutil.FakeRealm;
|
||||
import com.google.gerrit.testutil.InMemoryRepositoryManager;
|
||||
import com.google.gerrit.testutil.TestChanges;
|
||||
import com.google.gwtorm.client.KeyUtil;
|
||||
|
|
|
@ -35,9 +35,11 @@ import com.google.gerrit.rules.RulesCache;
|
|||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.account.AccountCache;
|
||||
import com.google.gerrit.server.account.CapabilityControl;
|
||||
import com.google.gerrit.server.account.FakeRealm;
|
||||
import com.google.gerrit.server.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.GroupMembership;
|
||||
import com.google.gerrit.server.account.ListGroupMembership;
|
||||
import com.google.gerrit.server.account.Realm;
|
||||
import com.google.gerrit.server.change.ChangeKindCache;
|
||||
import com.google.gerrit.server.change.ChangeKindCacheImpl;
|
||||
import com.google.gerrit.server.change.MergeabilityCache;
|
||||
|
@ -265,6 +267,7 @@ public class Util {
|
|||
bind(ReviewDb.class).toProvider(nullProvider);
|
||||
bind(GitRepositoryManager.class).toInstance(repoManager);
|
||||
bind(PatchListCache.class).toProvider(nullProvider);
|
||||
bind(Realm.class).to(FakeRealm.class);
|
||||
|
||||
factory(CapabilityControl.Factory.class);
|
||||
factory(ChangeControl.AssistedFactory.class);
|
||||
|
|
Loading…
Reference in New Issue