mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-04 21:32:15 +00:00
feat(domain): introduce error slugs (#12030)
# Which Problems Are Solved As part of https://github.com/zitadel/zitadel/issues/11917 we want to introduce error slugs so (API) clients can rely on stable, machine-readable errors and act accordingly. # How the Problems Are Solved - Added a `NewSlug` helper function in the domain package. - Added `ErrorDetails` to the `ZitadelError` - Added an `zitadel.error.v2.ErrorDetail` proto message - Updated the connectRPC error interceptor to map new slug based errors to the new `ErrorDetail` - Defined some common slugs and error functions like internal errors - Defined (session) specific slugs used in the `DeleteSession` and `CheckUser` functions and replaced old implementations - Updated integration tests to check specific errors if the relation database feature is enabled - Updated doc and guideline to reflect the latest changes and decisions - Updated DeleteSession endpoint API to list possible slugs # Additional Changes None # Additional Context - closes #11957 --------- Co-authored-by: Wim Van Laer <[email protected]>
This commit is contained in:
co-authored by
Wim Van Laer
parent
5c9361f6c7
commit
a2bf528ca0
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"strings"
|
||||
|
||||
"connectrpc.com/connect"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
@@ -12,10 +13,12 @@ import (
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"google.golang.org/protobuf/protoadapt"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
|
||||
"github.com/zitadel/zitadel/backend/v3/instrumentation/logging"
|
||||
commandErrors "github.com/zitadel/zitadel/internal/command/errors"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
"github.com/zitadel/zitadel/pkg/grpc/error/v2"
|
||||
"github.com/zitadel/zitadel/pkg/grpc/message"
|
||||
)
|
||||
|
||||
@@ -34,7 +37,7 @@ func ZITADELToGRPCError(ctx context.Context, err error) error {
|
||||
msg += " (" + id + ")"
|
||||
logging.Log(ctx, lvl, msg, "err", err, "code", code)
|
||||
|
||||
errorInfo := getErrorInfo(id, key, err)
|
||||
errorInfo := getErrorInfo(ctx, id, key, err)
|
||||
|
||||
s, err := status.New(code, msg).WithDetails(errorInfo)
|
||||
if err != nil {
|
||||
@@ -61,7 +64,7 @@ func ZITADELToConnectError(ctx context.Context, err error) error {
|
||||
msg += " (" + id + ")"
|
||||
logging.Log(ctx, lvl, msg, "err", err)
|
||||
|
||||
errorInfo := getErrorInfo(id, key, err)
|
||||
errorInfo := getErrorInfo(ctx, id, key, err)
|
||||
|
||||
cErr := connect.NewError(connect.Code(code), errors.New(msg))
|
||||
if detail, detailErr := connect.NewErrorDetail(errorInfo.(proto.Message)); detailErr == nil {
|
||||
@@ -152,15 +155,32 @@ func grpcStatusLevel(code codes.Code) slog.Level {
|
||||
}
|
||||
}
|
||||
|
||||
func getErrorInfo(id, key string, err error) protoadapt.MessageV1 {
|
||||
var errorInfo protoadapt.MessageV1
|
||||
|
||||
func getErrorInfo(ctx context.Context, id, key string, err error) protoadapt.MessageV1 {
|
||||
var wpe *commandErrors.WrongPasswordError
|
||||
var zerr *zerrors.ZitadelError
|
||||
if err != nil && errors.As(err, &wpe) {
|
||||
errorInfo = &message.CredentialsCheckError{Id: id, Message: key, FailedAttempts: wpe.FailedAttempts}
|
||||
} else {
|
||||
errorInfo = &message.ErrorDetail{Id: id, Message: key}
|
||||
return &message.CredentialsCheckError{Id: id, Message: key, FailedAttempts: wpe.FailedAttempts}
|
||||
}
|
||||
// Let's check the error is a zitadel error and does contain a slug, which should be the case for almost any error
|
||||
// in the future, but not yet. In case it's not, we'll fall back to the old error details to prevent a breaking change.
|
||||
if err != nil && errors.As(err, &zerr) && strings.Contains(zerr.GetID(), ".") {
|
||||
if zerr.Details == nil {
|
||||
return &errorpb.ErrorDetail{Slug: id, Message: key}
|
||||
}
|
||||
errDetails, err := zerr.GetDetails().MarshalJSON()
|
||||
if err != nil {
|
||||
logging.WithError(ctx, err).Error("unable to marshal error details")
|
||||
return &errorpb.ErrorDetail{Slug: id, Message: key}
|
||||
}
|
||||
|
||||
return errorInfo
|
||||
details := new(structpb.Struct)
|
||||
err = details.UnmarshalJSON(errDetails)
|
||||
if err != nil {
|
||||
logging.WithError(ctx, err).Error("unable to unmarshal error details")
|
||||
return &errorpb.ErrorDetail{Slug: id, Message: key}
|
||||
}
|
||||
return &errorpb.ErrorDetail{Slug: id, Message: key, Details: details}
|
||||
}
|
||||
return &message.ErrorDetail{Id: id, Message: key}
|
||||
}
|
||||
|
||||
@@ -10,10 +10,13 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"google.golang.org/protobuf/protoadapt"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
|
||||
commandErrors "github.com/zitadel/zitadel/internal/command/errors"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
errorpb "github.com/zitadel/zitadel/pkg/grpc/error/v2"
|
||||
"github.com/zitadel/zitadel/pkg/grpc/message"
|
||||
)
|
||||
|
||||
@@ -98,11 +101,35 @@ func Test_getErrorInfo(t *testing.T) {
|
||||
}(),
|
||||
result: &message.CredentialsCheckError{Id: "id", Message: "key", FailedAttempts: 26},
|
||||
},
|
||||
{
|
||||
name: "parent error not nil wrapped zerrors.ZitadelError{} with id, message errorpb.ErrorDetail{}",
|
||||
id: "id",
|
||||
key: "key",
|
||||
err: func() error {
|
||||
err := fmt.Errorf("normal error")
|
||||
zerr := &zerrors.ZitadelError{ID: "id", Message: "key"}
|
||||
return fmt.Errorf("%w: %w", err, zerr)
|
||||
}(),
|
||||
result: &message.ErrorDetail{Id: "id", Message: "key"},
|
||||
},
|
||||
{
|
||||
name: "parent error not nil wrapped zerrors.ZitadelError{} with slug, return errorpb.ErrorDetail{}",
|
||||
id: "slug.reason",
|
||||
key: "message",
|
||||
err: func() error {
|
||||
err := fmt.Errorf("normal error")
|
||||
zerr := &zerrors.ZitadelError{ID: "slug.reason", Message: "message", Details: zerrors.ErrorDetailsMap{"some": "details"}}
|
||||
return fmt.Errorf("%w: %w", err, zerr)
|
||||
}(),
|
||||
result: &errorpb.ErrorDetail{Slug: "slug.reason", Message: "message", Details: &structpb.Struct{Fields: map[string]*structpb.Value{"some": {Kind: &structpb.Value_StringValue{StringValue: "details"}}}}},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
errorInfo := getErrorInfo(tt.id, tt.key, tt.err)
|
||||
assert.Equal(t, tt.result, errorInfo)
|
||||
errorInfo := getErrorInfo(t.Context(), tt.id, tt.key, tt.err)
|
||||
if !proto.Equal(tt.result.(proto.Message), errorInfo.(proto.Message)) {
|
||||
t.Errorf("getErrorInfo() = %v, want %v", errorInfo, tt.result)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"google.golang.org/protobuf/types/known/durationpb"
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
|
||||
"github.com/zitadel/zitadel/backend/v3/domain"
|
||||
"github.com/zitadel/zitadel/internal/integration"
|
||||
"github.com/zitadel/zitadel/internal/integration/sink"
|
||||
"github.com/zitadel/zitadel/pkg/grpc/feature/v2"
|
||||
@@ -917,6 +918,9 @@ func TestServer_DeleteSession_token(t *testing.T) {
|
||||
SessionToken: gu.Ptr("invalid"),
|
||||
})
|
||||
require.Error(t, err)
|
||||
if stateCase.Enabled {
|
||||
integration.RequireStatusError(t, err, domain.ErrSessionTokenInvalid(nil))
|
||||
}
|
||||
|
||||
_, err = stateCase.Inst.Client.SessionV2.DeleteSession(stateCase.InstOwner, &session.DeleteSessionRequest{
|
||||
SessionId: createResp.GetSessionId(),
|
||||
@@ -965,6 +969,9 @@ func TestServer_DeleteSession_own_session(t *testing.T) {
|
||||
SessionId: createResp.GetSessionId(),
|
||||
})
|
||||
require.Error(t, err)
|
||||
if stateCase.Enabled {
|
||||
integration.RequireStatusError(t, err, domain.ErrAuthMissingPermission(nil, "", domain.SessionDeletePermission))
|
||||
}
|
||||
|
||||
// delete the new (user1) session by themselves
|
||||
_, err = stateCase.Inst.Client.SessionV2.DeleteSession(integration.WithAuthorizationToken(t.Context(), token1), &session.DeleteSessionRequest{
|
||||
@@ -1039,6 +1046,7 @@ func TestServer_DeleteSession_expired(t *testing.T) {
|
||||
sessionResp, err := stateCase.Inst.Client.SessionV2.GetSession(stateCase.Inst.WithAuthorizationToken(t.Context(), integration.UserTypeOrgOwner),
|
||||
&session.GetSessionRequest{SessionId: createResp.GetSessionId()})
|
||||
require.Error(t, err)
|
||||
// TODO: implement error check once the query side is relational
|
||||
require.Nil(t, sessionResp)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user