diff --git a/oidc/core/authorization_code/flow.go b/oidc/core/authorization_code/flow.go index c82e999..64780fc 100644 --- a/oidc/core/authorization_code/flow.go +++ b/oidc/core/authorization_code/flow.go @@ -42,32 +42,29 @@ func Must(cfg *Config) (*Flow, error) { } // ValidateAuthorizationRequest validates OIDC-specific parameters in the -// authorization request. It is a no-op when the openid scope is absent. +// /authorize request. It is a no-op when the openid scope is absent. func (f *Flow) ValidateAuthorizationRequest(r *requests.AuthorizationRequest) error { if isOIDCReq := r.Scopes.ContainOpenID(); !isOIDCReq { return nil } - if err := r.ValidateDisplay(false); err != nil { - return err - } - if err := f.validateNonce(r); err != nil { return err } - if err := f.validatePrompt(r); err != nil { - return err - } - return nil } -// ValidateConsentRequest re-runs authorization request validation then enforces -// prompt and user-presence rules. When prompt is absent and user is nil, it -// defaults to prompt=login so the handler can redirect to the login page. +// ValidateConsentRequest validates OIDC prompt rules at the consent step. +// It is a no-op when the openid scope is absent. When prompt is absent and +// user is nil, defaults to prompt=login so the handler can redirect to the +// login page. func (f *Flow) ValidateConsentRequest(r *requests.AuthorizationRequest) error { - if err := f.ValidateAuthorizationRequest(r); err != nil { + if isOIDCReq := r.Scopes.ContainOpenID(); !isOIDCReq { + return nil + } + + if err := f.validatePrompt(r); err != nil { return err } @@ -129,7 +126,7 @@ func (f *Flow) ProcessToken(r *requests.TokenRequest, _ models.Token, data map[s // validateNonce checks that nonce is present (when required) and has not been // used before (when ExistNonce is configured). func (f *Flow) validateNonce(r *requests.AuthorizationRequest) error { - if err := r.ValidateNonce(f.requireNonce); err != nil { + if err := r.CheckNonce(f.requireNonce); err != nil { return err } diff --git a/oidc/core/authorization_code/flow_test.go b/oidc/core/authorization_code/flow_test.go index 634d2fc..6444071 100644 --- a/oidc/core/authorization_code/flow_test.go +++ b/oidc/core/authorization_code/flow_test.go @@ -131,31 +131,6 @@ func TestFlow_ValidateAuthorizationRequest(t *testing.T) { assert.NoError(t, f2.ValidateAuthorizationRequest(r)) }) - t.Run("prompt_none_combined_with_other_returns_error", func(t *testing.T) { - r := authReq("openid") - r.Nonce = "nonce-1" - r.Prompts = types.NewPrompts([]string{"none", "login"}) - err := f.ValidateAuthorizationRequest(r) - require.Error(t, err) - assert.Contains(t, err.Error(), "none") - }) - - t.Run("prompt_none_alone_ok", func(t *testing.T) { - f2 := New(validConfig().SetRequireNonce(false)) - r := authReq("openid") - r.Prompts = types.NewPrompts([]string{"none"}) - assert.NoError(t, f2.ValidateAuthorizationRequest(r)) - }) - - t.Run("prompt_login_resets_max_age_to_zero", func(t *testing.T) { - r := authReq("openid") - r.Nonce = "nonce-1" - r.Prompts = types.NewPrompts([]string{"login"}) - require.NoError(t, f.ValidateAuthorizationRequest(r)) - require.NotNil(t, r.MaxAge) - assert.Equal(t, uint(0), *r.MaxAge) - }) - t.Run("valid_request_with_nonce", func(t *testing.T) { r := authReq("openid", "profile") r.Nonce = "nonce-1" @@ -225,13 +200,23 @@ func TestFlow_ValidateConsentRequest(t *testing.T) { assert.NoError(t, f.ValidateConsentRequest(r)) }) - t.Run("validation_error_from_auth_request_propagates", func(t *testing.T) { - // requireNonce=true (default): missing nonce must bubble up. - f := New(validConfig()) + t.Run("prompt_none_combined_with_other_returns_error", func(t *testing.T) { + f := New(cfg) r := authReq("openid") + r.Prompts = types.NewPrompts([]string{"none", "login"}) err := f.ValidateConsentRequest(r) require.Error(t, err) - assert.Contains(t, err.Error(), "nonce") + assert.Contains(t, err.Error(), "none") + }) + + t.Run("prompt_login_resets_max_age_to_zero", func(t *testing.T) { + f := New(cfg) + r := authReq("openid") + r.User = &sql.User{UserID: "user-1"} + r.Prompts = types.NewPrompts([]string{"login"}) + require.NoError(t, f.ValidateConsentRequest(r)) + require.NotNil(t, r.MaxAge) + assert.Equal(t, uint(0), *r.MaxAge) }) } diff --git a/requests/authorization.go b/requests/authorization.go index 22476cb..0e26f10 100644 --- a/requests/authorization.go +++ b/requests/authorization.go @@ -41,10 +41,12 @@ type AuthorizationRequest struct { Request *http.Request } -// NewAuthorizationRequestFromHttp parses an authorization request from an -// HTTP request. It reads all standard OAuth 2.0 and OIDC parameters from the -// URL query string. Returns an error only if max_age is present but cannot be -// parsed as a non-negative integer. +// NewAuthorizationRequestFromHttp parses an OAuth 2.0 / OIDC authorization +// request from r. Parameters are read from the URL query string (or form +// body after r.ParseForm). Invalid or missing parameters are not validated +// here — that is left to the grant flow. The only exception is max_age: if +// present but not a valid non-negative integer, it is silently ignored and +// defaults to 0. func NewAuthorizationRequestFromHttp(r *http.Request) (*AuthorizationRequest, error) { authReq := &AuthorizationRequest{ ResponseType: types.NewResponseType(r.FormValue("response_type")), @@ -65,23 +67,20 @@ func NewAuthorizationRequestFromHttp(r *http.Request) (*AuthorizationRequest, er Request: r, } - if maxAge := r.FormValue("max_age"); maxAge != "" { - ma, err := strconv.ParseUint(maxAge, 10, 64) - if err != nil { - return nil, autherrors.InvalidRequestError(). - WithDescription("\"max_age\" must be a non-negative integer"). - WithState(authReq.State) + maxAge := types.NewMaxAge(0) + if v := r.FormValue("max_age"); v != "" { + if ma, err := strconv.ParseUint(v, 10, 64); err == nil { + maxAge = types.NewMaxAge(uint(ma)) } - - authReq.MaxAge = types.NewMaxAge(uint(ma)) } + authReq.MaxAge = maxAge return authReq, nil } -// ValidateResponseType returns an error if response_type is missing. Required +// CheckResponseType returns an error if response_type is missing. Required // by default; pass false to treat it as optional. -func (r *AuthorizationRequest) ValidateResponseType(required ...bool) error { +func (r *AuthorizationRequest) CheckResponseType(required ...bool) error { if isRequired(true, required...) && r.ResponseType.IsEmpty() { return autherrors.InvalidRequestError().WithDescription("missing \"response_type\" in request").WithState(r.State) } @@ -89,9 +88,9 @@ func (r *AuthorizationRequest) ValidateResponseType(required ...bool) error { return nil } -// ValidateClientID returns an error if client_id is missing. Required by +// CheckClientID returns an error if client_id is missing. Required by // default; pass false to treat it as optional. -func (r *AuthorizationRequest) ValidateClientID(required ...bool) error { +func (r *AuthorizationRequest) CheckClientID(required ...bool) error { if isRequired(true, required...) && r.ClientID == "" { return autherrors.InvalidRequestError(). WithDescription("missing \"client_id\" in request"). @@ -101,21 +100,9 @@ func (r *AuthorizationRequest) ValidateClientID(required ...bool) error { return nil } -// ValidateRedirectURI returns an error if redirect_uri is missing. Required -// by default; pass false to treat it as optional. -func (r *AuthorizationRequest) ValidateRedirectURI(required ...bool) error { - if isRequired(true, required...) && r.RedirectURI == "" { - return autherrors.InvalidRequestError(). - WithDescription("missing \"redirect_uri\" in request"). - WithState(r.State) - } - - return nil -} - -// ValidateNonce returns an error if nonce is missing. Required by default; +// CheckNonce returns an error if nonce is missing. Required by default; // pass false to treat it as optional. -func (r *AuthorizationRequest) ValidateNonce(required ...bool) error { +func (r *AuthorizationRequest) CheckNonce(required ...bool) error { if isRequired(true, required...) && r.Nonce == "" { return autherrors.InvalidRequestError(). WithDescription("missing \"nonce\" in request"). @@ -126,45 +113,6 @@ func (r *AuthorizationRequest) ValidateNonce(required ...bool) error { return nil } -// ValidateResponseMode returns an error if response_mode is missing. Not -// required by default; pass true to enforce it. -func (r *AuthorizationRequest) ValidateResponseMode(required ...bool) error { - if isRequired(false, required...) && r.ResponseMode.IsEmpty() { - return autherrors.InvalidRequestError(). - WithDescription("missing \"response_mode\" in request"). - WithState(r.State). - WithRedirectURI(r.RedirectURI) - } - - return nil -} - -// ValidateDisplay returns an error if display is missing. Not required by -// default; pass true to enforce it. -func (r *AuthorizationRequest) ValidateDisplay(required ...bool) error { - if isRequired(false, required...) && r.Display.IsEmpty() { - return autherrors.InvalidRequestError(). - WithDescription("missing \"display\" in request"). - WithState(r.State). - WithRedirectURI(r.RedirectURI) - } - - return nil -} - -// ValidatePrompts returns an error if prompt is missing. Not required by -// default; pass true to enforce it. -func (r *AuthorizationRequest) ValidatePrompts(required ...bool) error { - if isRequired(false, required...) && len(r.Prompts) == 0 { - return autherrors.InvalidRequestError(). - WithDescription("missing \"prompt\" in request"). - WithState(r.State). - WithRedirectURI(r.RedirectURI) - } - - return nil -} - // Method returns the HTTP method of the underlying request. func (r *AuthorizationRequest) Method() string { return r.Request.Method diff --git a/requests/authorization_test.go b/requests/authorization_test.go index 8409d37..4038c59 100644 --- a/requests/authorization_test.go +++ b/requests/authorization_test.go @@ -23,100 +23,51 @@ func TestNewAuthorizationRequestFromHttp(t *testing.T) { assert.Equal(t, types.NewMaxAge(300), req.MaxAge) }) - t.Run("invalid max_age returns error", func(t *testing.T) { + t.Run("invalid_max_age_ignored", func(t *testing.T) { r := httptest.NewRequest("GET", "/?max_age=abc", nil) req, err := NewAuthorizationRequestFromHttp(r) - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - assert.Nil(t, req) + assert.NoError(t, err) + assert.NotNil(t, req) }) } -func TestAuthorizationRequest_ValidateResponseType(t *testing.T) { +func TestAuthorizationRequest_CheckResponseType(t *testing.T) { req := &AuthorizationRequest{} - err := req.ValidateResponseType() + err := req.CheckResponseType() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) req.ResponseType = types.ResponseTypeCode - assert.NoError(t, req.ValidateResponseType()) + assert.NoError(t, req.CheckResponseType()) // optional when false is passed empty := &AuthorizationRequest{} - assert.NoError(t, empty.ValidateResponseType(false)) + assert.NoError(t, empty.CheckResponseType(false)) } -func TestAuthorizationRequest_ValidateClientID(t *testing.T) { +func TestAuthorizationRequest_CheckClientID(t *testing.T) { req := &AuthorizationRequest{} - err := req.ValidateClientID() + err := req.CheckClientID() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - assert.NoError(t, req.ValidateClientID(false)) + assert.NoError(t, req.CheckClientID(false)) req.ClientID = "myclient" - assert.NoError(t, req.ValidateClientID()) -} - -func TestAuthorizationRequest_ValidateRedirectURI(t *testing.T) { - req := &AuthorizationRequest{} - err := req.ValidateRedirectURI() - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - - assert.NoError(t, req.ValidateRedirectURI(false)) - - req.RedirectURI = "https://example.com/cb" - assert.NoError(t, req.ValidateRedirectURI()) + assert.NoError(t, req.CheckClientID()) } -func TestAuthorizationRequest_ValidateNonce(t *testing.T) { +func TestAuthorizationRequest_CheckNonce(t *testing.T) { req := &AuthorizationRequest{} // required by default - err := req.ValidateNonce() + err := req.CheckNonce() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) // optional when false is passed - assert.NoError(t, req.ValidateNonce(false)) + assert.NoError(t, req.CheckNonce(false)) req.Nonce = "mynonce" - assert.NoError(t, req.ValidateNonce()) -} - -func TestAuthorizationRequest_ValidateResponseMode(t *testing.T) { - req := &AuthorizationRequest{} - assert.NoError(t, req.ValidateResponseMode()) - - err := req.ValidateResponseMode(true) - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - - req.ResponseMode = types.NewResponseMode("query") - assert.NoError(t, req.ValidateResponseMode(true)) -} - -func TestAuthorizationRequest_ValidateDisplay(t *testing.T) { - req := &AuthorizationRequest{} - assert.NoError(t, req.ValidateDisplay()) - - err := req.ValidateDisplay(true) - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - - req.Display = types.DisplayPage - assert.NoError(t, req.ValidateDisplay(true)) -} - -func TestAuthorizationRequest_ValidatePrompts(t *testing.T) { - req := &AuthorizationRequest{} - assert.NoError(t, req.ValidatePrompts()) - - err := req.ValidatePrompts(true) - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - - req.Prompts = types.NewPrompts([]string{"login"}) - assert.NoError(t, req.ValidatePrompts(true)) + assert.NoError(t, req.CheckNonce()) } diff --git a/requests/token.go b/requests/token.go index 34951fc..2324a2d 100644 --- a/requests/token.go +++ b/requests/token.go @@ -48,8 +48,8 @@ func NewTokenRequestFromHttp(r *http.Request) *TokenRequest { } } -// ValidateGrantType returns an error if grant_type is missing or empty. -func (r *TokenRequest) ValidateGrantType() error { +// CheckGrantType returns an error if grant_type is missing or empty. +func (r *TokenRequest) CheckGrantType() error { if r.GrantType.IsEmpty() { return autherrors.InvalidRequestError().WithDescription("missing \"grant_type\" in request") } @@ -57,9 +57,9 @@ func (r *TokenRequest) ValidateGrantType() error { return nil } -// ValidateCode returns an error if code is missing. Required by default; +// CheckCode returns an error if code is missing. Required by default; // pass false to treat it as optional. -func (r *TokenRequest) ValidateCode(required ...bool) error { +func (r *TokenRequest) CheckCode(required ...bool) error { if isRequired(true, required...) && r.Code == "" { return autherrors.InvalidRequestError().WithDescription("missing \"code\" in request") } @@ -67,18 +67,8 @@ func (r *TokenRequest) ValidateCode(required ...bool) error { return nil } -// ValidateRedirectURI returns an error if redirect_uri is missing. Required -// by default; pass false to treat it as optional. -func (r *TokenRequest) ValidateRedirectURI(required ...bool) error { - if isRequired(true, required...) && r.RedirectURI == "" { - return autherrors.InvalidRequestError().WithDescription("missing \"redirect_uri\" in request") - } - - return nil -} - -// ValidateUsername returns an error if username is missing or empty. -func (r *TokenRequest) ValidateUsername() error { +// CheckUsername returns an error if username is missing or empty. +func (r *TokenRequest) CheckUsername() error { if r.Username == "" { return autherrors.InvalidRequestError().WithDescription("missing \"username\" in request") } @@ -86,8 +76,8 @@ func (r *TokenRequest) ValidateUsername() error { return nil } -// ValidatePassword returns an error if password is missing or empty. -func (r *TokenRequest) ValidatePassword() error { +// CheckPassword returns an error if password is missing or empty. +func (r *TokenRequest) CheckPassword() error { if r.Password == "" { return autherrors.InvalidRequestError().WithDescription("missing \"password\" in request") } diff --git a/requests/token_test.go b/requests/token_test.go index f3c923e..c0cf9a7 100644 --- a/requests/token_test.go +++ b/requests/token_test.go @@ -28,62 +28,47 @@ func TestNewTokenRequestFromHttp(t *testing.T) { assert.Equal(t, r, req.Request) } -func TestTokenRequest_ValidateGrantType(t *testing.T) { +func TestTokenRequest_CheckGrantType(t *testing.T) { req := &TokenRequest{} - err := req.ValidateGrantType() + err := req.CheckGrantType() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) req.GrantType = types.GrantTypeAuthorizationCode - assert.NoError(t, req.ValidateGrantType()) + assert.NoError(t, req.CheckGrantType()) } -func TestTokenRequest_ValidateCode(t *testing.T) { +func TestTokenRequest_CheckCode(t *testing.T) { req := &TokenRequest{} // required by default - err := req.ValidateCode() + err := req.CheckCode() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) // optional when false is passed - assert.NoError(t, req.ValidateCode(false)) + assert.NoError(t, req.CheckCode(false)) req.Code = "mycode" - assert.NoError(t, req.ValidateCode()) + assert.NoError(t, req.CheckCode()) } -func TestTokenRequest_ValidateRedirectURI(t *testing.T) { +func TestTokenRequest_CheckUsername(t *testing.T) { req := &TokenRequest{} - - // required by default - err := req.ValidateRedirectURI() - authErr := autherrors.ToAuthLibError(err) - assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) - - // optional when false is passed - assert.NoError(t, req.ValidateRedirectURI(false)) - - req.RedirectURI = "https://example.com/cb" - assert.NoError(t, req.ValidateRedirectURI()) -} - -func TestTokenRequest_ValidateUsername(t *testing.T) { - req := &TokenRequest{} - err := req.ValidateUsername() + err := req.CheckUsername() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) req.Username = "alice" - assert.NoError(t, req.ValidateUsername()) + assert.NoError(t, req.CheckUsername()) } -func TestTokenRequest_ValidatePassword(t *testing.T) { +func TestTokenRequest_CheckPassword(t *testing.T) { req := &TokenRequest{} - err := req.ValidatePassword() + err := req.CheckPassword() authErr := autherrors.ToAuthLibError(err) assert.Equal(t, autherrors.ErrInvalidRequest, authErr.Code) req.Password = "secret" - assert.NoError(t, req.ValidatePassword()) + assert.NoError(t, req.CheckPassword()) } diff --git a/rfc6749/authorization_code/flow.go b/rfc6749/authorization_code/flow.go index 2bdd242..c6c485f 100644 --- a/rfc6749/authorization_code/flow.go +++ b/rfc6749/authorization_code/flow.go @@ -229,7 +229,7 @@ func (f *Flow) checkTokenEndpointHttpMethod(r *requests.TokenRequest) error { // checkClient validates client_id and loads the client record into r.Client. func (f *Flow) checkClient(r *requests.AuthorizationRequest) error { - if err := r.ValidateClientID(true); err != nil { + if err := r.CheckClientID(true); err != nil { return err } @@ -274,7 +274,7 @@ func (f *Flow) validateRedirectURI(r *requests.AuthorizationRequest) error { // validateResponseType verifies response_type=code and that the client is // permitted to use this response type. func (f *Flow) validateResponseType(r *requests.AuthorizationRequest) error { - if err := r.ValidateResponseType(true); err != nil { + if err := r.CheckResponseType(true); err != nil { return err } @@ -336,7 +336,7 @@ func (f *Flow) genAuthCode(r *requests.AuthorizationRequest) (models.Authorizati // validateGrantType checks that grant_type is present and equals authorization_code. func (f *Flow) validateGrantType(r *requests.TokenRequest) error { - if err := r.ValidateGrantType(); err != nil { + if err := r.CheckGrantType(); err != nil { return err } @@ -367,7 +367,7 @@ func (f *Flow) authenticateClient(r *requests.TokenRequest) error { // validateAuthCode verifies the authorization code: existence, client binding, // expiry, and redirect_uri match (RFC 6749 §4.1.3). Populates r.AuthCode on success. func (f *Flow) validateAuthCode(r *requests.TokenRequest) error { - if err := r.ValidateCode(); err != nil { + if err := r.CheckCode(); err != nil { return err } diff --git a/rfc6749/client_credentials/flow.go b/rfc6749/client_credentials/flow.go index 5a765f5..a2f4f14 100644 --- a/rfc6749/client_credentials/flow.go +++ b/rfc6749/client_credentials/flow.go @@ -115,7 +115,7 @@ func (f *Flow) checkTokenEndpointHttpMethod(r *requests.TokenRequest) error { // validateGrantType checks that grant_type is present and equals "client_credentials". func (f *Flow) validateGrantType(r *requests.TokenRequest) error { - if err := r.ValidateGrantType(); err != nil { + if err := r.CheckGrantType(); err != nil { return err } diff --git a/rfc6749/ropc/flow.go b/rfc6749/ropc/flow.go index fa6b6ef..b3c09de 100644 --- a/rfc6749/ropc/flow.go +++ b/rfc6749/ropc/flow.go @@ -106,11 +106,11 @@ func (f *Flow) checkParams(r *requests.TokenRequest) error { return err } - if err := r.ValidateUsername(); err != nil { + if err := r.CheckUsername(); err != nil { return err } - if err := r.ValidatePassword(); err != nil { + if err := r.CheckPassword(); err != nil { return err } @@ -131,7 +131,7 @@ func (f *Flow) checkTokenEndpointHttpMethod(r *requests.TokenRequest) error { // validateGrantType checks that grant_type is present and equals "password". func (f *Flow) validateGrantType(r *requests.TokenRequest) error { - if err := r.ValidateGrantType(); err != nil { + if err := r.CheckGrantType(); err != nil { return err }