Merge pull request #181 from AuthGuard/fix/inactive-identifiers

Prevent authentication using inactive identifiers
AuthGuard · Oct 8, 2021 · 1a837f2 · 1a837f2
2 parents 87611fb + 4972af9
commit 1a837f2
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 10 deletions.
diff --git a/basic-auth/src/main/java/com/nexblocks/authguard/basic/BasicAuthProvider.java b/basic-auth/src/main/java/com/nexblocks/authguard/basic/BasicAuthProvider.java
@@ -7,10 +7,7 @@
 import com.nexblocks.authguard.service.exceptions.ServiceAuthorizationException;
 import com.nexblocks.authguard.service.exceptions.ServiceException;
 import com.nexblocks.authguard.service.exceptions.codes.ErrorCode;
-import com.nexblocks.authguard.service.model.AccountBO;
-import com.nexblocks.authguard.service.model.AuthRequestBO;
-import com.nexblocks.authguard.service.model.CredentialsBO;
-import com.nexblocks.authguard.service.model.EntityType;
+import com.nexblocks.authguard.service.model.*;
 import com.google.inject.Inject;
 import io.vavr.control.Either;
 import org.slf4j.Logger;
@@ -86,7 +83,14 @@ private Either<Exception, AccountBO> handleBasicAuthenticationNoPassword(final S
     private Either<Exception, AccountBO> verifyCredentialsAndGetAccount(final String username, final String password) {
         final Optional<CredentialsBO> credentials = credentialsService.getByUsernameUnsafe(username);
 
+        // TODO replace this with Either mapping
         if (credentials.isPresent()) {
+            final Optional<Exception> validationError = checkIdentifier(credentials.get(), username);
+
+            if (validationError.isPresent()) {
+                return Either.left(validationError.get());
+            }
+
             if (securePassword.verify(password, credentials.get().getHashedPassword())) {
                 return getAccountById(credentials.get().getAccountId());
             } else {
@@ -103,13 +107,38 @@ private Either<Exception, AccountBO> verifyCredentialsAndGetAccount(final String
         final Optional<CredentialsBO> credentials = credentialsService.getByUsernameUnsafe(username);
 
         if (credentials.isPresent()) {
+            final Optional<Exception> validationError = checkIdentifier(credentials.get(), username);
+
+            if (validationError.isPresent()) {
+                return Either.left(validationError.get());
+            }
+
             return getAccountById(credentials.get().getAccountId());
         } else {
             return Either.left(new ServiceAuthorizationException(ErrorCode.CREDENTIALS_DOES_NOT_EXIST,
                     "Identifier " + username + " does not exist"));
         }
     }
 
+    private Optional<Exception> checkIdentifier(final CredentialsBO credentials,
+                                                final String identifier) {
+        final Optional<UserIdentifierBO> matchedIdentifier = credentials.getIdentifiers()
+                .stream()
+                .filter(existing -> identifier.equals(existing.getIdentifier()))
+                .findFirst();
+
+        if (matchedIdentifier.isEmpty()) {
+            return Optional.of(new IllegalStateException("No identifier matched but credentials were returned"));
+        }
+
+        if (!matchedIdentifier.get().isActive()) {
+            return Optional.of(new ServiceAuthorizationException(ErrorCode.INACTIVE_IDENTIFIER,
+                    "Identifier is not active", EntityType.ACCOUNT, credentials.getAccountId()));
+        }
+
+        return Optional.empty();
+    }
+
     private Either<Exception, AccountBO> getAccountById(final String accountId) {
         final Optional<AccountBO> account = accountsService.getById(accountId);
 

diff --git a/basic-auth/src/test/java/com/nexblocks/authguard/basic/BasicAuthProviderTest.java b/basic-auth/src/test/java/com/nexblocks/authguard/basic/BasicAuthProviderTest.java
@@ -64,6 +64,7 @@ void authenticate() {
                 .withIdentifiers(UserIdentifierBO.builder()
                         .identifier(username)
                         .type(UserIdentifier.Type.USERNAME)
+                        .active(true)
                         .build());
         final HashedPasswordBO hashedPasswordBO = HashedPasswordBO.builder()
                 .password(credentials.getHashedPassword().getPassword())
@@ -79,6 +80,27 @@ void authenticate() {
         assertThat(result.get()).isEqualTo(account);
     }
 
+    @Test
+    void authenticateInactiveIdentifier() {
+        final String username = "username";
+        final String password = "password";
+        final String authorization = Base64.getEncoder().encodeToString((username + ":" + password).getBytes());
+
+        final CredentialsBO credentials = RANDOM.nextObject(CredentialsBO.class)
+                .withIdentifiers(UserIdentifierBO.builder()
+                        .identifier(username)
+                        .type(UserIdentifier.Type.USERNAME)
+                        .active(false)
+                        .build());
+
+        Mockito.when(credentialsService.getByUsernameUnsafe(username)).thenReturn(Optional.of(credentials));
+
+        final Either<Exception, AccountBO> result = basicAuth.authenticateAndGetAccount(authorization);
+
+        assertThat(result.isLeft()).isTrue();
+        assertThat(result.getLeft()).isInstanceOf(ServiceAuthorizationException.class);
+    }
+
     @Test
     void authenticateNotFound() {
         final String username = "username";

diff --git a/pom.xml b/pom.xml
@@ -50,17 +50,17 @@
         <jackson.version>2.12.3</jackson.version>
         <immutables.version>2.7.4</immutables.version>
         <jetbrains-annotations.version>13.0</jetbrains-annotations.version>
-        <auth0-jwt.version>3.12.0</auth0-jwt.version>
+        <auth0-jwt.version>3.16.0</auth0-jwt.version>
         <apache-commons.version>3.12.0</apache-commons.version>
         <commons-validator.version>1.7</commons-validator.version>
         <commons-cli.version>1.4</commons-cli.version>
         <mapstruct.version>1.3.0.Final</mapstruct.version>
-        <bouncy-castle.version>1.62</bouncy-castle.version>
+        <bouncy-castle.version>1.68</bouncy-castle.version>
         <reflections.version>0.9.12</reflections.version>
-        <guice.version>4.2.3</guice.version>
+        <guice.version>5.0.0</guice.version>
         <rxjava.version>3.0.12</rxjava.version>
-        <vertx.version>3.9.4</vertx.version>
-        <okhttp.version>4.9.0</okhttp.version>
+        <vertx.version>3.9.7</vertx.version>
+        <okhttp.version>4.9.1</okhttp.version>
         <unbounded-ldap.version>5.1.4</unbounded-ldap.version>
         <javax-persistence.version>2.2</javax-persistence.version>
         <vavr.version>0.9.0</vavr.version>

diff --git a/service-api/src/main/java/com/nexblocks/authguard/service/exceptions/codes/ErrorCode.java b/service-api/src/main/java/com/nexblocks/authguard/service/exceptions/codes/ErrorCode.java
@@ -33,6 +33,7 @@ public enum ErrorCode {
     TOKEN_EXPIRED_OR_DOES_NOT_EXIST("AT.025"),
     TOKEN_GENERATION_FAILED("AT.031"),
     ACCOUNT_IS_LOCKED("AT.032"),
+    INACTIVE_IDENTIFIER("AT.033"),
     GENERIC_AUTH_FAILURE("AT.039"),
 
     UNSUPPORTED_JWT_ALGORITHM("JT.021"),

diff --git a/service-api/src/main/java/com/nexblocks/authguard/service/model/UserIdentifier.java b/service-api/src/main/java/com/nexblocks/authguard/service/model/UserIdentifier.java
@@ -8,7 +8,11 @@ public interface UserIdentifier {
     Long getId(); // only useful for relational DBs
     Type getType();
     String getIdentifier();
-    boolean isActive();
+
+    @Value.Default
+    default boolean isActive() {
+        return true;
+    }
 
     enum Type {
         USERNAME,