feat: adds verification context #3677
Conversation
wolf4ood
force-pushed
the
feat/3675_verification_context
branch
from
3f7b765 to
9cc1bf1
Compare
wolf4ood
force-pushed
the
feat/3675_verification_context
branch
from
9cc1bf1 to
a8ed6dc
Compare
wolf4ood
added
the
breaking-change
wolf4ood
requested review from
bscholtes1A and
paullatzelsperger
as code owners
spi/common/core-spi/src/main/java/org/eclipse/edc/spi/iam/VerificationContext.java
Outdated
Show resolved
Hide resolved
spi/common/core-spi/src/main/java/org/eclipse/edc/spi/iam/VerificationContext.java
Outdated
Show resolved
Hide resolved
…ficationContext.java Co-authored-by: Jim Marino <jim.marino@gmail.com>
…ficationContext.java Co-authored-by: Jim Marino <jim.marino@gmail.com>
| * @return Result of the validation. | ||
| */ | ||
| Result<ClaimToken> verifyJwtToken(TokenRepresentation tokenRepresentation, String audience); | ||
| Result<ClaimToken> verifyJwtToken(TokenRepresentation tokenRepresentation, VerificationContext context); |
There was a problem hiding this comment.
I can already here voices whining about backwards compatibility and breaking changes... Could we leave the old signature in place in a default method, and have it delegate to this new method?
the VerificationContext contains the audience anyway
There was a problem hiding this comment.
We can, but that method will not be called by our code in the protocol service layer, as we would need to populate the VerificationContext . Also the audience there will go away soon :)
There was a problem hiding this comment.
@paullatzelsperger hey, that's me 😆
In my opinion it really depends on how many extensions of this could exist, seems to be a pretty used interface so I'd go for the deprecation, but I will be also ok in changing without deprecating
(interface changes are generally more tolerated because only devs are involved, instead rest apis... well, you know)
| * @return Result of the validation. | ||
| */ | ||
| Result<ClaimToken> verifyJwtToken(TokenRepresentation tokenRepresentation, String audience); | ||
| Result<ClaimToken> verifyJwtToken(TokenRepresentation tokenRepresentation, VerificationContext context); |
There was a problem hiding this comment.
@paullatzelsperger hey, that's me 😆
In my opinion it really depends on how many extensions of this could exist, seems to be a pretty used interface so I'd go for the deprecation, but I will be also ok in changing without deprecating
(interface changes are generally more tolerated because only devs are involved, instead rest apis... well, you know)
|
I've restored as default method the one with |
What this PR changes/adds
Adds
VerificationContextin theIdentityService#verifyJwtmethodsWhy it does that
To be able to attach contextual information in the verification phase
Further notes
For now the
Policyit's always an empty one. Policy extraction will be tackled in subsequent PRs on each area (catalog/transfer process/ contract negotiation)Linked Issue(s)
Closes #3675