Skip to content

Commit

Permalink
refactor: utilize constants
Browse files Browse the repository at this point in the history
  • Loading branch information
@james-d-elliott
james-d-elliott committed Dec 21, 2023
1 parent 6584d34 commit 8bee5d5
Show file tree
Hide file tree
Showing 36 changed files with 412 additions and 314 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ following list of differences:
<sup>[commit](https://github.com/authelia/oauth2-provider/commit/2314625eb1f21987a9199fb1cdf6da6cee4df965)</sup>
- [x] RFC9068 must condition ignored
<sup>[commit](https://github.com/authelia/oauth2-provider/commit/f4652d60c850d167da00e2d2fe9096776eff9465)</sup>
- [ ] Arguments are treated as case-insensitive
- Refresh Flow:
- [x] Requested scope ignored
- [x] Original request id not set early enough
Expand Down
7 changes: 4 additions & 3 deletions authorize_request_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/http"
"strings"

"authelia.com/provider/oauth2/internal/consts"
"github.com/go-jose/go-jose/v3"
"github.com/pkg/errors"

Expand All @@ -33,7 +34,7 @@ func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(ctx context.
// Even if a scope parameter is present in the Request Object value, a scope parameter MUST always be passed using
// the OAuth 2.0 request syntax containing the openid scope value to indicate to the underlying OAuth 2.0 logic that this is an OpenID Connect request.
// Source: http://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth
if !scope.Has("openid") {
if !scope.Has(consts.ScopeOpenID) {
return nil
}

Expand Down Expand Up @@ -168,7 +169,7 @@ func (f *Fosite) validateAuthorizeRedirectURI(_ *http.Request, request *Authoriz
// Hybrid Flow - https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthRequest
//
// Note: as per the Hybrid Flow documentation the Hybrid Flow has the same requirements as the Authorization Code Flow.
if len(rawRedirURI) == 0 && request.GetRequestedScopes().Has("openid") {
if len(rawRedirURI) == 0 && request.GetRequestedScopes().Has(consts.ScopeOpenID) {
return errorsx.WithStack(ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
}

Expand All @@ -184,7 +185,7 @@ func (f *Fosite) validateAuthorizeRedirectURI(_ *http.Request, request *Authoriz
}

func (f *Fosite) parseAuthorizeScope(_ *http.Request, request *AuthorizeRequest) error {
request.SetRequestedScopes(RemoveEmpty(strings.Split(request.Form.Get("scope"), " ")))
request.SetRequestedScopes(RemoveEmpty(strings.Split(request.Form.Get(consts.FormParameterScope), " ")))

return nil
}
Expand Down
57 changes: 29 additions & 28 deletions authorize_request_handler_oidc_request_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"net/url"
"testing"

"authelia.com/provider/oauth2/internal/consts"
"github.com/go-jose/go-jose/v3"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -95,95 +96,95 @@ func TestAuthorizeRequestParametersFromOpenIDConnectRequest(t *testing.T) {
},
{
d: "should pass because no request context given",
form: url.Values{"scope": {"openid"}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
expectErr: nil,
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should pass because request context given but not openid",
form: url.Values{"request": {"foo"}},
form: url.Values{consts.FormParameterRequest: {"foo"}},
expectErr: nil,
expectForm: url.Values{"request": {"foo"}},
expectForm: url.Values{consts.FormParameterRequest: {"foo"}},
},
{
d: "should fail because not an OpenIDConnect compliant client",
form: url.Values{"scope": {"openid"}, "request": {"foo"}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {"foo"}},
expectErr: ErrRequestNotSupported,
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should fail because not an OpenIDConnect compliant client",
form: url.Values{"scope": {"openid"}, "request_uri": {"foo"}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequestURI: {"foo"}},
expectErr: ErrRequestURINotSupported,
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should fail because token invalid an no key set",
form: url.Values{"scope": {"openid"}, "request_uri": {"foo"}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequestURI: {"foo"}},
client: &DefaultOpenIDConnectClient{RequestObjectSigningAlgorithm: "RS256"},
expectErr: ErrInvalidRequest,
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should fail because token invalid",
form: url.Values{"scope": {"openid"}, "request": {"foo"}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {"foo"}},
client: &DefaultOpenIDConnectClient{JSONWebKeys: jwks, RequestObjectSigningAlgorithm: "RS256"},
expectErr: ErrInvalidRequestObject,
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should fail because kid does not exist",
form: url.Values{"scope": {"openid"}, "request": {mustGenerateAssertion(t, jwt.MapClaims{}, key, "does-not-exists")}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {mustGenerateAssertion(t, jwt.MapClaims{}, key, "does-not-exists")}},
client: &DefaultOpenIDConnectClient{JSONWebKeys: jwks, RequestObjectSigningAlgorithm: "RS256"},
expectErr: ErrInvalidRequestObject,
expectErrReason: "Unable to retrieve RSA signing key from OAuth 2.0 Client. The JSON Web Token uses signing key with kid 'does-not-exists', which could not be found.",
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should fail because not RS256 token",
form: url.Values{"scope": {"openid"}, "request": {mustGenerateHSAssertion(t, jwt.MapClaims{})}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {mustGenerateHSAssertion(t, jwt.MapClaims{})}},
client: &DefaultOpenIDConnectClient{JSONWebKeys: jwks, RequestObjectSigningAlgorithm: "RS256"},
expectErr: ErrInvalidRequestObject,
expectErrReason: "The request object uses signing algorithm 'HS256', but the requested OAuth 2.0 Client enforces signing algorithm 'RS256'.",
expectForm: url.Values{"scope": {"openid"}},
expectForm: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}},
},
{
d: "should pass and set request parameters properly",
form: url.Values{"scope": {"openid"}, "response_type": {"code"}, "response_mode": {"none"}, "request": {validRequestObject}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterResponseType: {consts.ResponseTypeAuthorizationCodeFlow}, consts.FormParameterResponseMode: {consts.ResponseModeNone}, consts.FormParameterRequest: {validRequestObject}},
client: &DefaultOpenIDConnectClient{JSONWebKeys: jwks, RequestObjectSigningAlgorithm: "RS256"},
// The values from form are overwritten by the request object.
expectForm: url.Values{"response_type": {"token"}, "response_mode": {"post_form"}, "scope": {"foo openid"}, "request": {validRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{consts.FormParameterResponseType: {consts.ResponseTypeImplicitFlowToken}, consts.FormParameterResponseMode: {"post_form"}, consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequest: {validRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
},
{
d: "should pass even if kid is unset",
form: url.Values{"scope": {"openid"}, "request": {validRequestObjectWithoutKid}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {validRequestObjectWithoutKid}},
client: &DefaultOpenIDConnectClient{JSONWebKeys: jwks, RequestObjectSigningAlgorithm: "RS256"},
expectForm: url.Values{"scope": {"foo openid"}, "request": {validRequestObjectWithoutKid}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequest: {validRequestObjectWithoutKid}, "foo": {"bar"}, "baz": {"baz"}},
},
{
d: "should fail because request uri is not whitelisted",
form: url.Values{"scope": {"openid"}, "request_uri": {reqTS.URL}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequestURI: {reqTS.URL}},
client: &DefaultOpenIDConnectClient{JSONWebKeysURI: reqJWK.URL, RequestObjectSigningAlgorithm: "RS256"},
expectForm: url.Values{"scope": {"foo openid"}, "request_uri": {reqTS.URL}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequestURI: {reqTS.URL}, "foo": {"bar"}, "baz": {"baz"}},
expectErr: ErrInvalidRequestURI,
},
{
d: "should pass and set request_uri parameters properly and also fetch jwk from remote",
form: url.Values{"scope": {"openid"}, "request_uri": {reqTS.URL}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequestURI: {reqTS.URL}},
client: &DefaultOpenIDConnectClient{JSONWebKeysURI: reqJWK.URL, RequestObjectSigningAlgorithm: "RS256", RequestURIs: []string{reqTS.URL}},
expectForm: url.Values{"response_type": {"token"}, "response_mode": {"post_form"}, "scope": {"foo openid"}, "request_uri": {reqTS.URL}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{"response_type": {"token"}, "response_mode": {"post_form"}, consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequestURI: {reqTS.URL}, "foo": {"bar"}, "baz": {"baz"}},
},
{
d: "should pass when request object uses algorithm none",
form: url.Values{"scope": {"openid"}, "request": {validNoneRequestObject}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {validNoneRequestObject}},
client: &DefaultOpenIDConnectClient{JSONWebKeysURI: reqJWK.URL, RequestObjectSigningAlgorithm: "none"},
expectForm: url.Values{"state": {"some-state"}, "scope": {"foo openid"}, "request": {validNoneRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{consts.FormParameterState: {"some-state"}, consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequest: {validNoneRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
},
{
d: "should pass when request object uses algorithm none and the client did not explicitly allow any algorithm",
form: url.Values{"scope": {"openid"}, "request": {validNoneRequestObject}},
form: url.Values{consts.FormParameterScope: {consts.ScopeOpenID}, consts.FormParameterRequest: {validNoneRequestObject}},
client: &DefaultOpenIDConnectClient{JSONWebKeysURI: reqJWK.URL},
expectForm: url.Values{"state": {"some-state"}, "scope": {"foo openid"}, "request": {validNoneRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
expectForm: url.Values{consts.FormParameterState: {"some-state"}, consts.FormParameterScope: {"foo openid"}, consts.FormParameterRequest: {validNoneRequestObject}, "foo": {"bar"}, "baz": {"baz"}},
},
} {
t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
Expand Down
27 changes: 15 additions & 12 deletions handler/oauth2/flow_authorize_code_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,10 @@ import (
"time"

"authelia.com/provider/oauth2"
"authelia.com/provider/oauth2/internal/consts"
"authelia.com/provider/oauth2/internal/errorsx"
)

var (
_ oauth2.AuthorizeEndpointHandler = (*AuthorizeExplicitGrantHandler)(nil)
_ oauth2.TokenEndpointHandler = (*AuthorizeExplicitGrantHandler)(nil)
)

// AuthorizeExplicitGrantHandler is a response handler for the Authorize Code grant using the explicit grant type
// as defined in https://datatracker.ietf.org/doc/html/rfc6749#section-4.1
type AuthorizeExplicitGrantHandler struct {
Expand All @@ -39,6 +35,11 @@ type AuthorizeExplicitGrantHandler struct {
}
}

var (
_ oauth2.AuthorizeEndpointHandler = (*AuthorizeExplicitGrantHandler)(nil)
_ oauth2.TokenEndpointHandler = (*AuthorizeExplicitGrantHandler)(nil)
)

func (c *AuthorizeExplicitGrantHandler) secureChecker(ctx context.Context) func(context.Context, *url.URL) bool {
if c.Config.GetRedirectSecureChecker(ctx) == nil {
return oauth2.IsRedirectURISecure
Expand All @@ -48,7 +49,7 @@ func (c *AuthorizeExplicitGrantHandler) secureChecker(ctx context.Context) func(

func (c *AuthorizeExplicitGrantHandler) HandleAuthorizeEndpointRequest(ctx context.Context, ar oauth2.AuthorizeRequester, resp oauth2.AuthorizeResponder) error {
// This let's us define multiple response types, for example open id connect's id_token
if !ar.GetResponseTypes().ExactOne("code") {
if !ar.GetResponseTypes().ExactOne(consts.ResponseTypeAuthorizationCodeFlow) {
return nil
}

Expand Down Expand Up @@ -84,17 +85,19 @@ func (c *AuthorizeExplicitGrantHandler) IssueAuthorizeCode(ctx context.Context,
}

ar.GetSession().SetExpiresAt(oauth2.AuthorizeCode, time.Now().UTC().Add(c.Config.GetAuthorizeCodeLifespan(ctx)))
if err := c.CoreStorage.CreateAuthorizeCodeSession(ctx, signature, ar.Sanitize(c.GetSanitationWhiteList(ctx))); err != nil {

if err = c.CoreStorage.CreateAuthorizeCodeSession(ctx, signature, ar.Sanitize(c.GetSanitationWhiteList(ctx))); err != nil {
return errorsx.WithStack(oauth2.ErrServerError.WithWrap(err).WithDebug(err.Error()))
}

resp.AddParameter("code", code)
resp.AddParameter("state", ar.GetState())
resp.AddParameter(consts.FormParameterAuthorizationCode, code)
resp.AddParameter(consts.FormParameterState, ar.GetState())
if !c.Config.GetOmitRedirectScopeParam(ctx) {
resp.AddParameter("scope", strings.Join(ar.GetGrantedScopes(), " "))
resp.AddParameter(consts.FormParameterScope, strings.Join(ar.GetGrantedScopes(), " "))
}

ar.SetResponseTypeHandled("code")
ar.SetResponseTypeHandled(consts.ResponseTypeAuthorizationCodeFlow)

return nil
}

Expand All @@ -103,5 +106,5 @@ func (c *AuthorizeExplicitGrantHandler) GetSanitationWhiteList(ctx context.Conte
return allowedList
}

return []string{"code", "redirect_uri"}
return []string{consts.FormParameterScope, consts.FormParameterRedirectURI}
}
29 changes: 15 additions & 14 deletions handler/oauth2/flow_authorize_code_auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"
"time"

"authelia.com/provider/oauth2/internal/consts"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -62,10 +63,10 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
{
handler: handler,
areq: &oauth2.AuthorizeRequest{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
Request: oauth2.Request{
Client: &oauth2.DefaultClient{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
RedirectURIs: []string{"http://asdf.com/cb"},
},
},
Expand All @@ -77,10 +78,10 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
{
handler: handler,
areq: &oauth2.AuthorizeRequest{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
Request: oauth2.Request{
Client: &oauth2.DefaultClient{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
RedirectURIs: []string{"https://asdf.com/cb"},
Audience: []string{"https://www.authelia.com/api"},
},
Expand All @@ -94,10 +95,10 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
{
handler: handler,
areq: &oauth2.AuthorizeRequest{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
Request: oauth2.Request{
Client: &oauth2.DefaultClient{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
RedirectURIs: []string{"https://asdf.de/cb"},
Audience: []string{"https://www.authelia.com/api"},
},
Expand All @@ -113,11 +114,11 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
},
description: "should pass",
expect: func(t *testing.T, areq *oauth2.AuthorizeRequest, aresp *oauth2.AuthorizeResponse) {
code := aresp.GetParameters().Get("code")
code := aresp.GetParameters().Get(consts.FormParameterAuthorizationCode)
assert.NotEmpty(t, code)

assert.Equal(t, strings.Join(areq.GrantedScope, " "), aresp.GetParameters().Get("scope"))
assert.Equal(t, areq.State, aresp.GetParameters().Get("state"))
assert.Equal(t, strings.Join(areq.GrantedScope, " "), aresp.GetParameters().Get(consts.FormParameterScope))
assert.Equal(t, areq.State, aresp.GetParameters().Get(consts.FormParameterState))
assert.Equal(t, oauth2.ResponseModeQuery, areq.GetResponseMode())
},
},
Expand All @@ -132,10 +133,10 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
},
},
areq: &oauth2.AuthorizeRequest{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
Request: oauth2.Request{
Client: &oauth2.DefaultClient{
ResponseTypes: oauth2.Arguments{"code"},
ResponseTypes: oauth2.Arguments{consts.ResponseTypeAuthorizationCodeFlow},
RedirectURIs: []string{"https://asdf.de/cb"},
Audience: []string{"https://www.authelia.com/api"},
},
Expand All @@ -151,11 +152,11 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
},
description: "should pass but no scope in redirect uri",
expect: func(t *testing.T, areq *oauth2.AuthorizeRequest, aresp *oauth2.AuthorizeResponse) {
code := aresp.GetParameters().Get("code")
code := aresp.GetParameters().Get(consts.FormParameterAuthorizationCode)
assert.NotEmpty(t, code)

assert.Empty(t, aresp.GetParameters().Get("scope"))
assert.Equal(t, areq.State, aresp.GetParameters().Get("state"))
assert.Empty(t, aresp.GetParameters().Get(consts.FormParameterScope))
assert.Equal(t, areq.State, aresp.GetParameters().Get(consts.FormParameterState))
assert.Equal(t, oauth2.ResponseModeQuery, areq.GetResponseMode())
},
},
Expand Down
Loading

0 comments on commit 8bee5d5

Please sign in to comment.