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

feat: [#628] PAR implementation #660

Merged
merged 23 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f9e075b
feat: [#628] PAR implementation
vivshankar Mar 26, 2022
a4ca02e
feat: Added the PAR handler and fixed a lint error
vivshankar Mar 26, 2022
bc679eb
fix: [#628] Validate that request_uri is not included in the request …
vivshankar Mar 26, 2022
0b255d6
fix: [#628] Error code should be invalid_request_object
vivshankar Mar 26, 2022
2966302
Merge branch 'master' of github.com:ory/fosite into vivshankar-628-par
vivshankar May 6, 2022
17b394c
fix: [#628] Fixed messages and added test
vivshankar May 6, 2022
c1a97c0
test: [#628] Added PAR integration test
vivshankar May 28, 2022
0d0fdcd
doc: Added reference to the RFC
vivshankar May 29, 2022
1c6d3b8
feat: [#628] PAR implementation
vivshankar Mar 26, 2022
e308949
feat: Added the PAR handler and fixed a lint error
vivshankar Mar 26, 2022
fa21602
fix: [#628] Validate that request_uri is not included in the request …
vivshankar Mar 26, 2022
c6cfc66
fix: [#628] Error code should be invalid_request_object
vivshankar Mar 26, 2022
ec00439
fix: [#628] Fixed messages and added test
vivshankar May 6, 2022
96fe9d6
test: [#628] Added PAR integration test
vivshankar May 28, 2022
7e7319a
doc: Added reference to the RFC
vivshankar May 29, 2022
962698a
Merge branch 'vivshankar-628-par' of github.com:vivshankar/fosite int…
vivshankar Jun 25, 2022
de4888a
fix: [#628] Changes to adopt the Configurator interface pattern
vivshankar Jun 25, 2022
ef94374
fix: Minor error message change
vivshankar Jun 25, 2022
850ff16
chore: code review
aeneasr Jul 11, 2022
9bfe344
fix: [#628] Add more error checking in tests and standardize config e…
vivshankar Jul 11, 2022
5a68b42
fix: [#628] Add more error checking in tests and standardize config e…
vivshankar Jul 11, 2022
49729fe
chore: remove empty method check
aeneasr Jul 19, 2022
da3dab6
chore: improve error message
aeneasr Jul 19, 2022
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
66 changes: 64 additions & 2 deletions authorize_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func wrapSigningKeyFailure(outer *RFC6749Error, inner error) *RFC6749Error {
return outer
}

func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(request *AuthorizeRequest) error {
func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(request *AuthorizeRequest, isPARRequest bool) error {
var scope Arguments = RemoveEmpty(strings.Split(request.Form.Get("scope"), " "))

// Even if a scope parameter is present in the Request Object value, a scope parameter MUST always be passed using
Expand Down Expand Up @@ -158,6 +158,12 @@ func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(request *Aut
}

claims := token.Claims
// Reject the request if the "request_uri" authorization request
// parameter is provided.
if requestURI, _ := claims["request_uri"].(string); isPARRequest && requestURI != "" {
aeneasr marked this conversation as resolved.
Show resolved Hide resolved
return errorsx.WithStack(ErrInvalidRequestObject.WithHint("Pushed Authorization Requests can not contain the 'request_uri' parameter."))
}

for k, v := range claims {
request.Form.Set(k, fmt.Sprintf("%s", v))
}
Expand Down Expand Up @@ -275,7 +281,51 @@ func (f *Fosite) validateResponseMode(r *http.Request, request *AuthorizeRequest
return nil
}

func (f *Fosite) authorizeRequestFromPAR(ctx context.Context, r *http.Request, request *AuthorizeRequest) (bool, error) {
requestURI := r.Form.Get("request_uri")
if requestURI == "" || !strings.HasPrefix(requestURI, f.PushedAuthorizationRequestURIPrefix) {
// nothing to do here
return false, nil
}

clientID := r.Form.Get("client_id")

storage, ok := f.Store.(PARStorage)
if !ok {
Copy link
Member

Choose a reason for hiding this comment

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

We have a similar error check in the code above. Can we introduce a singular error that we reuse when the storage does not implement PAR?

Copy link
Member

Choose a reason for hiding this comment

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

@vivshankar did you have time to address this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have addressed this comment by adding a specific error message for this. Not quite sure what you mean by "similar error check".

Copy link
Member

Choose a reason for hiding this comment

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

We check on multiple occasions that the config or store implement PAR. We could use a single error var PARNotImplemented = ... and reuse that error instead of writing new errors for every check

return false, errorsx.WithStack(ErrServerError.WithHint("Invalid storage").WithDebug("PARStorage not implemented"))
}

// hydrate the requester
var parRequest AuthorizeRequester
var err error
if parRequest, err = storage.GetPARSession(ctx, requestURI); err != nil {
return false, errorsx.WithStack(ErrInvalidRequestURI.WithHint("Invalid PAR session").WithWrap(err).WithDebug(err.Error()))
}

// hydrate the request object
request.Merge(parRequest)
request.RedirectURI = parRequest.GetRedirectURI()
request.ResponseTypes = parRequest.GetResponseTypes()
request.State = parRequest.GetState()
request.ResponseMode = parRequest.GetResponseMode()
Copy link
Member

Choose a reason for hiding this comment

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

Could you point to where these values are also set so that we ensure we did not forget setting some important ones? Could you please also point to the RFC section (and maybe add them as a comment) that state which parameters can be set as part of PAR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Link to RFC section

A client sends the parameters that comprise an authorization request directly to the PAR endpoint. A typical parameter set might include: client_id, response_type, redirect_uri, scope, state, code_challenge, and code_challenge_method as shown in the example below. However, the pushed authorization request can be composed of any of the parameters applicable for use at the authorization endpoint, including those defined in [[RFC6749](https://www.rfc-editor.org/rfc/rfc9126.html#RFC6749)] as well as all applicable extensions. The request_uri authorization request parameter is one exception, and it MUST NOT be provided.

These are set in the call to NewAuthorizeRequest from within the NewPushedAuthorizationRequest.


if err := storage.DeletePARSession(ctx, requestURI); err != nil {
return false, errorsx.WithStack(ErrServerError.WithWrap(err).WithDebug(err.Error()))
}

// validate the clients match
if clientID != request.GetClient().GetID() {
return false, errorsx.WithStack(ErrInvalidRequest.WithHint("The 'client_id' must match the one sent in the pushed authorization request."))
}

return true, nil
}

func (f *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (AuthorizeRequester, error) {
return f.newAuthorizeRequest(ctx, r, false)
}

func (f *Fosite) newAuthorizeRequest(ctx context.Context, r *http.Request, isPARRequest bool) (AuthorizeRequester, error) {
request := NewAuthorizeRequest()
request.Request.Lang = i18n.GetLangFromRequest(f.MessageCatalog, r)

Expand All @@ -290,6 +340,18 @@ func (f *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
// Save state to the request to be returned in error conditions (https://github.com/ory/hydra/issues/1642)
request.State = request.Form.Get("state")

// Check if this is a continuation from a pushed authorization request
if !isPARRequest {
if isPAR, err := f.authorizeRequestFromPAR(ctx, r, request); err != nil {
return request, err
} else if isPAR {
// No need to continue
return request, nil
} else if f.EnforcePushedAuthorization {
return request, errorsx.WithStack(ErrInvalidRequest.WithHint("Pushed Authorization Requests are enforced but no such request was sent."))
}
}

client, err := f.Store.GetClient(ctx, request.GetRequestForm().Get("client_id"))
if err != nil {
return request, errorsx.WithStack(ErrInvalidClient.WithHint("The requested OAuth 2.0 Client does not exist.").WithWrap(err).WithDebug(err.Error()))
Expand All @@ -301,7 +363,7 @@ func (f *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
//
// All other parse methods should come afterwards so that we ensure that the data is taken
// from the request_object if set.
if err := f.authorizeRequestParametersFromOpenIDConnectRequest(request); err != nil {
if err := f.authorizeRequestParametersFromOpenIDConnectRequest(request, isPARRequest); err != nil {
return request, err
}

Expand Down
2 changes: 1 addition & 1 deletion authorize_request_handler_oidc_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func TestAuthorizeRequestParametersFromOpenIDConnectRequest(t *testing.T) {
},
}

err := f.authorizeRequestParametersFromOpenIDConnectRequest(req)
err := f.authorizeRequestParametersFromOpenIDConnectRequest(req, false)
if tc.expectErr != nil {
require.EqualError(t, err, tc.expectErr.Error(), "%+v", err)
if tc.expectErrReason != "" {
Expand Down
58 changes: 41 additions & 17 deletions compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,17 @@ package compose

import (
"crypto/rsa"
"time"

"github.com/ory/fosite"
"github.com/ory/fosite/token/jwt"
)

const (
defaultPARPrefix = "urn:ietf:params:oauth:request_uri:"
defaultPARContextLifetime = 5 * time.Minute
)

type Factory func(config *Config, storage interface{}, strategy interface{}) interface{}

// Compose takes a config, a storage, a strategy and handlers to instantiate an OAuth2Provider:
Expand All @@ -53,28 +59,32 @@ type Factory func(config *Config, storage interface{}, strategy interface{}) int
//
// Compose makes use of interface{} types in order to be able to handle a all types of stores, strategies and handlers.
func Compose(config *Config, storage interface{}, strategy interface{}, hasher fosite.Hasher, factories ...Factory) fosite.OAuth2Provider {
setDefaults(config)
if hasher == nil {
hasher = &fosite.BCrypt{WorkFactor: config.GetHashCost()}
}

f := &fosite.Fosite{
Store: storage.(fosite.Storage),
AuthorizeEndpointHandlers: fosite.AuthorizeEndpointHandlers{},
TokenEndpointHandlers: fosite.TokenEndpointHandlers{},
TokenIntrospectionHandlers: fosite.TokenIntrospectionHandlers{},
RevocationHandlers: fosite.RevocationHandlers{},
Hasher: hasher,
ScopeStrategy: config.GetScopeStrategy(),
AudienceMatchingStrategy: config.GetAudienceStrategy(),
SendDebugMessagesToClients: config.SendDebugMessagesToClients,
TokenURL: config.TokenURL,
JWKSFetcherStrategy: config.GetJWKSFetcherStrategy(),
MinParameterEntropy: config.GetMinParameterEntropy(),
UseLegacyErrorFormat: config.UseLegacyErrorFormat,
ClientAuthenticationStrategy: config.GetClientAuthenticationStrategy(),
ResponseModeHandlerExtension: config.ResponseModeHandlerExtension,
MessageCatalog: config.MessageCatalog,
FormPostHTMLTemplate: config.FormPostHTMLTemplate,
Store: storage.(fosite.Storage),
AuthorizeEndpointHandlers: fosite.AuthorizeEndpointHandlers{},
TokenEndpointHandlers: fosite.TokenEndpointHandlers{},
TokenIntrospectionHandlers: fosite.TokenIntrospectionHandlers{},
RevocationHandlers: fosite.RevocationHandlers{},
Hasher: hasher,
ScopeStrategy: config.GetScopeStrategy(),
AudienceMatchingStrategy: config.GetAudienceStrategy(),
SendDebugMessagesToClients: config.SendDebugMessagesToClients,
TokenURL: config.TokenURL,
JWKSFetcherStrategy: config.GetJWKSFetcherStrategy(),
MinParameterEntropy: config.GetMinParameterEntropy(),
UseLegacyErrorFormat: config.UseLegacyErrorFormat,
ClientAuthenticationStrategy: config.GetClientAuthenticationStrategy(),
ResponseModeHandlerExtension: config.ResponseModeHandlerExtension,
MessageCatalog: config.MessageCatalog,
FormPostHTMLTemplate: config.FormPostHTMLTemplate,
PushedAuthorizationRequestURIPrefix: config.PushedAuthorizationRequestURIPrefix,
PushedAuthorizationContextLifespan: config.PushedAuthorizationContextLifespan,
EnforcePushedAuthorization: config.EnforcePushedAuthorization,
}

for _, factory := range factories {
Expand All @@ -91,6 +101,9 @@ func Compose(config *Config, storage interface{}, strategy interface{}, hasher f
if rh, ok := res.(fosite.RevocationHandler); ok {
f.RevocationHandlers.Append(rh)
}
if ph, ok := res.(fosite.PushedAuthorizeEndpointHandler); ok {
f.PushedAuthorizeEndpointHandlers.Append(ph)
}
}

return f
Expand Down Expand Up @@ -126,5 +139,16 @@ func ComposeAllEnabled(config *Config, storage interface{}, secret []byte, key *
OAuth2TokenRevocationFactory,

OAuth2PKCEFactory,
PushedAuthorizeHandlerFactory,
)
}

func setDefaults(config *Config) {
if config.PushedAuthorizationRequestURIPrefix == "" {
config.PushedAuthorizationRequestURIPrefix = defaultPARPrefix
}

if config.PushedAuthorizationContextLifespan <= 0 {
config.PushedAuthorizationContextLifespan = defaultPARContextLifetime
}
}
17 changes: 17 additions & 0 deletions compose/compose_par.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package compose

import (
"github.com/ory/fosite/handler/par"
)

// PushedAuthorizeHandlerFactory creates the basic PAR handler
func PushedAuthorizeHandlerFactory(config *Config, storage interface{}, strategy interface{}) interface{} {
return &par.PushedAuthorizeHandler{
Storage: storage,
RequestURIPrefix: config.PushedAuthorizationRequestURIPrefix,
PARContextLifetime: config.PushedAuthorizationContextLifespan,
ScopeStrategy: config.GetScopeStrategy(),
AudienceMatchingStrategy: config.GetAudienceStrategy(),
IsRedirectURISecure: config.GetRedirectSecureChecker(),
}
}
10 changes: 10 additions & 0 deletions compose/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ type Config struct {

// FormPostHTMLTemplate sets html template for rendering the authorization response when the request has response_mode=form_post.
FormPostHTMLTemplate *template.Template

// PushedAuthorizationRequestURIPrefix is the URI prefix for the PAR request_uri.
// This is defaulted to 'urn:ietf:params:oauth:request_uri:'.
PushedAuthorizationRequestURIPrefix string

// PushedAuthorizationContextLifespan is the lifespan of the PAR context
PushedAuthorizationContextLifespan time.Duration

// EnforcePushedAuthorization enforces pushed authorization request for /authorize
EnforcePushedAuthorization bool
}

// GetScopeStrategy returns the scope strategy to be used. Defaults to glob scope strategy.
Expand Down
2 changes: 2 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ const (
AccessResponseContextKey = ContextKey("accessResponse")
AuthorizeRequestContextKey = ContextKey("authorizeRequest")
AuthorizeResponseContextKey = ContextKey("authorizeResponse")
// PushedAuthorizeResponseContextKey is the response context
PushedAuthorizeResponseContextKey = ContextKey("pushedAuthorizeResponse")
)
48 changes: 37 additions & 11 deletions fosite.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"html/template"
"net/http"
"reflect"
"time"

"github.com/ory/fosite/i18n"
)
Expand Down Expand Up @@ -85,19 +86,34 @@ func (t *RevocationHandlers) Append(h RevocationHandler) {
*t = append(*t, h)
}

// PushedAuthorizeEndpointHandlers is a list of PushedAuthorizeEndpointHandler
type PushedAuthorizeEndpointHandlers []PushedAuthorizeEndpointHandler

// Append adds an AuthorizeEndpointHandler to this list. Ignores duplicates based on reflect.TypeOf.
func (a *PushedAuthorizeEndpointHandlers) Append(h PushedAuthorizeEndpointHandler) {
aeneasr marked this conversation as resolved.
Show resolved Hide resolved
for _, this := range *a {
if reflect.TypeOf(this) == reflect.TypeOf(h) {
return
}
}

*a = append(*a, h)
}

// Fosite implements OAuth2Provider.
type Fosite struct {
Store Storage
AuthorizeEndpointHandlers AuthorizeEndpointHandlers
TokenEndpointHandlers TokenEndpointHandlers
TokenIntrospectionHandlers TokenIntrospectionHandlers
RevocationHandlers RevocationHandlers
Hasher Hasher
ScopeStrategy ScopeStrategy
AudienceMatchingStrategy AudienceMatchingStrategy
JWKSFetcherStrategy JWKSFetcherStrategy
HTTPClient *http.Client
UseLegacyErrorFormat bool
Store Storage
AuthorizeEndpointHandlers AuthorizeEndpointHandlers
TokenEndpointHandlers TokenEndpointHandlers
TokenIntrospectionHandlers TokenIntrospectionHandlers
RevocationHandlers RevocationHandlers
PushedAuthorizeEndpointHandlers PushedAuthorizeEndpointHandlers
Hasher Hasher
ScopeStrategy ScopeStrategy
AudienceMatchingStrategy AudienceMatchingStrategy
JWKSFetcherStrategy JWKSFetcherStrategy
HTTPClient *http.Client
UseLegacyErrorFormat bool

// TokenURL is the the URL of the Authorization Server's Token Endpoint.
TokenURL string
Expand All @@ -120,6 +136,16 @@ type Fosite struct {

// MessageCatalog is the catalog of messages used for i18n
MessageCatalog i18n.MessageCatalog

// PushedAuthorizationRequestURIPrefix is the URI prefix for the PAR request_uri.
// This is defaulted to 'urn:ietf:params:oauth:request_uri:'.
PushedAuthorizationRequestURIPrefix string

// PushedAuthorizationContextLifespan is the lifespan of the PAR context
PushedAuthorizationContextLifespan time.Duration

// EnforcePushedAuthorization enforces pushed authorization request for /authorize
EnforcePushedAuthorization bool
}

const MinParameterEntropy = 8
Expand Down
10 changes: 10 additions & 0 deletions fosite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

. "github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/par"
)

func TestAuthorizeEndpointHandlers(t *testing.T) {
Expand Down Expand Up @@ -64,6 +65,15 @@ func TestAuthorizedRequestValidators(t *testing.T) {
assert.Equal(t, hs[0], h)
}

func TestPushedAuthorizedRequestHandlers(t *testing.T) {
h := &par.PushedAuthorizeHandler{}
hs := PushedAuthorizeEndpointHandlers{}
hs.Append(h)
hs.Append(h)
require.Len(t, hs, 1)
assert.Equal(t, hs[0], h)
}

func TestMinParameterEntropy(t *testing.T) {
f := Fosite{}
assert.Equal(t, MinParameterEntropy, f.GetMinParameterEntropy())
Expand Down
8 changes: 8 additions & 0 deletions handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,11 @@ type RevocationHandler interface {
// RevokeToken handles access and refresh token revocation.
RevokeToken(ctx context.Context, token string, tokenType TokenType, client Client) error
}

// PushedAuthorizeEndpointHandler is the interface that handles PAR (https://datatracker.ietf.org/doc/html/rfc9126)
type PushedAuthorizeEndpointHandler interface {
// HandlePushedAuthorizeRequest handles a pushed authorize endpoint request. To extend the handler's capabilities, the http request
// is passed along, if further information retrieval is required. If the handler feels that he is not responsible for
// the pushed authorize request, he must return nil and NOT modify session nor responder neither requester.
HandlePushedAuthorizeEndpointRequest(ctx context.Context, requester AuthorizeRequester, responder PushedAuthorizeResponder) error
}
Loading