Skip to content

chore: Use constants for all errors #2676

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.

Sign up for GitHub

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

Open
TobCar wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: Use constants for all errors #2676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 18 additions & 1 deletion internal/api/apierrors/apierrors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@ import (
"net/http"
)

// OAuthErrorCode is a value for the "error" field of an OAuth2 error response, defined in RFC 6749.
type OAuthErrorCode = string

const (
// Error codes for the authorization endpoint (RFC 6749 Section 4.1.2.1) and the token endpoint (RFC 6749 Section 5.2).
OAuthErrorCodeInvalidRequest OAuthErrorCode = "invalid_request"
OAuthErrorCodeInvalidClient OAuthErrorCode = "invalid_client"
OAuthErrorCodeInvalidGrant OAuthErrorCode = "invalid_grant"
OAuthErrorCodeUnauthorizedClient OAuthErrorCode = "unauthorized_client"
OAuthErrorCodeUnsupportedGrantType OAuthErrorCode = "unsupported_grant_type"
OAuthErrorCodeInvalidScope OAuthErrorCode = "invalid_scope"
OAuthErrorCodeAccessDenied OAuthErrorCode = "access_denied"
OAuthErrorCodeUnsupportedResponseType OAuthErrorCode = "unsupported_response_type"
OAuthErrorCodeServerError OAuthErrorCode = "server_error"
OAuthErrorCodeTemporarilyUnavailable OAuthErrorCode = "temporarily_unavailable"
)

// OAuthError is the JSON handler for OAuth2 error responses
type OAuthError struct {
Err string `json:"error"`
Expand All @@ -13,7 +30,7 @@ type OAuthError struct {
InternalMessage string `json:"-"`
}

func NewOAuthError(err string, description string) *OAuthError {
func NewOAuthError(err OAuthErrorCode, description string) *OAuthError {
return &OAuthError{Err: err, Description: description}
}

Expand Down
10 changes: 5 additions & 5 deletions internal/api/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ var (
const InvalidChannelError = "Invalid channel, supported values are 'sms' or 'whatsapp'. 'whatsapp' is only supported if Twilio or Twilio Verify is used as the provider."

var oauthErrorMap = map[int]string{
http.StatusBadRequest: "invalid_request",
http.StatusUnauthorized: "unauthorized_client",
http.StatusForbidden: "access_denied",
http.StatusInternalServerError: "server_error",
http.StatusServiceUnavailable: "temporarily_unavailable",
http.StatusBadRequest: apierrors.OAuthErrorCodeInvalidRequest,
http.StatusUnauthorized: apierrors.OAuthErrorCodeUnauthorizedClient,
http.StatusForbidden: apierrors.OAuthErrorCodeAccessDenied,
http.StatusInternalServerError: apierrors.OAuthErrorCodeServerError,
http.StatusServiceUnavailable: apierrors.OAuthErrorCodeTemporarilyUnavailable,
}

// Type aliases while we slowly refactor api errors.
Expand Down
12 changes: 6 additions & 6 deletions internal/api/external.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
}

if terr != nil {
return apierrors.NewOAuthError("server_error", terr.Error())
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeServerError, terr.Error())
}
return nil
})
Expand Down Expand Up @@ -832,15 +832,15 @@ func getErrorQueryString(err error, errorID string, log logrus.FieldLogger, q ur
switch e := err.(type) {
case *HTTPError:
if e.ErrorCode == apierrors.ErrorCodeSignupDisabled {
q.Set("error", "access_denied")
q.Set("error", apierrors.OAuthErrorCodeAccessDenied)
} else if e.ErrorCode == apierrors.ErrorCodeUserBanned {
q.Set("error", "access_denied")
q.Set("error", apierrors.OAuthErrorCodeAccessDenied)
} else if e.ErrorCode == apierrors.ErrorCodeProviderEmailNeedsVerification {
q.Set("error", "access_denied")
q.Set("error", apierrors.OAuthErrorCodeAccessDenied)
} else if str, ok := oauthErrorMap[e.HTTPStatus]; ok {
q.Set("error", str)
} else {
q.Set("error", "server_error")
q.Set("error", apierrors.OAuthErrorCodeServerError)
}
if e.HTTPStatus >= http.StatusInternalServerError {
e.ErrorID = errorID
Expand All @@ -858,7 +858,7 @@ func getErrorQueryString(err error, errorID string, log logrus.FieldLogger, q ur
case ErrorCause:
return getErrorQueryString(e.Cause(), errorID, log, q)
default:
error_type, error_description := "server_error", err.Error()
error_type, error_description := apierrors.OAuthErrorCodeServerError, err.Error()

// Provide better error messages for certain user-triggered Postgres errors.
if pgErr := utilities.NewPostgresError(e); pgErr != nil {
Expand Down
15 changes: 4 additions & 11 deletions internal/api/oauthserver/authorize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,6 @@ const (
OAuthServerConsentActionDeny OAuthServerConsentAction = "deny"
)

// OAuth2 error codes per RFC 6749
const (
oAuth2ErrorInvalidRequest = "invalid_request"
oAuth2ErrorServerError = "server_error"
oAuth2ErrorAccessDenied = "access_denied"
)

// OAuthServerAuthorize handles GET /oauth/authorize
func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) error {
ctx := r.Context()
Expand Down Expand Up @@ -130,7 +123,7 @@ func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) er
// From this point on, we have valid client + redirect_uri + all params, so we can redirect errors
// validate all other parameters - now we can redirect errors
if err := s.validateRemainingAuthorizeParams(params); err != nil {
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, oAuth2ErrorInvalidRequest, err.Error(), params.State)
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, apierrors.OAuthErrorCodeInvalidRequest, err.Error(), params.State)
http.Redirect(w, r, errorRedirectURL, http.StatusFound)
return nil
}
Expand All @@ -150,7 +143,7 @@ func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) er

if err := models.CreateOAuthServerAuthorization(db, authorization); err != nil {
// Error creating authorization - redirect with server_error
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, oAuth2ErrorServerError, "error creating authorization", params.State)
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, apierrors.OAuthErrorCodeServerError, "error creating authorization", params.State)
http.Redirect(w, r, errorRedirectURL, http.StatusFound)
return nil
}
Expand All @@ -161,7 +154,7 @@ func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) er
// Redirect to authorization path with authorization_id
if config.OAuthServer.AuthorizationPath == "" {
// OAuth authorization path not configured - redirect with server_error
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, oAuth2ErrorServerError, "oauth authorization path not configured", params.State)
errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, apierrors.OAuthErrorCodeServerError, "oauth authorization path not configured", params.State)
http.Redirect(w, r, errorRedirectURL, http.StatusFound)
return nil
}
Expand Down Expand Up @@ -392,7 +385,7 @@ func (s *Server) OAuthServerConsent(w http.ResponseWriter, r *http.Request) erro
if authorization.State != nil {
state = *authorization.State
}
redirectURL = s.buildErrorRedirectURL(authorization.RedirectURI, oAuth2ErrorAccessDenied, "User denied the request", state)
redirectURL = s.buildErrorRedirectURL(authorization.RedirectURI, apierrors.OAuthErrorCodeAccessDenied, "User denied the request", state)

observability.LogEntrySetField(r, "oauth_consent_action", string(OAuthServerConsentActionDeny))
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/oauthserver/authorize_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ func (ts *OAuthAuthorizeTestSuite) TestConsent_DenyReturnsAccessDenied() {
require.NotEmpty(ts.T(), resp.RedirectURL)
parsed, err := url.Parse(resp.RedirectURL)
require.NoError(ts.T(), err)
assert.Equal(ts.T(), oAuth2ErrorAccessDenied, parsed.Query().Get("error"))
assert.Equal(ts.T(), apierrors.OAuthErrorCodeAccessDenied, parsed.Query().Get("error"))
assert.Empty(ts.T(), parsed.Query().Get("code"))

reloaded := ts.reload(auth.AuthorizationID)
Expand Down
38 changes: 19 additions & 19 deletions internal/api/oauthserver/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ func (s *Server) OAuthToken(w http.ResponseWriter, r *http.Request) error {
contentType := r.Header.Get("Content-Type")
if strings.Contains(contentType, "application/json") {
if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
return apierrors.NewOAuthError("invalid_request", "Invalid JSON body")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Invalid JSON body")
}
} else {
// Parse form data
if err := r.ParseForm(); err != nil {
return apierrors.NewOAuthError("invalid_request", "Failed to parse form data")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Failed to parse form data")
}

params.GrantType = r.FormValue("grant_type")
Expand All @@ -290,17 +290,17 @@ func (s *Server) OAuthToken(w http.ResponseWriter, r *http.Request) error {

// Validate grant_type
if params.GrantType == "" {
return apierrors.NewOAuthError("invalid_request", "grant_type is required")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "grant_type is required")
}

client := shared.GetOAuthServerClient(ctx)
if client == nil {
return apierrors.NewOAuthError("invalid_client", "Client authentication required")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidClient, "Client authentication required")
}

// Validate that the authenticated client is allowed to use the requested grant type
if !client.IsGrantTypeAllowed(params.GrantType) {
return apierrors.NewOAuthError("unsupported_grant_type", "Client is not allowed to use grant type: "+params.GrantType)
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeUnsupportedGrantType, "Client is not allowed to use grant type: "+params.GrantType)
}

switch params.GrantType {
Expand All @@ -309,20 +309,20 @@ func (s *Server) OAuthToken(w http.ResponseWriter, r *http.Request) error {
case GrantTypeRefreshToken:
return s.handleRefreshTokenGrant(ctx, w, r, &params)
default:
return apierrors.NewOAuthError("unsupported_grant_type", "Unsupported grant type: "+params.GrantType)
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeUnsupportedGrantType, "Unsupported grant type: "+params.GrantType)
}
}

// handleAuthorizationCodeGrant handles the authorization_code grant type
func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.ResponseWriter, r *http.Request, params *OAuthTokenParams) error {
if params.Code == "" {
return apierrors.NewOAuthError("invalid_request", "code is required for authorization_code grant")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "code is required for authorization_code grant")
}

// Get authenticated client from middleware
client := shared.GetOAuthServerClient(ctx)
if client == nil {
return apierrors.NewOAuthError("invalid_client", "Client authentication required")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidClient, "Client authentication required")
}

// Exchange authorization code for tokens
Expand All @@ -336,51 +336,51 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon
authorization, err := models.FindOAuthServerAuthorizationByCode(db, params.Code)
if err != nil {
if models.IsNotFoundError(err) {
return apierrors.NewOAuthError("invalid_grant", "Invalid authorization code")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Invalid authorization code")
}
return apierrors.NewInternalServerError("Error finding authorization code").WithInternalError(err)
}

// Check if the authorization has expired
if authorization.IsExpired() {
return apierrors.NewOAuthError("invalid_grant", "Authorization code has expired")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Authorization code has expired")
}

// Validate that the authorization code was issued for this client
if authorization.ClientID != client.ID {
return apierrors.NewOAuthError("invalid_grant", "Authorization code was not issued for this client")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Authorization code was not issued for this client")
}

// Validate that (if exists) the resource parameter matches the authorization code resource
if params.Resource != "" && params.Resource != utilities.StringValue(authorization.Resource) {
return apierrors.NewOAuthError("invalid_grant", "Authorization code resource does not match the resource parameter")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Authorization code resource does not match the resource parameter")
}

// Validate redirect_uri if provided - must match the one used in authorization
if params.RedirectURI != "" && params.RedirectURI != authorization.RedirectURI {
return apierrors.NewOAuthError("invalid_grant", "Invalid redirect_uri")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Invalid redirect_uri")
}

// Validate PKCE if used in the authorization
if err := authorization.VerifyPKCE(params.CodeVerifier); err != nil {
return apierrors.NewOAuthError("invalid_grant", "PKCE verification failed: "+err.Error())
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "PKCE verification failed: "+err.Error())
}

// Get the user for the authorization code
if authorization.UserID == nil {
return apierrors.NewOAuthError("invalid_grant", "Authorization code has no associated user")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Authorization code has no associated user")
}

user, err := models.FindUserByID(db, *authorization.UserID)
if err != nil {
if models.IsNotFoundError(err) {
return apierrors.NewOAuthError("invalid_grant", "User not found for authorization code")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "User not found for authorization code")
}
return apierrors.NewInternalServerError("Error finding user").WithInternalError(err)
}

if user.IsBanned() {
return apierrors.NewOAuthError("access_denied", "User is banned")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeAccessDenied, "User is banned")
}

// Exchange the authorization code for tokens
Expand All @@ -396,7 +396,7 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon
err = db.Transaction(func(tx *storage.Connection) error {
if _, terr := models.FindOAuthServerAuthorizationByIDForUpdate(tx, authorization.AuthorizationID); terr != nil {
if models.IsNotFoundError(terr) {
return apierrors.NewOAuthError("invalid_grant", "Invalid authorization code")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidGrant, "Invalid authorization code")
}
return apierrors.NewInternalServerError("Error locking authorization code").WithInternalError(terr)
}
Expand Down Expand Up @@ -478,7 +478,7 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon
// handleRefreshTokenGrant handles the refresh_token grant type
func (s *Server) handleRefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.Request, params *OAuthTokenParams) error {
if params.RefreshToken == "" {
return apierrors.NewOAuthError("invalid_request", "refresh_token is required for refresh_token grant")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "refresh_token is required for refresh_token grant")
}

// Use the token service to handle refresh token grant
Expand Down
20 changes: 10 additions & 10 deletions internal/api/token_oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,16 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
}

if params.IdToken == "" {
return apierrors.NewOAuthError("invalid request", "id_token required")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "id_token required")
}

if params.Provider == "" && (params.ClientID == "" || params.Issuer == "") {
return apierrors.NewOAuthError("invalid request", "provider or client_id and issuer required")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "provider or client_id and issuer required")
}

if params.LinkIdentity {
if r.Header.Get("Authorization") == "" {
return apierrors.NewOAuthError("invalid request", "Linking requires a valid user access token in Authorization")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Linking requires a valid user access token in Authorization")
}

requireAuthCtx, err := a.requireAuthentication(w, r)
Expand All @@ -230,7 +230,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R

targetUser := getUser(requireAuthCtx)
if targetUser == nil {
return apierrors.NewOAuthError("invalid request", "Linking requires a valid user authentication")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Linking requires a valid user authentication")
}

// set it so linkIdentityToUser works below
Expand All @@ -256,7 +256,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
AccessToken: params.AccessToken,
})
if err != nil {
return apierrors.NewOAuthError("invalid request", "Bad ID token").WithInternalError(err)
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Bad ID token").WithInternalError(err)
}

userData.Metadata.EmailVerified = false
Expand All @@ -272,7 +272,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
}

if idToken.Subject == "" {
return apierrors.NewOAuthError("invalid request", "Missing sub claim in id_token")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Missing sub claim in id_token")
}

correctAudience := false
Expand All @@ -288,20 +288,20 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
}

if !correctAudience {
return apierrors.NewOAuthError("invalid request", fmt.Sprintf("Unacceptable audience in id_token: %v", idToken.Audience))
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, fmt.Sprintf("Unacceptable audience in id_token: %v", idToken.Audience))
}

if !skipNonceCheck {
tokenHasNonce := idToken.Nonce != ""
paramsHasNonce := params.Nonce != ""

if tokenHasNonce != paramsHasNonce {
return apierrors.NewOAuthError("invalid request", "Passed nonce and nonce in id_token should either both exist or not.")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Passed nonce and nonce in id_token should either both exist or not.")
} else if tokenHasNonce && paramsHasNonce {
// verify nonce to mitigate replay attacks
hash := fmt.Sprintf("%x", sha256.Sum256([]byte(params.Nonce)))
if hash != idToken.Nonce {
return apierrors.NewOAuthError("invalid nonce", "Nonces mismatch")
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeInvalidRequest, "Nonces mismatch")
}
}
}
Expand Down Expand Up @@ -356,7 +356,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
case *HTTPError:
return err
default:
return apierrors.NewOAuthError("server_error", "Internal Server Error").WithInternalError(err)
return apierrors.NewOAuthError(apierrors.OAuthErrorCodeServerError, "Internal Server Error").WithInternalError(err)
}
}
if createdUser {
Expand Down
Loading