From c8e0b30e172bb9aace14dd5b77ec7f0379fb8502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 9 Apr 2024 09:42:59 +0300 Subject: [PATCH] fix(oidc): return bad request for base64 errors (#7730) * fix(oidc): return bad request for base64 errors We've recently noticed an increased amount of 500: internal server error status returns on zitadel cloud. The source of these errors appear to be erroneous input in fields that are supposed to be bas64 formatted. ``` time=2024-04-08T14:05:47.600Z level=ERROR msg="request error" oidc_error.parent="ID=OIDC-AhX2u Message=Errors.Internal Parent=(illegal base64 data at input byte 8)" oidc_error.description=Errors.Internal oidc_error.type=server_error status_code=500 ``` Within the possible code paths of the token endpoint there are a couple of uses of base64.Encoding.DecodeString of which a returned error was not properly wrapped, but returned as-is. This causes the oidc error handler to return a 500 with the `OIDC-AhX2u` ID. We were not able to pinpoint the exact errors that are happening to any one call of `DecodeString`. This fix wraps all errors from `DecodeString` so that proper 400: bad request is returned with information about the error. Each wrapper now has an unique error ID, so that logs will contain the source of the error as well. This bug was reported internally by the ops team. * catch op.ErrInvalidRefreshToken --- internal/api/authz/session_token.go | 2 +- internal/api/oidc/auth_request.go | 4 ++-- internal/api/oidc/error.go | 4 +++- internal/command/oidc_session.go | 2 +- internal/domain/refresh_token.go | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/api/authz/session_token.go b/internal/api/authz/session_token.go index eb46008659..c6a827b0a1 100644 --- a/internal/api/authz/session_token.go +++ b/internal/api/authz/session_token.go @@ -19,7 +19,7 @@ func SessionTokenVerifier(algorithm crypto.EncryptionAlgorithm) func(ctx context return func(ctx context.Context, sessionToken, sessionID, tokenID string) (err error) { decodedToken, err := base64.RawURLEncoding.DecodeString(sessionToken) if err != nil { - return err + return zerrors.ThrowInvalidArgument(err, "COMMAND-hi6Ph", "Errors.Session.Token.Invalid") } _, spanPasswordComparison := tracing.NewNamedSpan(ctx, "crypto.CompareHash") token, err := algorithm.DecryptString(decodedToken, algorithm.EncryptionKeyID()) diff --git a/internal/api/oidc/auth_request.go b/internal/api/oidc/auth_request.go index 4f6dcdb5d1..384dc402a5 100644 --- a/internal/api/oidc/auth_request.go +++ b/internal/api/oidc/auth_request.go @@ -157,7 +157,7 @@ func (o *OPStorage) AuthRequestByCode(ctx context.Context, code string) (_ op.Au plainCode, err := o.decryptGrant(code) if err != nil { - return nil, err + return nil, zerrors.ThrowInvalidArgument(err, "OIDC-ahLi2", "Errors.User.Code.Invalid") } if strings.HasPrefix(plainCode, command.IDPrefixV2) { authReq, err := o.command.ExchangeAuthCode(ctx, plainCode) @@ -311,7 +311,7 @@ func (o *OPStorage) TokenRequestByRefreshToken(ctx context.Context, refreshToken plainToken, err := o.decryptGrant(refreshToken) if err != nil { - return nil, err + return nil, op.ErrInvalidRefreshToken } if strings.HasPrefix(plainToken, command.IDPrefixV2) { oidcSession, err := o.command.OIDCSessionByRefreshToken(ctx, plainToken) diff --git a/internal/api/oidc/error.go b/internal/api/oidc/error.go index 9c7154092f..781a1d3815 100644 --- a/internal/api/oidc/error.go +++ b/internal/api/oidc/error.go @@ -19,7 +19,9 @@ func oidcError(err error) error { if err == nil { return nil } - + if errors.Is(err, op.ErrInvalidRefreshToken) { + err = zerrors.ThrowInvalidArgument(err, "OIDCS-ef2Gi", "Errors.User.RefreshToken.Invalid") + } var ( sError op.StatusError oError *oidc.Error diff --git a/internal/command/oidc_session.go b/internal/command/oidc_session.go index 90d852d872..7d04205aa4 100644 --- a/internal/command/oidc_session.go +++ b/internal/command/oidc_session.go @@ -207,7 +207,7 @@ func (c *Commands) getResourceOwnerOfSessionUser(ctx context.Context, userID, in func (c *Commands) decryptRefreshToken(refreshToken string) (refreshTokenID string, err error) { decoded, err := base64.RawURLEncoding.DecodeString(refreshToken) if err != nil { - return "", err + return "", zerrors.ThrowInvalidArgument(err, "OIDCS-Cux9a", "Errors.User.RefreshToken.Invalid") } decrypted, err := c.keyAlgorithm.DecryptString(decoded, c.keyAlgorithm.EncryptionKeyID()) if err != nil { diff --git a/internal/domain/refresh_token.go b/internal/domain/refresh_token.go index 3bd6aed2f4..6f2d883df5 100644 --- a/internal/domain/refresh_token.go +++ b/internal/domain/refresh_token.go @@ -23,7 +23,7 @@ func RefreshToken(userID, tokenID, token string, algorithm crypto.EncryptionAlgo func FromRefreshToken(refreshToken string, algorithm crypto.EncryptionAlgorithm) (userID, tokenID, token string, err error) { decoded, err := base64.RawURLEncoding.DecodeString(refreshToken) if err != nil { - return "", "", "", err + return "", "", "", zerrors.ThrowInvalidArgument(err, "DOMAIN-BGDhn", "Errors.User.RefreshToken.Invalid") } decrypted, err := algorithm.Decrypt(decoded, algorithm.EncryptionKeyID()) if err != nil {