Refactor JAR logic #3078
Refactor JAR logic #3078
Conversation
| keyResolver resolver.KeyResolver | ||
| } | ||
|
|
||
| type JAR interface { |
There was a problem hiding this comment.
Maybe split it in JARCreater and JARParser? Since I'd expected JAR to be the signed request object itself
There was a problem hiding this comment.
Does this follow the design pattern Wout used in his DPoP PR (can it?)? Since it's sort of the same thing/principle
There was a problem hiding this comment.
I've left this as is since the goal is to logically group some methods together so they can be mocked. Might change when the request_uri_method=post is added. (next PR)
As for comparison to the DPoP PR. I see the resemblance between the two, but I don't want to mirror it since that requires moving JAR to the crypto module. The JWTSigner interface currently meets the needs we had for did:nuts, but not for did:web I think that interface should be updated so we don't pollute the crypto package.
auth/api/iam/jar.go
Outdated
| // Sign the requestObject, which is available on requestObject.Token. | ||
| // Returns an error if the requestObject already contains a signed JWT. | ||
| // TODO: check if signature type of client is supported by the AS/wallet. | ||
| Sign(ctx context.Context, ro *requestObject) error |
There was a problem hiding this comment.
could return a signed requestobject
There was a problem hiding this comment.
signing now happens when the request object is fetched, so this now returns the token as sting. This change allows for consistent behavior in both get/post request_uri_methods. It also means the signed token is no longer part of the state / in storage.
Signing now happens on the external call, and not the internal call. This probably means the audit on signing is in the wrong place again?
| @@ -145,8 +145,17 @@ paths: | |||
| schema: | |||
| type: string | |||
| format: uri | |||
| /oauth2/request.jwt/{id}: | |||
| /oauth2/{did}/request.jwt/{id}: | |||
There was a problem hiding this comment.
technically we don't need the DID in the path, but all other /oauth2/... have it...
There was a problem hiding this comment.
The same applies to a OpenID4VCI endpoint, so I don't think it needs to be per se. One more thing to validate...
gerardsn
force-pushed
the
feature/refactor-JAR-to-interface
branch
from
5ad2553 to
dfc259f
Compare
this change allows mocking and prepares to for adding
request_uri_methodpart of #2712