mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-27 09:42:16 +00:00
fix: propagate non-NotFound errors from instance interceptor (#12019)
This commit is contained in:
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/zitadel/logging"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/authz"
|
||||
"github.com/zitadel/zitadel/internal/api/grpc/gerrors"
|
||||
zitadel_http "github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/i18n"
|
||||
"github.com/zitadel/zitadel/internal/telemetry/tracing"
|
||||
@@ -81,7 +82,8 @@ func addInstanceByDomain(ctx context.Context, req connect.AnyRequest, handler co
|
||||
if errors.As(err, ¬FoundErr) && notFoundErr.Kind == zerrors.KindNotFound {
|
||||
notFoundErr.Message = translator.LocalizeFromCtx(ctx, notFoundErr.GetMessage(), nil)
|
||||
}
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("unable to set instance using domain %s: %w", domain, notFoundErr))
|
||||
code, _, _ := gerrors.ExtractZITADELError(err)
|
||||
return nil, connect.NewError(connect.Code(code), fmt.Errorf("unable to set instance using domain %s: %w", domain, err))
|
||||
}
|
||||
return handler(authz.WithInstance(ctx, instance), req)
|
||||
}
|
||||
@@ -97,12 +99,12 @@ func addInstanceByRequestedHost(ctx context.Context, req connect.AnyRequest, han
|
||||
origin := zitadel_http.DomainContext(ctx)
|
||||
logging.WithFields("origin", requestContext.Origin(), "externalDomain", externalDomain).WithError(err).Error("unable to set instance")
|
||||
zErr := new(zerrors.ZitadelError)
|
||||
code, _, _ := gerrors.ExtractZITADELError(err)
|
||||
if errors.As(err, &zErr) {
|
||||
zErr.SetMessage(translator.LocalizeFromCtx(ctx, zErr.GetMessage(), nil))
|
||||
zErr.Parent = err
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, externalDomain, zErr.Error()))
|
||||
return nil, connect.NewError(connect.Code(code), fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, externalDomain, zErr.Error()))
|
||||
}
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s)", origin, externalDomain))
|
||||
return nil, connect.NewError(connect.Code(code), fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s)", origin, externalDomain))
|
||||
}
|
||||
return handler(authz.WithInstance(ctx, instance), req)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
package connect_middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"connectrpc.com/connect"
|
||||
"golang.org/x/text/language"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/authz"
|
||||
http_util "github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/i18n"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
object_v3 "github.com/zitadel/zitadel/pkg/grpc/object/v3alpha"
|
||||
)
|
||||
|
||||
func Test_setInstance_errorCodes(t *testing.T) {
|
||||
i18n.SupportLanguages(language.English)
|
||||
translator := i18n.NewZitadelTranslator(language.English)
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
wantCode connect.Code
|
||||
}{
|
||||
{
|
||||
name: "not found from verifier propagates as NotFound",
|
||||
err: zerrors.ThrowNotFound(nil, "TEST-001", "Errors.Instance.NotFound"),
|
||||
wantCode: connect.CodeNotFound,
|
||||
},
|
||||
{
|
||||
name: "internal error from verifier propagates as Internal",
|
||||
err: zerrors.ThrowInternal(errors.New("FATAL: the database system is shutting down (SQLSTATE 57P03)"), "TEST-002", "Errors.Internal"),
|
||||
wantCode: connect.CodeInternal,
|
||||
},
|
||||
{
|
||||
name: "unavailable error from verifier propagates as Unavailable",
|
||||
err: zerrors.ThrowUnavailable(nil, "TEST-003", "Errors.Unavailable"),
|
||||
wantCode: connect.CodeUnavailable,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
verifier := &mockInstanceVerifier{err: tc.err}
|
||||
|
||||
t.Run("byRequestedHost/"+tc.name, func(t *testing.T) {
|
||||
ctx := http_util.WithDomainContext(context.Background(), &http_util.DomainCtx{InstanceHost: "host"})
|
||||
_, err := setInstance(ctx, &mockReq[struct{}]{}, nil, verifier, "", translator)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
var connectErr *connect.Error
|
||||
if !errors.As(err, &connectErr) {
|
||||
t.Fatalf("expected *connect.Error, got %T", err)
|
||||
}
|
||||
if got := connectErr.Code(); got != tc.wantCode {
|
||||
t.Errorf("got code %v, want %v", got, tc.wantCode)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("byDomain/"+tc.name, func(t *testing.T) {
|
||||
req := &mockReq[mockInstanceMsg]{
|
||||
Request: connect.Request[mockInstanceMsg]{
|
||||
Msg: &mockInstanceMsg{instance: object_v3.Instance{
|
||||
Property: &object_v3.Instance_Domain{Domain: "host"},
|
||||
}},
|
||||
},
|
||||
}
|
||||
_, err := setInstance(context.Background(), req, nil, verifier, "", translator)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
var connectErr *connect.Error
|
||||
if !errors.As(err, &connectErr) {
|
||||
t.Fatalf("expected *connect.Error, got %T", err)
|
||||
}
|
||||
if got := connectErr.Code(); got != tc.wantCode {
|
||||
t.Errorf("got code %v, want %v", got, tc.wantCode)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type mockInstanceMsg struct {
|
||||
instance object_v3.Instance
|
||||
}
|
||||
|
||||
func (m *mockInstanceMsg) GetInstance() *object_v3.Instance {
|
||||
return &m.instance
|
||||
}
|
||||
|
||||
type mockInstanceVerifier struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (m *mockInstanceVerifier) InstanceByHost(_ context.Context, _, _ string) (authz.Instance, error) {
|
||||
return nil, m.err
|
||||
}
|
||||
|
||||
func (m *mockInstanceVerifier) InstanceByID(_ context.Context, _ string) (authz.Instance, error) {
|
||||
return nil, m.err
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"google.golang.org/grpc/status"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/authz"
|
||||
"github.com/zitadel/zitadel/internal/api/grpc/gerrors"
|
||||
zitadel_http "github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/i18n"
|
||||
"github.com/zitadel/zitadel/internal/telemetry/tracing"
|
||||
@@ -69,7 +70,8 @@ func addInstanceByID(ctx context.Context, req interface{}, handler grpc.UnaryHan
|
||||
if errors.As(err, ¬FoundErr) {
|
||||
notFoundErr.Message = translator.LocalizeFromCtx(ctx, notFoundErr.GetMessage(), nil)
|
||||
}
|
||||
return nil, status.Error(codes.NotFound, fmt.Errorf("unable to set instance using id %s: %w", id, notFoundErr).Error())
|
||||
code, _, _ := gerrors.ExtractZITADELError(err)
|
||||
return nil, status.Error(code, fmt.Errorf("unable to set instance using id %s: %w", id, err).Error())
|
||||
}
|
||||
return handler(authz.WithInstance(ctx, instance), req)
|
||||
}
|
||||
@@ -81,7 +83,8 @@ func addInstanceByDomain(ctx context.Context, req interface{}, handler grpc.Unar
|
||||
if errors.As(err, ¬FoundErr) && notFoundErr.Kind == zerrors.KindNotFound {
|
||||
notFoundErr.Message = translator.LocalizeFromCtx(ctx, notFoundErr.GetMessage(), nil)
|
||||
}
|
||||
return nil, status.Error(codes.NotFound, fmt.Errorf("unable to set instance using domain %s: %w", domain, notFoundErr).Error())
|
||||
code, _, _ := gerrors.ExtractZITADELError(err)
|
||||
return nil, status.Error(code, fmt.Errorf("unable to set instance using domain %s: %w", domain, err).Error())
|
||||
}
|
||||
return handler(authz.WithInstance(ctx, instance), req)
|
||||
}
|
||||
@@ -96,13 +99,13 @@ func addInstanceByRequestedHost(ctx context.Context, req interface{}, handler gr
|
||||
if err != nil {
|
||||
origin := zitadel_http.DomainContext(ctx)
|
||||
logging.WithFields("origin", requestContext.Origin(), "externalDomain", externalDomain).WithError(err).Error("unable to set instance")
|
||||
code, _, _ := gerrors.ExtractZITADELError(err)
|
||||
zErr := new(zerrors.ZitadelError)
|
||||
if errors.As(err, &zErr) {
|
||||
zErr.SetMessage(translator.LocalizeFromCtx(ctx, zErr.GetMessage(), nil))
|
||||
zErr.Parent = err
|
||||
return nil, status.Error(codes.NotFound, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, externalDomain, zErr))
|
||||
return nil, status.Error(code, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, externalDomain, zErr.Error()))
|
||||
}
|
||||
return nil, status.Error(codes.NotFound, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, externalDomain))
|
||||
return nil, status.Error(code, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, externalDomain))
|
||||
}
|
||||
return handler(authz.WithInstance(ctx, instance), req)
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ package middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"testing"
|
||||
@@ -9,11 +10,15 @@ import (
|
||||
|
||||
"golang.org/x/text/language"
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/authz"
|
||||
http_util "github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/execution/target"
|
||||
"github.com/zitadel/zitadel/internal/feature"
|
||||
"github.com/zitadel/zitadel/internal/i18n"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
object_v3 "github.com/zitadel/zitadel/pkg/grpc/object/v3alpha"
|
||||
)
|
||||
|
||||
@@ -229,9 +234,13 @@ type mockInstanceVerifier struct {
|
||||
id string
|
||||
instanceHost string
|
||||
publicHost string
|
||||
err error
|
||||
}
|
||||
|
||||
func (m *mockInstanceVerifier) InstanceByHost(_ context.Context, instanceHost, publicHost string) (authz.Instance, error) {
|
||||
if m.err != nil {
|
||||
return nil, m.err
|
||||
}
|
||||
if instanceHost != m.instanceHost {
|
||||
return nil, fmt.Errorf("invalid host")
|
||||
}
|
||||
@@ -245,6 +254,9 @@ func (m *mockInstanceVerifier) InstanceByHost(_ context.Context, instanceHost, p
|
||||
}
|
||||
|
||||
func (m *mockInstanceVerifier) InstanceByID(_ context.Context, id string) (authz.Instance, error) {
|
||||
if m.err != nil {
|
||||
return nil, m.err
|
||||
}
|
||||
if id != m.id {
|
||||
return nil, fmt.Errorf("not found")
|
||||
}
|
||||
@@ -304,3 +316,75 @@ func (m *mockInstance) Features() feature.Features {
|
||||
func (m *mockInstance) ExecutionRouter() target.Router {
|
||||
return target.NewRouter(nil)
|
||||
}
|
||||
|
||||
func Test_setInstance_errorCodes(t *testing.T) {
|
||||
i18n.SupportLanguages(language.English)
|
||||
translator := i18n.NewZitadelTranslator(language.English)
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
wantCode codes.Code
|
||||
}{
|
||||
{
|
||||
name: "not found from verifier propagates as NotFound",
|
||||
err: zerrors.ThrowNotFound(nil, "TEST-001", "Errors.Instance.NotFound"),
|
||||
wantCode: codes.NotFound,
|
||||
},
|
||||
{
|
||||
name: "internal error from verifier propagates as Internal",
|
||||
err: zerrors.ThrowInternal(errors.New("FATAL: the database system is shutting down (SQLSTATE 57P03)"), "TEST-002", "Errors.Internal"),
|
||||
wantCode: codes.Internal,
|
||||
},
|
||||
{
|
||||
name: "unavailable error from verifier propagates as Unavailable",
|
||||
err: zerrors.ThrowUnavailable(nil, "TEST-003", "Errors.Unavailable"),
|
||||
wantCode: codes.Unavailable,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
verifier := &mockInstanceVerifier{err: tc.err}
|
||||
|
||||
t.Run("byRequestedHost/"+tc.name, func(t *testing.T) {
|
||||
ctx := http_util.WithDomainContext(context.Background(), &http_util.DomainCtx{InstanceHost: "host"})
|
||||
_, err := setInstance(ctx, &mockRequest{}, &grpc.UnaryServerInfo{}, nil, verifier, "", translator)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
if got := status.Code(err); got != tc.wantCode {
|
||||
t.Errorf("got code %v, want %v", got, tc.wantCode)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("byID/"+tc.name, func(t *testing.T) {
|
||||
req := &mockRequestWithExplicitInstance{
|
||||
instance: object_v3.Instance{
|
||||
Property: &object_v3.Instance_Id{Id: "any"},
|
||||
},
|
||||
}
|
||||
_, err := setInstance(context.Background(), req, &grpc.UnaryServerInfo{}, nil, verifier, "", translator)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
if got := status.Code(err); got != tc.wantCode {
|
||||
t.Errorf("got code %v, want %v", got, tc.wantCode)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("byDomain/"+tc.name, func(t *testing.T) {
|
||||
req := &mockRequestWithExplicitInstance{
|
||||
instance: object_v3.Instance{
|
||||
Property: &object_v3.Instance_Domain{Domain: "any"},
|
||||
},
|
||||
}
|
||||
_, err := setInstance(context.Background(), req, &grpc.UnaryServerInfo{}, nil, verifier, "", translator)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
if got := status.Code(err); got != tc.wantCode {
|
||||
t.Errorf("got code %v, want %v", got, tc.wantCode)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -48,14 +48,18 @@ func (a *instanceInterceptor) HandlerFunc(next http.Handler) http.HandlerFunc {
|
||||
origin := zitadel_http.DomainContext(r.Context())
|
||||
logging.WithFields("origin", origin.Origin(), "externalDomain", a.externalDomain).WithError(err).Error("unable to set instance")
|
||||
|
||||
statusCode := http.StatusInternalServerError
|
||||
if zerrors.IsNotFound(err) {
|
||||
statusCode = http.StatusNotFound
|
||||
}
|
||||
zErr := new(zerrors.ZitadelError)
|
||||
if errors.As(err, &zErr) {
|
||||
zErr.SetMessage(a.translator.LocalizeFromRequest(r, zErr.GetMessage(), nil))
|
||||
http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, a.externalDomain, zErr), http.StatusNotFound)
|
||||
http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, a.externalDomain, zErr), statusCode)
|
||||
return
|
||||
}
|
||||
|
||||
http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, a.externalDomain), http.StatusNotFound)
|
||||
http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, a.externalDomain), statusCode)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -17,6 +18,7 @@ import (
|
||||
"github.com/zitadel/zitadel/internal/execution/target"
|
||||
"github.com/zitadel/zitadel/internal/feature"
|
||||
"github.com/zitadel/zitadel/internal/i18n"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
func Test_instanceInterceptor_Handler(t *testing.T) {
|
||||
@@ -278,6 +280,43 @@ func Test_setInstance(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func Test_instanceInterceptor_HandlerFunc_statusCodes(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
wantStatusCode int
|
||||
}{
|
||||
{
|
||||
name: "not found from verifier propagates as 404",
|
||||
err: zerrors.ThrowNotFound(nil, "TEST-001", "Errors.Instance.NotFound"),
|
||||
wantStatusCode: http.StatusNotFound,
|
||||
},
|
||||
{
|
||||
name: "internal error from verifier propagates as 500",
|
||||
err: zerrors.ThrowInternal(errors.New("FATAL: the database system is shutting down (SQLSTATE 57P03)"), "TEST-002", "Errors.Internal"),
|
||||
wantStatusCode: http.StatusInternalServerError,
|
||||
},
|
||||
{
|
||||
name: "unavailable error from verifier propagates as 500",
|
||||
err: zerrors.ThrowUnavailable(nil, "TEST-003", "Errors.Unavailable"),
|
||||
wantStatusCode: http.StatusInternalServerError,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
a := &instanceInterceptor{
|
||||
verifier: &mockInstanceVerifier{err: tc.err},
|
||||
translator: i18n.NewZitadelTranslator(language.English),
|
||||
}
|
||||
r := httptest.NewRequest("", "/url", nil)
|
||||
r = r.WithContext(zitadel_http.WithDomainContext(r.Context(), &zitadel_http.DomainCtx{InstanceHost: "host"}))
|
||||
rr := httptest.NewRecorder()
|
||||
a.HandlerFunc(&testHandler{}).ServeHTTP(rr, r)
|
||||
assert.Equal(t, tc.wantStatusCode, rr.Code)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type testHandler struct {
|
||||
context context.Context
|
||||
}
|
||||
@@ -289,9 +328,13 @@ func (t *testHandler) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
|
||||
type mockInstanceVerifier struct {
|
||||
instanceHost string
|
||||
publicHost string
|
||||
err error
|
||||
}
|
||||
|
||||
func (m *mockInstanceVerifier) InstanceByHost(_ context.Context, instanceHost, publicHost string) (authz.Instance, error) {
|
||||
if m.err != nil {
|
||||
return nil, m.err
|
||||
}
|
||||
if instanceHost != m.instanceHost {
|
||||
return nil, fmt.Errorf("invalid host")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user