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

Remove nonce from default Request Object params #3125

Merged
merged 4 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions auth/api/iam/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,18 +867,11 @@ func (r Wrapper) openidIssuerEndpoints(ctx context.Context, issuerDid did.DID) (
return authorizationEndpoint, tokenEndpoint, credentialEndpoint, nil
}

// CreateAuthorizationRequest creates an OAuth2.0 authorizationRequest redirect URL that redirects to the authorization server.
// It can create both regular OAuth2 requests and OpenID4VP requests due to the RequestModifier.
// createAuthorizationRequest creates an OAuth2.0 authorizationRequest redirect URL that redirects to the authorization server.
// It can create both regular OAuth2 requests and OpenID4VP requests due to the requestObjectModifier.
// This modifier is used by JAR.Create to generate a (JAR) request object that is added as 'request_uri' parameter.
// It's able to create an unsigned request and a signed request (JAR) based on the OAuth Server Metadata.
// By default, it adds the following parameters to a regular request:
// - client_id
// and to a signed request:
// - client_id
// - jwt.Issuer
// - jwt.Audience
// - nonce
// any of these params can be overridden by the requestObjectModifier.
func (r Wrapper) CreateAuthorizationRequest(ctx context.Context, client did.DID, server *did.DID, modifier requestObjectModifier) (*url.URL, error) {
func (r Wrapper) createAuthorizationRequest(ctx context.Context, client did.DID, server *did.DID, modifier requestObjectModifier) (*url.URL, error) {
metadata := new(oauth.AuthorizationServerMetadata)
if server != nil {
// we want to make a call according to §4.1.1 of RFC6749, https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.1
Expand Down
10 changes: 5 additions & 5 deletions auth/api/iam/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ func TestWrapper_CreateAuthorizationRequest(t *testing.T) {
return expectedJarReq
})

redirectURL, err := ctx.client.CreateAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)
redirectURL, err := ctx.client.createAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)

// return
assert.NoError(t, err)
Expand All @@ -1133,7 +1133,7 @@ func TestWrapper_CreateAuthorizationRequest(t *testing.T) {
return expectedJarReq
})

redirectURL, err := ctx.client.CreateAuthorizationRequest(context.Background(), clientDID, nil, modifier)
redirectURL, err := ctx.client.createAuthorizationRequest(context.Background(), clientDID, nil, modifier)

assert.NoError(t, err)
assert.Equal(t, "value", redirectURL.Query().Get("custom"))
Expand All @@ -1145,7 +1145,7 @@ func TestWrapper_CreateAuthorizationRequest(t *testing.T) {
ctx := newTestClient(t)
ctx.iamClient.EXPECT().AuthorizationServerMetadata(gomock.Any(), serverDID).Return(&oauth.AuthorizationServerMetadata{}, nil)

_, err := ctx.client.CreateAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)
_, err := ctx.client.createAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)

assert.Error(t, err)
assert.ErrorContains(t, err, "no authorization endpoint found in metadata for")
Expand All @@ -1154,15 +1154,15 @@ func TestWrapper_CreateAuthorizationRequest(t *testing.T) {
ctx := newTestClient(t)
ctx.iamClient.EXPECT().AuthorizationServerMetadata(gomock.Any(), serverDID).Return(nil, assert.AnError)

_, err := ctx.client.CreateAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)
_, err := ctx.client.createAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)

assert.Error(t, err)
})
t.Run("error - failed to get metadata", func(t *testing.T) {
ctx := newTestClient(t)
ctx.iamClient.EXPECT().AuthorizationServerMetadata(gomock.Any(), serverDID).Return(&oauth.AuthorizationServerMetadata{AuthorizationEndpoint: ":"}, nil)

_, err := ctx.client.CreateAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)
_, err := ctx.client.createAuthorizationRequest(context.Background(), clientDID, &serverDID, modifier)

assert.Error(t, err)
assert.ErrorContains(t, err, "failed to parse authorization endpoint URL")
Expand Down
10 changes: 4 additions & 6 deletions auth/api/iam/jar.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ type jar struct {
type JAR interface {
// Create an unsigned request object.
// By default, it adds the following parameters:
// - client_id
// - jwt.Issuer
// - jwt.Audience (if server is not nil)
// - nonce
// - client_id
// - iss
// - aud (if server is not nil)
// the request_uri_method is determined by the presence of a server (get) or not (post)
Create(client did.DID, server *did.DID, modifier requestObjectModifier) jarRequest
// Sign the jarRequest, which is available on jarRequest.Token.
// Returns an error if the jarRequest already contains a signed JWT.
Expand All @@ -75,8 +75,6 @@ func createJarRequest(client did.DID, server *did.DID, modifier requestObjectMod
params := map[string]string{
jwt.IssuerKey: client.String(),
oauth.ClientIDParam: client.String(),
// added by default, can be overriden by the caller
oauth.NonceParam: cryptoNuts.GenerateNonce(),
}
if server != nil {
requestURIMethod = "get"
Expand Down
6 changes: 2 additions & 4 deletions auth/api/iam/jar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,11 @@ func TestJar_Create(t *testing.T) {
req := jar{}.Create(verifierDID, &holderDID, modifier)
assert.Equal(t, "get", req.RequestURIMethod)
assert.Equal(t, verifierDID, req.Client)
assert.Len(t, req.Claims, 5)
assert.Len(t, req.Claims, 4)
assert.Equal(t, req.Claims[oauth.ClientIDParam], verifierDID.String())
assert.Equal(t, req.Claims[jwt.IssuerKey], verifierDID.String())
assert.Equal(t, req.Claims[jwt.AudienceKey], holderDID.String())
assert.Equal(t, req.Claims["requestObjectModifier"], "works")
assert.NotEmpty(t, req.Claims[oauth.NonceParam])
})
t.Run("request_uri_method=post", func(t *testing.T) {
modifier := func(claims map[string]string) {
Expand All @@ -60,11 +59,10 @@ func TestJar_Create(t *testing.T) {
req := jar{}.Create(verifierDID, nil, modifier)
assert.Equal(t, "post", req.RequestURIMethod)
assert.Equal(t, verifierDID, req.Client)
assert.Len(t, req.Claims, 3)
assert.Len(t, req.Claims, 2)
assert.Equal(t, req.Claims[jwt.IssuerKey], holderDID.String())
assert.Equal(t, req.Claims[oauth.ClientIDParam], verifierDID.String())
assert.Empty(t, req.Claims[jwt.AudienceKey])
assert.NotEmpty(t, req.Claims[oauth.NonceParam])
})
}
func TestJar_Sign(t *testing.T) {
Expand Down
10 changes: 2 additions & 8 deletions auth/api/iam/openid4vp.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,6 @@ func (r Wrapper) handleAuthorizeRequestFromHolder(ctx context.Context, verifier
if params.get(jwt.AudienceKey) != verifier.String() {
return nil, withCallbackURI(oauthError(oauth.InvalidRequest, fmt.Sprintf("invalid audience, verifier = %s, audience = %s", verifier.String(), params.get(jwt.AudienceKey))), redirectURL)
}
// check nonce
// nonce in JWT must be present for signing to be unique for every request
// we currently do not check the nonce against a nonce store, but we could do that in the future
if params.get(oauth.NonceParam) == "" {
return nil, withCallbackURI(oauthError(oauth.InvalidRequest, "missing nonce parameter"), redirectURL)
}
// we require PKCE (RFC7636) for authorization code flows
// check code_challenge and code_challenge_method
if params.get(oauth.CodeChallengeParam) == "" {
Expand Down Expand Up @@ -212,10 +206,10 @@ func (r Wrapper) nextOpenID4VPFlow(ctx context.Context, state string, session OA
var authServerURL *url.URL
if *walletOwnerType == pe.WalletOwnerUser {
// User wallet, make an openid4vp: request URL
authServerURL, err = r.CreateAuthorizationRequest(ctx, *session.OwnDID, nil, modifier)
authServerURL, err = r.createAuthorizationRequest(ctx, *session.OwnDID, nil, modifier)
} else {
walletDID, _ := did.ParseDID(session.ClientID)
authServerURL, err = r.CreateAuthorizationRequest(ctx, *session.OwnDID, walletDID, modifier)
authServerURL, err = r.createAuthorizationRequest(ctx, *session.OwnDID, walletDID, modifier)
}
if err != nil {
return nil, oauth.OAuth2Error{
Expand Down
2 changes: 1 addition & 1 deletion auth/api/iam/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (r Wrapper) handleUserLanding(echoCtx echo.Context) error {
values[oauth.ScopeParam] = accessTokenRequest.Body.Scope
}
// TODO: First create user session, or AuthorizationRequest first? (which one is more expensive? both sign stuff)
redirectURL, err := r.CreateAuthorizationRequest(echoCtx.Request().Context(), redirectSession.OwnDID, verifier, modifier)
redirectURL, err := r.createAuthorizationRequest(echoCtx.Request().Context(), redirectSession.OwnDID, verifier, modifier)
if err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/oauth-flow/openid4vp/run-test.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#!/usr/bin/env bash
set -e
./do-test.sh didweb-user
./do-test.sh didweb-root
Loading