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 18 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This library considered and implemented:
- [Proof Key for Code Exchange by OAuth Public Clients](https://tools.ietf.org/html/rfc7636)
- [OAuth 2.0 for Native Apps](https://tools.ietf.org/html/rfc8252)
- [OpenID Connect Core 1.0](https://openid.net/specs/openid-connect-core-1_0.html)
- [OAuth 2.0 Pushed Authorization Request](https://datatracker.ietf.org/doc/html/rfc9126)

OAuth2 and OpenID Connect are difficult protocols. If you want quick wins, we
strongly encourage you to look at [Hydra](https://github.com/ory-am/hydra).
Expand Down
72 changes: 70 additions & 2 deletions authorize_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func wrapSigningKeyFailure(outer *RFC6749Error, inner error) *RFC6749Error {
return outer
}

func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(ctx context.Context, request *AuthorizeRequest) error {
func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(ctx context.Context, 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 @@ -155,6 +155,12 @@ func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(ctx context.
}

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 @@ -272,7 +278,57 @@ 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) {
configProvider, ok := f.Config.(PushedAuthorizeRequestConfigProvider)
if !ok {
// If the config provider is not implemented, PAR cannot be used.
return false, nil
}

requestURI := r.Form.Get("request_uri")
if requestURI == "" || !strings.HasPrefix(requestURI, configProvider.GetPushedAuthorizeRequestURIPrefix(ctx)) {
// 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.WithDebug("Request failed because the 'PARStorage' interface is 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.Config.GetMessageCatalog(ctx), r)

Expand All @@ -287,6 +343,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 configProvider, ok := f.Config.(PushedAuthorizeRequestConfigProvider); ok && configProvider.EnforcePushedAuthorize(ctx) {
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 @@ -298,7 +366,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(ctx, request); err != nil {
if err := f.authorizeRequestParametersFromOpenIDConnectRequest(ctx, 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 @@ -213,7 +213,7 @@ func TestAuthorizeRequestParametersFromOpenIDConnectRequest(t *testing.T) {
},
}

err := f.authorizeRequestParametersFromOpenIDConnectRequest(context.Background(), req)
err := f.authorizeRequestParametersFromOpenIDConnectRequest(context.Background(), req, false)
aeneasr marked this conversation as resolved.
Show resolved Hide resolved
if tc.expectErr != nil {
require.EqualError(t, err, tc.expectErr.Error(), "%+v", err)
if tc.expectErrReason != "" {
Expand Down
5 changes: 4 additions & 1 deletion compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ type Factory func(config fosite.Configurator, storage interface{}, strategy inte
// Compose makes use of interface{} types in order to be able to handle a all types of stores, strategies and handlers.
func Compose(config *fosite.Config, storage interface{}, strategy interface{}, factories ...Factory) fosite.OAuth2Provider {
f := fosite.NewOAuth2Provider(storage.(fosite.Storage), config)

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

return f
Expand Down Expand Up @@ -103,5 +105,6 @@ func ComposeAllEnabled(config *fosite.Config, storage interface{}, key interface
OAuth2TokenRevocationFactory,

OAuth2PKCEFactory,
PushedAuthorizeHandlerFactory,
)
}
14 changes: 14 additions & 0 deletions compose/compose_par.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package compose

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

// PushedAuthorizeHandlerFactory creates the basic PAR handler
func PushedAuthorizeHandlerFactory(config fosite.Configurator, storage interface{}, strategy interface{}) interface{} {
return &par.PushedAuthorizeHandler{
Storage: storage,
Config: config,
}
}
22 changes: 22 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ type RevocationHandlersProvider interface {
GetRevocationHandlers(ctx context.Context) RevocationHandlers
}

// PushedAuthorizeEndpointHandlersProvider returns the provider for configuring the PAR handlers.
type PushedAuthorizeRequestHandlersProvider interface {
// GetPushedAuthorizeEndpointHandlers returns the handlers.
GetPushedAuthorizeEndpointHandlers(ctx context.Context) PushedAuthorizeEndpointHandlers
}

// UseLegacyErrorFormatProvider returns the provider for configuring whether to use the legacy error format.
//
// DEPRECATED: Do not use this flag anymore.
Expand All @@ -275,3 +281,19 @@ type UseLegacyErrorFormatProvider interface {
// DEPRECATED: Do not use this flag anymore.
GetUseLegacyErrorFormat(ctx context.Context) bool
}

// PushedAuthorizeRequestConfigProvider is the configuration provider for pushed
// authorization request.
type PushedAuthorizeRequestConfigProvider interface {
// GetPushedAuthorizeRequestURIPrefix is the request URI prefix. This is
// usually 'urn:ietf:params:oauth:request_uri:'.
GetPushedAuthorizeRequestURIPrefix(ctx context.Context) string

// GetPushedAuthorizeContextLifespan is the lifespan of the short-lived PAR context.
GetPushedAuthorizeContextLifespan(ctx context.Context) time.Duration

// EnforcePushedAuthorize indicates if PAR is enforced. In this mode, a client
// cannot pass authorize parameters at the 'authorize' endpoint. The 'authorize' endpoint
// must contain the PAR request_uri.
EnforcePushedAuthorize(ctx context.Context) bool
}
51 changes: 51 additions & 0 deletions config_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ import (
"github.com/ory/fosite/i18n"
)

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

var (
_ AuthorizeCodeLifespanProvider = (*Config)(nil)
_ RefreshTokenLifespanProvider = (*Config)(nil)
Expand Down Expand Up @@ -73,6 +78,8 @@ var (
_ TokenEndpointHandlersProvider = (*Config)(nil)
_ TokenIntrospectionHandlersProvider = (*Config)(nil)
_ RevocationHandlersProvider = (*Config)(nil)
_ PushedAuthorizeRequestHandlersProvider = (*Config)(nil)
_ PushedAuthorizeRequestConfigProvider = (*Config)(nil)
)

type Config struct {
Expand Down Expand Up @@ -202,6 +209,9 @@ type Config struct {
// RevocationHandlers is a list of handlers that are called before the revocation endpoint is served.
RevocationHandlers RevocationHandlers

// PushedAuthorizeEndpointHandlers is a list of handlers that are called before the PAR endpoint is served.
PushedAuthorizeEndpointHandlers PushedAuthorizeEndpointHandlers

// GlobalSecret is the global secret used to sign and verify signatures.
GlobalSecret []byte

Expand All @@ -210,6 +220,16 @@ type Config struct {

// HMACHasher is the hasher used to generate HMAC signatures.
HMACHasher func() hash.Hash

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

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

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

func (c *Config) GetGlobalSecret(ctx context.Context) []byte {
Expand Down Expand Up @@ -455,3 +475,34 @@ func (c *Config) GetClientAuthenticationStrategy(_ context.Context) ClientAuthen
func (c *Config) GetDisableRefreshTokenValidation(_ context.Context) bool {
return c.DisableRefreshTokenValidation
}

// GetPushedAuthorizeEndpointHandlers returns the handlers.
func (c *Config) GetPushedAuthorizeEndpointHandlers(ctx context.Context) PushedAuthorizeEndpointHandlers {
return c.PushedAuthorizeEndpointHandlers
}

// GetPushedAuthorizeRequestURIPrefix is the request URI prefix. This is
// usually 'urn:ietf:params:oauth:request_uri:'.
func (c *Config) GetPushedAuthorizeRequestURIPrefix(ctx context.Context) string {
if c.PushedAuthorizeRequestURIPrefix == "" {
return defaultPARPrefix
}

return c.PushedAuthorizeRequestURIPrefix
}

// GetPushedAuthorizeContextLifespan is the lifespan of the short-lived PAR context.
func (c *Config) GetPushedAuthorizeContextLifespan(ctx context.Context) time.Duration {
if c.PushedAuthorizeContextLifespan <= 0 {
return defaultPARContextLifetime
}

return c.PushedAuthorizeContextLifespan
}

// EnforcePushedAuthorize indicates if PAR is enforced. In this mode, a client
// cannot pass authorize parameters at the 'authorize' endpoint. The 'authorize' endpoint
// must contain the PAR request_uri.
func (c *Config) EnforcePushedAuthorize(ctx context.Context) bool {
return c.IsPushedAuthorizeEnforced
}
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")
)
14 changes: 14 additions & 0 deletions fosite.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,20 @@ 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)
}

var _ OAuth2Provider = (*Fosite)(nil)

type Configurator interface {
Expand Down
10 changes: 10 additions & 0 deletions fosite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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 @@ -65,6 +66,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{Config: new(Config)}
assert.Equal(t, MinParameterEntropy, f.GetMinParameterEntropy(context.Background()))
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