From a93c8782cf5ae5709af5bef41f5eba7fc8da83d0 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Wed, 7 Aug 2019 11:13:26 +0200 Subject: [PATCH] Move #checkPassword to separate class Change-Id: I04ad756c8332de070e4f0c4deb001c987375d761 --- .../gerrit/httpd/ProjectBasicAuthFilter.java | 5 +- .../gerrit/server/account/AccountState.java | 30 ----------- .../account/externalids/PasswordVerifier.java | 54 +++++++++++++++++++ .../server/auth/InternalAuthBackend.java | 3 +- 4 files changed, 59 insertions(+), 33 deletions(-) create mode 100644 java/com/google/gerrit/server/account/externalids/PasswordVerifier.java diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java index e3ab70d831..aa38c27489 100644 --- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java +++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.AuthResult; import com.google.gerrit.server.account.AuthenticationFailedException; +import com.google.gerrit.server.account.externalids.PasswordVerifier; import com.google.gerrit.server.auth.NoSuchUserException; import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; @@ -140,7 +141,7 @@ class ProjectBasicAuthFilter implements Filter { GitBasicAuthPolicy gitBasicAuthPolicy = authConfig.getGitBasicAuthPolicy(); if (gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP || gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP_LDAP) { - if (who.checkPassword(password, username)) { + if (PasswordVerifier.checkPassword(who.getExternalIds(), username, password)) { return succeedAuthentication(who); } } @@ -157,7 +158,7 @@ class ProjectBasicAuthFilter implements Filter { setUserIdentified(whoAuthResult.getAccountId()); return true; } catch (NoSuchUserException e) { - if (who.checkPassword(password, username)) { + if (PasswordVerifier.checkPassword(who.getExternalIds(), username, password)) { return succeedAuthentication(who); } logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req)); diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 8555166043..6eb6ca101d 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -14,13 +14,9 @@ package com.google.gerrit.server.account; -import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; - import com.google.common.base.MoreObjects; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.client.EditPreferencesInfo; @@ -34,7 +30,6 @@ import com.google.gerrit.server.account.externalids.ExternalIds; import java.io.IOException; import java.util.Collection; import java.util.Optional; -import org.apache.commons.codec.DecoderException; import org.eclipse.jgit.lib.ObjectId; /** @@ -45,8 +40,6 @@ import org.eclipse.jgit.lib.ObjectId; * account cache (see {@link AccountCache#get(Account.Id)}). */ public class AccountState { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - /** * Creates an AccountState from the given account config. * @@ -175,29 +168,6 @@ public class AccountState { return userName; } - public boolean checkPassword(@Nullable String password, String username) { - if (password == null) { - return false; - } - for (ExternalId id : getExternalIds()) { - // Only process the "username:$USER" entry, which is unique. - if (!id.isScheme(SCHEME_USERNAME) || !username.equals(id.key().id())) { - continue; - } - - String hashedStr = id.password(); - if (!Strings.isNullOrEmpty(hashedStr)) { - try { - return HashedPassword.decode(hashedStr).checkPassword(password); - } catch (DecoderException e) { - logger.atSevere().log("DecoderException for user %s: %s ", username, e.getMessage()); - return false; - } - } - } - return false; - } - /** The external identities that identify the account holder. */ public ImmutableSet getExternalIds() { return externalIds; diff --git a/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java new file mode 100644 index 0000000000..e4bf27b740 --- /dev/null +++ b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java @@ -0,0 +1,54 @@ +// Copyright (C) 2019 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.externalids; + +import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; + +import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.server.account.HashedPassword; +import java.util.Collection; +import org.apache.commons.codec.DecoderException; + +/** Checks if a given username and password match a user's external IDs. */ +public class PasswordVerifier { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + /** Returns {@code true} if there is an external ID matching both the username and password. */ + public static boolean checkPassword( + Collection externalIds, String username, @Nullable String password) { + if (password == null) { + return false; + } + for (ExternalId id : externalIds) { + // Only process the "username:$USER" entry, which is unique. + if (!id.isScheme(SCHEME_USERNAME) || !username.equals(id.key().id())) { + continue; + } + + String hashedStr = id.password(); + if (!Strings.isNullOrEmpty(hashedStr)) { + try { + return HashedPassword.decode(hashedStr).checkPassword(password); + } catch (DecoderException e) { + logger.atSevere().log("DecoderException for user %s: %s ", username, e.getMessage()); + return false; + } + } + } + return false; + } +} diff --git a/java/com/google/gerrit/server/auth/InternalAuthBackend.java b/java/com/google/gerrit/server/auth/InternalAuthBackend.java index c06c66bd8e..2821bf6aa3 100644 --- a/java/com/google/gerrit/server/auth/InternalAuthBackend.java +++ b/java/com/google/gerrit/server/auth/InternalAuthBackend.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.auth; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.account.externalids.PasswordVerifier; import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -62,7 +63,7 @@ public class InternalAuthBackend implements AuthBackend { + ": account inactive or not provisioned in Gerrit"); } - if (!who.checkPassword(req.getPassword().get(), username)) { + if (!PasswordVerifier.checkPassword(who.getExternalIds(), username, req.getPassword().get())) { throw new InvalidCredentialsException(); } return new AuthUser(AuthUser.UUID.create(username), username);