Improve token error handling + integration tests (BlueBrain#3709)

Co-authored-by: Simon Dumas <[email protected]>
bilalesi · Feb 24, 2023 · a2dc774 · a2dc774
1 parent e6d07ea
commit a2dc774
Show file tree

Hide file tree

Showing 12 changed files with 102 additions and 26 deletions.
diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImpl.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImpl.scala
@@ -8,7 +8,7 @@ import ch.epfl.bluebrain.nexus.delta.kernel.kamon.KamonMetricComponent
 import ch.epfl.bluebrain.nexus.delta.sdk.cache.{CacheConfig, KeyValueStore}
 import ch.epfl.bluebrain.nexus.delta.sdk.http.HttpClientError
 import ch.epfl.bluebrain.nexus.delta.sdk.http.HttpClientError.HttpClientStatusError
-import ch.epfl.bluebrain.nexus.delta.sdk.identities.IdentitiesImpl.{extractGroups, GroupsCache}
+import ch.epfl.bluebrain.nexus.delta.sdk.identities.IdentitiesImpl.{extractGroups, logger, GroupsCache}
 import ch.epfl.bluebrain.nexus.delta.sdk.identities.model.TokenRejection.{GetGroupsFromOidcError, InvalidAccessToken, UnknownAccessTokenIssuer}
 import ch.epfl.bluebrain.nexus.delta.sdk.identities.model.{AuthToken, Caller, TokenRejection}
 import ch.epfl.bluebrain.nexus.delta.sdk.realms.model.Realm
@@ -18,7 +18,6 @@ import com.nimbusds.jose.JWSAlgorithm
 import com.nimbusds.jose.jwk.source.ImmutableJWKSet
 import com.nimbusds.jose.jwk.{JWK, JWKSet}
 import com.nimbusds.jose.proc.{JWSVerificationKeySelector, SecurityContext}
-import com.nimbusds.jwt.SignedJWT
 import com.nimbusds.jwt.proc.{DefaultJWTClaimsVerifier, DefaultJWTProcessor}
 import com.typesafe.scalalogging.Logger
 import io.circe.{Decoder, HCursor, Json}
@@ -43,7 +42,7 @@ class IdentitiesImpl private (
       new JWKSet(jwks.toList.asJava)
     }
 
-    def validate(audiences: Option[NonEmptySet[String]], jwt: SignedJWT, keySet: JWKSet) = {
+    def validate(audiences: Option[NonEmptySet[String]], token: ParsedToken, keySet: JWKSet) = {
       val proc        = new DefaultJWTProcessor[SecurityContext]
       val keySelector = new JWSVerificationKeySelector(JWSAlgorithm.RS256, new ImmutableJWKSet[SecurityContext](keySet))
       proc.setJWSKeySelector(keySelector)
@@ -52,8 +51,8 @@ class IdentitiesImpl private (
       }
       IO.fromEither(
         Either
-          .catchNonFatal(proc.process(jwt, null))
-          .leftMap(err => InvalidAccessToken(Option(err.getMessage).filter(_.trim.nonEmpty)))
+          .catchNonFatal(proc.process(token.jwtToken, null))
+          .leftMap(err => InvalidAccessToken(token.subject, token.issuer, err.getMessage))
       )
     }
 
@@ -76,13 +75,15 @@ class IdentitiesImpl private (
       parsedToken       <- IO.fromEither(ParsedToken.fromToken(token))
       activeRealmOption <- findActiveRealm(parsedToken.issuer)
       activeRealm       <- IO.fromOption(activeRealmOption, UnknownAccessTokenIssuer)
-      _                 <- validate(activeRealm.acceptedAudiences, parsedToken.jwtToken, realmKeyset(activeRealm))
+      _                 <- validate(activeRealm.acceptedAudiences, parsedToken, realmKeyset(activeRealm))
       groups            <- fetchGroups(parsedToken, activeRealm)
     } yield {
       val user = User(parsedToken.subject, activeRealm.label)
       Caller(user, groups ++ Set(Anonymous, user, Authenticated(activeRealm.label)))
     }
     result.span("exchangeToken")
+  }.tapError { rejection =>
+    UIO.delay(logger.error(s"Extracting and validating the caller failed for the reason: $rejection"))
   }
 }
 
@@ -109,11 +110,11 @@ object IdentitiesImpl {
           val message =
             s"A provided client token was rejected by the OIDC provider for user '${token.subject}' of realm '${token.issuer}', reason: '${e.reason}'"
           UIO.delay(logger.error(message, e)) >>
-            IO.raiseError(InvalidAccessToken(Option.when(e.message.trim.nonEmpty)(e.message)))
+            IO.raiseError(InvalidAccessToken(token.subject, token.issuer, e.getMessage))
         case e                                                                                                 =>
           val message =
             s"A call to get the groups from the OIDC provider failed unexpectedly for user '${token.subject}' of realm '${token.issuer}'."
-          UIO.delay(logger.error(message, e)) >> IO.raiseError(GetGroupsFromOidcError)
+          UIO.delay(logger.error(message, e)) >> IO.raiseError(GetGroupsFromOidcError(token.subject, token.issuer))
       }
   }
 

diff --git a/...dk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/model/TokenRejection.scala b/...dk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/model/TokenRejection.scala
@@ -48,13 +48,13 @@ object TokenRejection {
   /**
     * Rejection for cases where the access token is invalid according to JWTClaimsVerifier
     */
-  final case class InvalidAccessToken(details: Option[String] = None)
-      extends TokenRejection("The provided token is invalid.")
+  final case class InvalidAccessToken(subject: String, issuer: String, details: String)
+      extends TokenRejection(s"The provided token is invalid for user '$subject/$issuer' .")
 
   /**
     * Rejection for cases where we couldn't fetch the groups from the OIDC provider
     */
-  final case object GetGroupsFromOidcError
+  final case class GetGroupsFromOidcError(subject: String, issuer: String)
       extends TokenRejection(
         "The token is invalid; possible causes are: the OIDC provider is unreachable."
       )
@@ -64,8 +64,8 @@ object TokenRejection {
       val tpe  = ClassUtils.simpleName(r)
       val json = JsonObject.empty.add(keywords.tpe, tpe.asJson).add("reason", r.reason.asJson)
       r match {
-        case InvalidAccessToken(Some(details)) => json.add("details", details.asJson)
-        case _                                 => json
+        case InvalidAccessToken(_, _, error) => json.add("details", error.asJson)
+        case _                               => json
       }
     }
 

diff --git a/delta/sdk/src/test/resources/identities/invalid-access-token.json b/delta/sdk/src/test/resources/identities/invalid-access-token.json
@@ -1,6 +1,6 @@
 {
   "@context" : "https://bluebrain.github.io/nexus/contexts/error.json",
   "@type" : "InvalidAccessToken",
-  "reason" : "The provided token is invalid.",
+  "reason" : "The provided token is invalid for user '{{subject}}/{{issuer}}' .",
   "details": "Expired JWT"
 }
diff --git a/.../sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/AuthDirectivesSpec.scala b/.../sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/directives/AuthDirectivesSpec.scala
@@ -50,7 +50,7 @@ class AuthDirectivesSpec extends RouteHelpers with TestHelpers with Matchers wit
       token match {
         case AuthToken("alice") => IO.pure(userCaller)
         case AuthToken("bob")   => IO.pure(user2Caller)
-        case _                  => IO.raiseError(InvalidAccessToken(Some("Expired JWT")))
+        case _                  => IO.raiseError(InvalidAccessToken("John", "Doe", "Expired JWT"))
 
       }
     }
@@ -118,7 +118,11 @@ class AuthDirectivesSpec extends RouteHelpers with TestHelpers with Matchers wit
     "fail with an invalid token" in {
       Get("/user") ~> addCredentials(OAuth2BearerToken("unknown")) ~> callerRoute ~> check {
         response.status shouldEqual StatusCodes.Unauthorized
-        response.asJson shouldEqual jsonContentOf("identities/invalid-access-token.json")
+        response.asJson shouldEqual jsonContentOf(
+          "identities/invalid-access-token.json",
+          "subject" -> "John",
+          "issuer"  -> "Doe"
+        )
       }
     }
 

diff --git a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesDummy.scala b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesDummy.scala
@@ -11,7 +11,9 @@ import monix.bio.IO
 class IdentitiesDummy private (expected: Map[AuthToken, Caller]) extends Identities {
 
   override def exchange(token: AuthToken): IO[TokenRejection, Caller] =
-    IO.fromEither(expected.get(token).toRight(InvalidAccessToken()))
+    IO.fromEither(
+      expected.get(token).toRight(InvalidAccessToken("Someone", "Some realm", "The caller could not be found."))
+    )
 }
 
 object IdentitiesDummy {

diff --git a/.../sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImplSpec.scala b/.../sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/identities/IdentitiesImplSpec.scala
@@ -240,7 +240,11 @@ class IdentitiesImplSpec
         groups = Some(Set("group1", "group2"))
       )
 
-      identities.exchange(token).rejected shouldEqual InvalidAccessToken(Some("JWT audience rejected: [ca, de]"))
+      identities.exchange(token).rejected shouldEqual InvalidAccessToken(
+        "Robert",
+        githubLabel2.value,
+        "JWT audience rejected: [ca, de]"
+      )
     }
 
     "fail when the token is invalid" in {
@@ -296,7 +300,7 @@ class IdentitiesImplSpec
         useCommas = true
       )
 
-      identities.exchange(token).rejected shouldEqual InvalidAccessToken(Some("Expired JWT"))
+      identities.exchange(token).rejected shouldEqual InvalidAccessToken("Robert", githubLabel.value, "Expired JWT")
     }
 
     "fail when the token is not yet valid" in {
@@ -310,7 +314,11 @@ class IdentitiesImplSpec
         useCommas = true
       )
 
-      identities.exchange(token).rejected shouldEqual InvalidAccessToken(Some("JWT before use time"))
+      identities.exchange(token).rejected shouldEqual InvalidAccessToken(
+        "Robert",
+        githubLabel.value,
+        "JWT before use time"
+      )
     }
 
     "fail when the signature is invalid" in {
@@ -323,7 +331,9 @@ class IdentitiesImplSpec
       )
 
       identities.exchange(token).rejected shouldEqual InvalidAccessToken(
-        Some("Signed JWT rejected: Another algorithm expected, or no matching key(s) found")
+        "Robert",
+        githubLabel.value,
+        "Signed JWT rejected: Another algorithm expected, or no matching key(s) found"
       )
     }
 
@@ -336,7 +346,7 @@ class IdentitiesImplSpec
         useCommas = true
       )
 
-      identities.exchange(token).rejected shouldEqual GetGroupsFromOidcError
+      identities.exchange(token).rejected shouldEqual GetGroupsFromOidcError("Robert", gitlabLabel.value)
     }
   }
 

diff --git a/tests/src/test/resources/iam/identities/errors.json b/tests/src/test/resources/iam/identities/errors.json
@@ -0,0 +1,5 @@
+{
+  "@context": "https://bluebrain.github.io/nexus/contexts/error.json",
+  "@type": "InvalidAccessTokenFormat",
+  "reason": "Access token is invalid; possible causes are: JWT not signed, encoded parts are not properly encoded or each part is not a valid json."
+}
diff --git a/tests/src/test/resources/iam/identities/response.json b/tests/src/test/resources/iam/identities/response.json
@@ -0,0 +1,23 @@
+{
+  "@context": [
+    "https://bluebrain.github.io/nexus/contexts/metadata.json",
+    "https://bluebrain.github.io/nexus/contexts/identities.json"
+  ],
+  "identities": [
+    {
+      "@id": "{{deltaUri}}/anonymous",
+      "@type": "Anonymous"
+    },
+    {
+      "@id": "{{deltaUri}}/realms/internal/authenticated",
+      "@type": "Authenticated",
+      "realm": "internal"
+    },
+    {
+      "@id": "{{deltaUri}}/realms/internal/users/service-account-delta",
+      "@type": "User",
+      "realm": "internal",
+      "subject": "service-account-delta"
+    }
+  ]
+}
diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/BaseSpec.scala
@@ -110,7 +110,7 @@ trait BaseSpec
   override def afterAll(): Unit =
     Task.when(config.cleanUp)(elasticsearchDsl.deleteAllIndices().void).runSyncUnsafe()
 
-  private def toAuthorizationHeader(token: String) =
+  protected def toAuthorizationHeader(token: String) =
     Authorization(
       HttpCredentials.createOAuth2BearerToken(token)
     )

diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/Identity.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/Identity.scala
@@ -33,6 +33,9 @@ object Identity extends TestHelpers {
   val testRealm  = Realm("test-" + genString())
   val testClient = Identity.ClientCredentials(genString(), genString(), testRealm)
 
+  // User with an invalid token
+  val InvalidTokenUser: UserCredentials = UserCredentials(genString(), genString(), testRealm)
+
   object acls {
     val Marge = UserCredentials(genString(), genString(), testRealm)
   }

diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/iam/IdentitiesSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/iam/IdentitiesSpec.scala
@@ -0,0 +1,30 @@
+package ch.epfl.bluebrain.nexus.tests.iam
+
+import akka.http.scaladsl.model.StatusCodes
+import ch.epfl.bluebrain.nexus.tests.HttpClient.tokensMap
+import ch.epfl.bluebrain.nexus.tests.{BaseSpec, Identity}
+import io.circe.Json
+
+class IdentitiesSpec extends BaseSpec {
+
+  "The /identities endpoint" should {
+    s"return identities of the user" in {
+      deltaClient.get[Json]("/identities", Identity.ServiceAccount) { (json, response) =>
+        response.status shouldEqual StatusCodes.OK
+        json shouldEqual jsonContentOf(
+          "/iam/identities/response.json",
+          "deltaUri" -> config.deltaUri.toString()
+        )
+      }
+    }
+
+    "return the error for an invalid token" in {
+      tokensMap.put(Identity.InvalidTokenUser, toAuthorizationHeader("INVALID"))
+
+      deltaClient.get[Json]("/identities", Identity.InvalidTokenUser) { (json, response) =>
+        response.status shouldEqual StatusCodes.Unauthorized
+        json shouldEqual jsonContentOf("/iam/identities/errors.json")
+      }
+    }
+  }
+}
diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/VersionSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/VersionSpec.scala
@@ -1,6 +1,6 @@
 package ch.epfl.bluebrain.nexus.tests.kg
 
-import akka.http.scaladsl.model.{StatusCodes, Uri}
+import akka.http.scaladsl.model.StatusCodes
 import ch.epfl.bluebrain.nexus.testkit.EitherValuable
 import ch.epfl.bluebrain.nexus.tests.iam.types.Permission
 import ch.epfl.bluebrain.nexus.tests.kg.VersionSpec.VersionBundle
@@ -11,8 +11,6 @@ import monix.execution.Scheduler.Implicits.global
 
 class VersionSpec extends BaseSpec with EitherValuable {
 
-  val versionUri: Uri = Uri(s"http://${System.getProperty("delta:8080")}/v1/version")
-
   "The /version endpoint" should {
     s"be protected by ${Permission.Version.Read.value}" in {
       deltaClient.get[Json]("/version", Identity.Anonymous) { (_, response) =>