fix: don't allow to use the same apikey twice #2 ATL-5768

Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
hyperledger · Oct 2, 2023 · 13bbeb3 · 13bbeb3
1 parent 58e3c0a
commit 13bbeb3
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 12 deletions.
diff --git a/...rver/src/main/scala/io/iohk/atala/iam/authentication/apikey/ApiKeyAuthenticatorImpl.scala b/...rver/src/main/scala/io/iohk/atala/iam/authentication/apikey/ApiKeyAuthenticatorImpl.scala
@@ -104,7 +104,7 @@ case class ApiKeyAuthenticatorImpl(
         .logError("Failed to compute SHA256 hash")
         .mapError(cause => AuthenticationError.UnexpectedError(cause.getMessage))
       _ <- repository
-        .deleteByEntityIdAndTypeAndSecret(entityId, AuthenticationMethodType.ApiKey, secret)
+        .delete(entityId, AuthenticationMethodType.ApiKey, secret)
         .mapError(are => AuthenticationError.UnexpectedError(are.message))
     } yield ()
   }

diff --git a/...ver/src/main/scala/io/iohk/atala/iam/authentication/apikey/AuthenticationRepository.scala b/...ver/src/main/scala/io/iohk/atala/iam/authentication/apikey/AuthenticationRepository.scala
@@ -52,7 +52,7 @@ trait AuthenticationRepository {
       amt: AuthenticationMethodType
   ): zio.IO[AuthenticationRepositoryError, Unit]
 
-  def deleteByEntityIdAndTypeAndSecret(
+  def delete(
       entityId: UUID,
       amt: AuthenticationMethodType,
       secret: String

diff --git a/...src/main/scala/io/iohk/atala/iam/authentication/apikey/JdbcAuthenticationRepository.scala b/...src/main/scala/io/iohk/atala/iam/authentication/apikey/JdbcAuthenticationRepository.scala
@@ -12,6 +12,7 @@ import java.util.UUID
 case class JdbcAuthenticationRepository(xa: Transactor[Task]) extends AuthenticationRepository {
 
   import AuthenticationRepositorySql.*
+  import AuthenticationRepositoryError.*
   override def insert(
       entityId: UUID,
       amt: AuthenticationMethodType,
@@ -29,18 +30,41 @@ case class JdbcAuthenticationRepository(xa: Transactor[Task]) extends Authentica
         case sqlException: PSQLException
             if sqlException.getMessage
               .contains("ERROR: duplicate key value violates unique constraint \"unique_type_secret_constraint\"") =>
-          AuthenticationRepositoryError.AuthenticationCompromised(entityId, amt, secret)
+          AuthenticationCompromised(entityId, amt, secret)
         case otherSqlException: PSQLException =>
-          AuthenticationRepositoryError.StorageError(otherSqlException)
+          StorageError(otherSqlException)
         case unexpected: Throwable =>
-          AuthenticationRepositoryError.UnexpectedError(unexpected)
+          UnexpectedError(unexpected)
       }
-      .catchSome { case ac @ AuthenticationRepositoryError.AuthenticationCompromised(entityId, amt, secret) =>
-        val failure: IO[AuthenticationRepositoryError, Unit] = ZIO.fail(ac)
-        deleteByEntityIdAndTypeAndSecret(entityId, amt, secret).map(_ => ()) *> failure
+      .catchSome { case AuthenticationCompromised(eId, amt, s) =>
+        ensureThatTheApiKeyIsNotCompromised(eId, amt, s)
       }
   }
 
+  private def ensureThatTheApiKeyIsNotCompromised(
+      entityId: UUID,
+      authenticationMethodType: AuthenticationMethodType,
+      secret: String
+  ): IO[AuthenticationRepositoryError, Unit] = {
+    val ac = AuthenticationCompromised(entityId, authenticationMethodType, secret)
+    val acZIO: IO[AuthenticationRepositoryError, Unit] = ZIO.fail(ac)
+
+    for {
+      authRecordOpt <- findAuthenticationMethodByTypeAndSecret(authenticationMethodType, secret)
+      authRecord <- ZIO.fromOption(authRecordOpt).mapError(_ => ac)
+      compromisedEntityId = authRecord.entityId
+      isTheSameEntityId = authRecord.entityId == entityId
+      isNotDeleted = authRecord.deletedAt.isEmpty
+      result <-
+        if (isTheSameEntityId && isNotDeleted)
+          ZIO.unit
+        else if (isNotDeleted)
+          delete(compromisedEntityId, authenticationMethodType, secret) *> acZIO
+        else
+          acZIO
+    } yield result
+  }
+
   override def getEntityIdByMethodAndSecret(
       amt: AuthenticationMethodType,
       secret: String
@@ -81,7 +105,7 @@ case class JdbcAuthenticationRepository(xa: Transactor[Task]) extends Authentica
       .map(_ => ())
   }
 
-  override def deleteByEntityIdAndTypeAndSecret(
+  override def delete(
       entityId: UUID,
       amt: AuthenticationMethodType,
       secret: String

diff --git a/...test/scala/io/iohk/atala/iam/authentication/apikey/JdbcAuthenticationRepositorySpec.scala b/...test/scala/io/iohk/atala/iam/authentication/apikey/JdbcAuthenticationRepositorySpec.scala
@@ -7,6 +7,7 @@ import zio.test.*
 import zio.test.TestAspect.*
 import zio.test.Assertion.*
 import io.iohk.atala.container.util.MigrationAspects.migrate
+import io.iohk.atala.iam.authentication.apikey.AuthenticationMethodType.ApiKey
 import zio.Runtime.removeDefaultLoggers
 
 object JdbcAuthenticationRepositorySpec extends ZIOSpecDefault, PostgresTestContainerSupport {
@@ -44,20 +45,48 @@ object JdbcAuthenticationRepositorySpec extends ZIOSpecDefault, PostgresTestCont
           assert(notFoundEntityId)(isSubtype[AuthenticationRepositoryError.AuthenticationNotFound](anything))
       }
     },
-    test("insert a similar secret must fail") {
+    test("insert a similar secret for a different tenant must fail") {
+      check(entityIdAndSecretGen <*> entityIdGen) { case (entityId, secret, anotherEntityId) =>
+        for {
+          repository <- ZIO.service[AuthenticationRepository]
+          _ <- repository.insert(entityId, AuthenticationMethodType.ApiKey, secret)
+          insertSameSecret <- repository.insert(anotherEntityId, AuthenticationMethodType.ApiKey, secret).flip
+          authenticationMethod <- repository
+            .findAuthenticationMethodByTypeAndSecret(AuthenticationMethodType.ApiKey, secret)
+        } yield assert(insertSameSecret)(
+          isSubtype[AuthenticationRepositoryError.AuthenticationCompromised](anything)
+        ) &&
+          assert(authenticationMethod)(isSome(anything)) &&
+          assert(authenticationMethod.flatMap(_.deletedAt))(isSome(anything))
+      }
+    },
+    test("insert a similar secret for the same tenant must succeed") {
       check(entityIdAndSecretGen) { case (entityId, secret) =>
         for {
           repository <- ZIO.service[AuthenticationRepository]
           _ <- repository.insert(entityId, AuthenticationMethodType.ApiKey, secret)
-          insertSameSecret <- repository.insert(entityId, AuthenticationMethodType.ApiKey, secret).flip
+          _ <- repository.insert(entityId, AuthenticationMethodType.ApiKey, secret)
           authenticationMethod <- repository
             .findAuthenticationMethodByTypeAndSecret(AuthenticationMethodType.ApiKey, secret)
+        } yield assert(authenticationMethod)(isSome(anything)) &&
+          assert(authenticationMethod.flatMap(_.deletedAt))(isNone)
+      }
+    },
+    test("insert a similar secret after deletion for the same tenant must fail") {
+      check(entityIdAndSecretGen) { case (entityId, secret) =>
+        for {
+          repository <- ZIO.service[AuthenticationRepository]
+          _ <- repository.insert(entityId, ApiKey, secret)
+          _ <- repository.delete(entityId, ApiKey, secret)
+          insertSameSecret <- repository.insert(entityId, ApiKey, secret).flip
+          authenticationMethod <- repository
+            .findAuthenticationMethodByTypeAndSecret(ApiKey, secret)
         } yield assert(insertSameSecret)(
           isSubtype[AuthenticationRepositoryError.AuthenticationCompromised](anything)
         ) &&
           assert(authenticationMethod)(isSome(anything)) &&
           assert(authenticationMethod.flatMap(_.deletedAt))(isSome(anything))
       }
     }
-  ) @@ samples(1) @@ nondeterministic
+  ) @@ samples(10) @@ nondeterministic
 }