Remove callback to Delta to check the token, validate it locally #4340
Conversation
| delta { | ||
| public-iri = "https://test.nexus.bbp.epfl.ch" | ||
| internal-iri = "http://delta:8080" | ||
| authorization { |
There was a problem hiding this comment.
We can't really test this part in the integration tests anymore as we need the public keys for the realm.
In the different envs, we will get notified if Delta can't interact with the storage though
storage/src/test/scala/ch/epfl/bluebrain/nexus/storage/routes/AuthDirectivesSpec.scala
Outdated
Show resolved
Hide resolved
storage/src/test/scala/ch/epfl/bluebrain/nexus/storage/routes/AuthDirectivesSpec.scala
Outdated
Show resolved
Hide resolved
storage/src/test/scala/ch/epfl/bluebrain/nexus/storage/routes/AuthDirectivesSpec.scala
Show resolved
Hide resolved
storage/src/main/scala/ch/epfl/bluebrain/nexus/storage/auth/AuthorizationError.scala
Outdated
Show resolved
Hide resolved
storage/src/main/scala/ch/epfl/bluebrain/nexus/storage/auth/AuthorizationMethod.scala
Outdated
Show resolved
Hide resolved
storage/src/main/scala/ch/epfl/bluebrain/nexus/storage/routes/Routes.scala
Outdated
Show resolved
Hide resolved
| def validate(audiences: Option[NonEmptySet[String]], keySet: JWKSet): Either[InvalidAccessToken, JWTClaimsSet] = { | ||
| val proc = new DefaultJWTProcessor[SecurityContext] | ||
| val keySelector = new JWSVerificationKeySelector(JWSAlgorithm.RS256, new ImmutableJWKSet[SecurityContext](keySet)) | ||
| proc.setJWSKeySelector(keySelector) | ||
| audiences.foreach { aud => | ||
| proc.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier(aud.toSet.asJava, null, null, null)) | ||
| } | ||
| Either | ||
| .catchNonFatal(proc.process(jwtToken, null)) | ||
| .leftMap(err => InvalidAccessToken(subject, issuer, err.getMessage)) | ||
| } |
There was a problem hiding this comment.
I'm expecting there might be complication with this, but it might be better if we has a constructor that returned an already validated token? There is something about validate being a method you can optionally call which feels a bit weird. Not a big deal though since you've got testing to cover
There was a problem hiding this comment.
It comes from the way we do things in IdentitiesImpl, we do some surface checks first and extract the issuer to:
- Get the matching realm and the public keys associated to it
- Validate the token with the keys
I will check if there is a way to improve that a bit
There was a problem hiding this comment.
One other thing: if we don't use the JWTClaimsSet then it's probably clearer to return Unit
delta/kernel/src/main/scala/ch/epfl/bluebrain/nexus/delta/kernel/jwt/ParsedToken.scala
Outdated
Show resolved
Hide resolved
| def validate(audiences: Option[NonEmptySet[String]], keySet: JWKSet): Either[InvalidAccessToken, Unit] = { | ||
| val proc = new DefaultJWTProcessor[SecurityContext] | ||
| val keySelector = new JWSVerificationKeySelector(JWSAlgorithm.RS256, new ImmutableJWKSet[SecurityContext](keySet)) | ||
| proc.setJWSKeySelector(keySelector) | ||
| audiences.foreach { aud => | ||
| proc.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier(aud.toSet.asJava, null, null, null)) | ||
| } | ||
| Either | ||
| .catchNonFatal(proc.process(jwtToken, null)) | ||
| .bimap( | ||
| err => InvalidAccessToken(subject, issuer, err.getMessage), | ||
| _ => () | ||
| ) | ||
| } |
There was a problem hiding this comment.
As someone unfamiliar with this process, it's not clear what an audience or keySet is. And it's not clear what about them is being validated
I think with some name changes and perhaps pulling some code into private methods this could be made really clear.
There was a problem hiding this comment.
This part is already documented in the realm documentation:
https://bluebrainnexus.io/snapshot/docs/delta/api/realms-api.html#payload
And in the configuration file
Fixes #4275