Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@gerardsn
gerardsn committed May 14, 2024
1 parent 6e49453 commit 38d018a
Show file tree
Hide file tree
Showing 14 changed files with 194 additions and 158 deletions.
45 changes: 21 additions & 24 deletions auth/api/iam/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,9 @@ func (r Wrapper) handleAuthorizeRequest(ctx context.Context, ownDID did.DID, req
}
}

// GetRequestJWT returns the Request Object referenced as 'request_uri' in an authorization request.
// RequestJWTByGet returns the Request Object referenced as 'request_uri' in an authorization request.
// RFC9101: The OAuth 2.0 Authorization Framework: JWT-Secured Authorization Request (JAR).
func (r Wrapper) GetRequestJWT(ctx context.Context, request GetRequestJWTRequestObject) (GetRequestJWTResponseObject, error) {
func (r Wrapper) RequestJWTByGet(ctx context.Context, request RequestJWTByGetRequestObject) (RequestJWTByGetResponseObject, error) {
ro := new(jarRequest)
err := r.authzRequestObjectStore().Get(request.Id, ro)
if err != nil {
Expand All @@ -394,12 +394,11 @@ func (r Wrapper) GetRequestJWT(ctx context.Context, request GetRequestJWTRequest
// compare raw strings, don't waste a db call to see if we own the request.Did.
if ro.Client.String() != request.Did {
return nil, oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "client_id does not match request",
InternalError: errors.New("DID does not match client_id for requestID"),
Code: oauth.InvalidRequest,
Description: "client_id does not match request",
}
}
if ro.RequestURIMethod != "get" {
if ro.RequestURIMethod != "get" { // case sensitive
// TODO: wallet does not support `request_uri_method=post`. Signing the current jarRequest would leave it without 'aud'.
// is this acceptable, should it fail, or does it default to using staticAuthorizationServerMetadata.
return nil, oauth.OAuth2Error{
Expand All @@ -414,20 +413,20 @@ func (r Wrapper) GetRequestJWT(ctx context.Context, request GetRequestJWTRequest
if err != nil {
return nil, oauth.OAuth2Error{
Code: oauth.ServerError,
Description: "failed to sign authorization RequestObject",
InternalError: err,
Description: "unable to create RequestObjectByGet",
InternalError: fmt.Errorf("failed to sign authorization RequestObjectByGet :%w", err),
}
}
return GetRequestJWT200ApplicationoauthAuthzReqJwtResponse{
return RequestJWTByGet200ApplicationoauthAuthzReqJwtResponse{
Body: bytes.NewReader([]byte(token)),
ContentLength: int64(len(token)),
}, nil
}

// PostRequestJWT returns the Request Object referenced as 'request_uri' in an authorization request.
// RequestJWTByPost returns the Request Object referenced as 'request_uri' in an authorization request.
// Extension of OpenID 4 Verifiable Presentations (OpenID4VP) on
// RFC9101: The OAuth 2.0 Authorization Framework: JWT-Secured Authorization Request (JAR).
func (r Wrapper) PostRequestJWT(ctx context.Context, request PostRequestJWTRequestObject) (PostRequestJWTResponseObject, error) {
func (r Wrapper) RequestJWTByPost(ctx context.Context, request RequestJWTByPostRequestObject) (RequestJWTByPostResponseObject, error) {
ro := new(jarRequest)
err := r.authzRequestObjectStore().Get(request.Id, ro)
if err != nil {
Expand All @@ -439,12 +438,11 @@ func (r Wrapper) PostRequestJWT(ctx context.Context, request PostRequestJWTReque
// compare raw strings, don't waste a db call to see if we own the request.Did.
if ro.Client.String() != request.Did {
return nil, oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "client_id does not match request",
InternalError: errors.New("DID does not match client_id for requestID"),
Code: oauth.InvalidRequest,
Description: "client_id does not match request",
}
}
if ro.RequestURIMethod != "post" {
if ro.RequestURIMethod != "post" { // case sensitive
return nil, oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "used request_uri_method 'post' on a 'get' request_uri",
Expand All @@ -467,11 +465,11 @@ func (r Wrapper) PostRequestJWT(ctx context.Context, request PostRequestJWTReque
if err != nil {
return nil, oauth.OAuth2Error{
Code: oauth.ServerError,
Description: "failed to sign authorization RequestObject",
InternalError: err,
Description: "unable to create RequestObjectByGet",
InternalError: fmt.Errorf("failed to sign authorization RequestObjectByGet :%w", err),
}
}
return PostRequestJWT200ApplicationoauthAuthzReqJwtResponse{
return RequestJWTByPost200ApplicationoauthAuthzReqJwtResponse{
Body: bytes.NewReader([]byte(token)),
ContentLength: int64(len(token)),
}, nil
Expand Down Expand Up @@ -880,8 +878,6 @@ func (r Wrapper) openidIssuerEndpoints(ctx context.Context, issuerDid did.DID) (
// - 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) {
// if the server is unknown/nil we are talking to a wallet.
// by default requireSignedRequestObject=true to make sure the produced Authorization Request URL does not exceed request URL limit on mobile devices
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 All @@ -891,16 +887,17 @@ func (r Wrapper) CreateAuthorizationRequest(ctx context.Context, client did.DID,
if err != nil {
return nil, fmt.Errorf("failed to retrieve remote OAuth Authorization Server metadata: %w", err)
}
if len(metadata.AuthorizationEndpoint) == 0 {
return nil, fmt.Errorf("no authorization endpoint found in metadata for %s", *server)
}
} else {
// use static configuration until while we try to determine the wallet that will answer the authorization request. (user wallet / QR code flow)
// if the server is unknown/nil we are talking to a wallet.
// use static configuration while we try to determine the wallet that will answer the authorization request. (user wallet / QR code flow)
*metadata = staticAuthorizationServerMetadata()
// TODO: metadata.RequireSignedRequestObject == false.
// This means we send both a request_uri and add all params to the authorization request as query params.
// The resulting url is too long and will be rejected by mobile devices.
}
if len(metadata.AuthorizationEndpoint) == 0 {
return nil, fmt.Errorf("no authorization endpoint found in metadata for %s", *server)
}
endpoint, err := url.Parse(metadata.AuthorizationEndpoint)
if err != nil {
return nil, fmt.Errorf("failed to parse authorization endpoint URL: %w", err)
Expand Down
38 changes: 19 additions & 19 deletions auth/api/iam/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -951,18 +951,18 @@ func TestWrapper_GetRequestJWT(t *testing.T) {
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))
ctx.jar.EXPECT().Sign(cont, ro.Claims).Return(expectedToken, nil)

response, err := ctx.client.GetRequestJWT(cont, GetRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByGet(cont, RequestJWTByGetRequestObject{Did: webDID.String(), Id: requestID})

assert.NoError(t, err)
assert.Equal(t, GetRequestJWT200ApplicationoauthAuthzReqJwtResponse{
assert.Equal(t, RequestJWTByGet200ApplicationoauthAuthzReqJwtResponse{
Body: bytes.NewReader([]byte(expectedToken)),
ContentLength: 10,
}, response)
})
t.Run("error - not found", func(t *testing.T) {
ctx := newTestClient(t)

response, err := ctx.client.GetRequestJWT(nil, GetRequestJWTRequestObject{Id: "unknownID"})
response, err := ctx.client.RequestJWTByGet(nil, RequestJWTByGetRequestObject{Id: "unknownID"})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - request object not found")
Expand All @@ -972,18 +972,18 @@ func TestWrapper_GetRequestJWT(t *testing.T) {
ro := jar{}.Create(webDID, &holderDID, func(claims map[string]string) {})
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))

response, err := ctx.client.GetRequestJWT(cont, GetRequestJWTRequestObject{Did: holderDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByGet(cont, RequestJWTByGetRequestObject{Did: holderDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - DID does not match client_id for requestID - client_id does not match request")
assert.EqualError(t, err, "invalid_request - client_id does not match request")
})
t.Run("error - wrong request_uri_method used", func(t *testing.T) {
ctx := newTestClient(t)
ro := jar{}.Create(webDID, &holderDID, func(claims map[string]string) {})
ro.RequestURIMethod = "post"
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))

response, err := ctx.client.GetRequestJWT(cont, GetRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByGet(cont, RequestJWTByGetRequestObject{Did: webDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - wrong 'request_uri_method' authorization server or wallet probably does not support 'request_uri_method' - used request_uri_method 'get' on a 'post' request_uri")
Expand All @@ -994,10 +994,10 @@ func TestWrapper_GetRequestJWT(t *testing.T) {
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))
ctx.jar.EXPECT().Sign(cont, ro.Claims).Return("", errors.New("fail"))

response, err := ctx.client.GetRequestJWT(cont, GetRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByGet(cont, RequestJWTByGetRequestObject{Did: webDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "server_error - fail - failed to sign authorization RequestObject")
assert.EqualError(t, err, "server_error - failed to sign authorization RequestObjectByGet :fail - unable to create RequestObjectByGet")
})
}

Expand All @@ -1021,10 +1021,10 @@ func TestWrapper_PostRequestJWT(t *testing.T) {
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))
ctx.jar.EXPECT().Sign(cont, ro.Claims).Return(expectedToken, nil)

response, err := ctx.client.PostRequestJWT(cont, PostRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByPost(cont, RequestJWTByPostRequestObject{Did: webDID.String(), Id: requestID})

assert.NoError(t, err)
assert.Equal(t, PostRequestJWT200ApplicationoauthAuthzReqJwtResponse{
assert.Equal(t, RequestJWTByPost200ApplicationoauthAuthzReqJwtResponse{
Body: bytes.NewReader([]byte(expectedToken)),
ContentLength: 10,
}, response)
Expand All @@ -1035,23 +1035,23 @@ func TestWrapper_PostRequestJWT(t *testing.T) {
ro := newReqObj("mario", wallet_nonce)
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))
ctx.jar.EXPECT().Sign(cont, ro.Claims).Return(expectedToken, nil)
body := PostRequestJWTFormdataRequestBody(PostRequestJWTFormdataBody{
body := RequestJWTByPostFormdataRequestBody(RequestJWTByPostFormdataBody{
WalletMetadata: &oauth.AuthorizationServerMetadata{Issuer: "mario"},
WalletNonce: &wallet_nonce,
})

response, err := ctx.client.PostRequestJWT(cont, PostRequestJWTRequestObject{Did: webDID.String(), Id: requestID, Body: &body})
response, err := ctx.client.RequestJWTByPost(cont, RequestJWTByPostRequestObject{Did: webDID.String(), Id: requestID, Body: &body})

assert.NoError(t, err)
assert.Equal(t, PostRequestJWT200ApplicationoauthAuthzReqJwtResponse{
assert.Equal(t, RequestJWTByPost200ApplicationoauthAuthzReqJwtResponse{
Body: bytes.NewReader([]byte(expectedToken)),
ContentLength: 10,
}, response)
})
t.Run("error - not found", func(t *testing.T) {
ctx := newTestClient(t)

response, err := ctx.client.PostRequestJWT(nil, PostRequestJWTRequestObject{Id: "unknownID"})
response, err := ctx.client.RequestJWTByPost(nil, RequestJWTByPostRequestObject{Id: "unknownID"})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - request object not found")
Expand All @@ -1060,18 +1060,18 @@ func TestWrapper_PostRequestJWT(t *testing.T) {
ctx := newTestClient(t)
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, newReqObj("", "")))

response, err := ctx.client.PostRequestJWT(cont, PostRequestJWTRequestObject{Did: holderDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByPost(cont, RequestJWTByPostRequestObject{Did: holderDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - DID does not match client_id for requestID - client_id does not match request")
assert.EqualError(t, err, "invalid_request - client_id does not match request")
})
t.Run("error - wrong request_uri_method used", func(t *testing.T) {
ctx := newTestClient(t)
ro := newReqObj("", "")
ro.RequestURIMethod = "get"
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))

response, err := ctx.client.PostRequestJWT(cont, PostRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByPost(cont, RequestJWTByPostRequestObject{Did: webDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "invalid_request - used request_uri_method 'post' on a 'get' request_uri")
Expand All @@ -1082,10 +1082,10 @@ func TestWrapper_PostRequestJWT(t *testing.T) {
require.NoError(t, ctx.client.authzRequestObjectStore().Put(requestID, ro))
ctx.jar.EXPECT().Sign(cont, ro.Claims).Return("", errors.New("fail"))

response, err := ctx.client.PostRequestJWT(cont, PostRequestJWTRequestObject{Did: webDID.String(), Id: requestID})
response, err := ctx.client.RequestJWTByPost(cont, RequestJWTByPostRequestObject{Did: webDID.String(), Id: requestID})

assert.Nil(t, response)
assert.EqualError(t, err, "server_error - fail - failed to sign authorization RequestObject")
assert.EqualError(t, err, "server_error - failed to sign authorization RequestObjectByGet :fail - unable to create RequestObjectByGet")
})
}

Expand Down
Loading

0 comments on commit 38d018a

Please sign in to comment.