Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support request_uri_method=post #3102

Merged
merged 8 commits into from
May 15, 2024
Merged

Conversation

gerardsn
Copy link
Member

@gerardsn gerardsn commented May 10, 2024

closes #2712

TODO:

  • tests

@gerardsn gerardsn marked this pull request as ready for review May 13, 2024 11:50
@gerardsn gerardsn force-pushed the feature/request_uri_method=post branch from f9c256d to 66b1d52 Compare May 13, 2024 12:03
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
InternalError: errors.New("DID does not match client_id for requestID"),
}
}
if ro.RequestURIMethod != "post" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http.MethodPost

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request_uri_method and http methods are both specced as case sensitive, but annoyingly use opposite casing. added a comment

auth/client/iam/openid4vp.go Outdated Show resolved Hide resolved
auth/client/iam/openid4vp.go Outdated Show resolved Hide resolved
auth/client/iam/openid4vp.go Outdated Show resolved Hide resolved
auth/oauth/error.go Outdated Show resolved Hide resolved
@@ -193,7 +194,7 @@ func (r Wrapper) loadUserSession(cookies CookieReader, tenantDID did.DID, preAut
}
// Note that the session itself does not have an expiration field:
// it depends on the session store to clean up when it expires.
if !session.TenantDID.Equals(tenantDID) {
if !session.TenantDID.Equals(tenantDID) && !session.Wallet.DID.Equals(tenantDID) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wallet DID is always tenant DID?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the second OpenID4VP flow the tenantDID is the user wallet DID, (first is organization wallet)

Copy link
Member

@woutslakhorst woutslakhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update pages/deployment/oauth.rst with new behaviour

auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/jar.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
auth/api/iam/metadata.go Outdated Show resolved Hide resolved
// RequestObject is returned from the authorization request's 'request_uri' endpoint defined in RFC9101
// If 'request_uri_method' is 'post', a 'wallet_metadata' object can be provided to send with the request, OpenID4VP.
RequestObject(ctx context.Context, requestURI, requestURIMethod string, walletMetadata *oauth.AuthorizationServerMetadata) (string, error)
// RequestObjectByGet retrieves the RequestObjectByGet from the authorization request's 'request_uri' endpoint using a GET method as defined in RFC9101/OpenID4VP.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// RequestObjectByGet retrieves the RequestObjectByGet from the authorization request's 'request_uri' endpoint using a GET method as defined in RFC9101/OpenID4VP.
// RequestObjectByGet retrieves the Request Object from the authorization request's 'request_uri' endpoint using a GET method as defined in RFC9101/OpenID4VP.

Copy link
Member Author

@gerardsn gerardsn May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RequestObjectBaguette
(reverted)

auth/client/iam/interface.go Show resolved Hide resolved
auth/api/iam/api.go Outdated Show resolved Hide resolved
@gerardsn gerardsn requested a review from reinkrul May 14, 2024 12:49
@gerardsn gerardsn merged commit 4c5afc8 into master May 15, 2024
9 checks passed
@gerardsn gerardsn deleted the feature/request_uri_method=post branch May 15, 2024 07:24
rolandgroen added a commit that referenced this pull request May 23, 2024
* refs/heads/master: (71 commits)
  Remove nonce from default Request Object params (#3125)
  Burn nonce type SessionStore entries after first use (#3123)
  Crypto: alter Storage interface to create keys inside key store (#3120)
  Crypto: let Exists() return an error if one occurs (#3127)
  Bump github.com/nats-io/nats-server/v2 from 2.10.14 to 2.10.15 (#3121)
  Bump github.com/nats-io/nats.go from 1.34.1 to 1.35.0 (#3122)
  Bump google.golang.org/grpc from 1.63.2 to 1.64.0 (#3119)
  Bump azure/setup-helm from 3.5 to 4 (#3050)
  cleanup oauth constants (#3117)
  prevent panic (#3118)
  Support request_uri_method=post (#3102)
  add GetAndDelete to SessionStore (#3116)
  bugfix: redirect browser instead of returning error when requested scope is unknown (3104) (#3113)
  SQL: Fix SQL Server e2e test connection strings (#3112)
  Docs: updated v6 release notes to include missing stuff (#3108)
  IAM: nil deref when re-using same user session (#3106)
  cleanup metadata (#3103)
  Docs: fixed MySql example DSN (#3110)
  Change tag in discovery service to simple lamport clock value (int) (#3098)
  e2e tests: have Nuts containers wait for DB container healthy (#3109)
  ...

# Conflicts:
#	charts/nuts-node/values.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support request_uri in OpenID4VP flow
3 participants