fix: instance interceptors return NotFound (404) error for unknown hosts (#4184)

* fix: instance interceptors return "NotFound" (404) error for unknown hosts

* fix tests
This commit is contained in:
Livio Spring 2022-08-17 08:07:41 +02:00 committed by GitHub
parent d0733b3185
commit d656b3f3c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 71 additions and 46 deletions

View File

@ -2,15 +2,20 @@ package middleware
import (
"context"
"errors"
"fmt"
"strings"
"github.com/zitadel/logging"
"golang.org/x/text/language"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"
"github.com/zitadel/zitadel/internal/api/authz"
caos_errors "github.com/zitadel/zitadel/internal/errors"
"github.com/zitadel/zitadel/internal/i18n"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
)
@ -23,12 +28,14 @@ type InstanceVerifier interface {
}
func InstanceInterceptor(verifier authz.InstanceVerifier, headerName string, ignoredServices ...string) grpc.UnaryServerInterceptor {
translator, err := newZitadelTranslator(language.English)
logging.OnError(err).Panic("unable to get translator")
return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
return setInstance(ctx, req, info, handler, verifier, headerName, ignoredServices...)
return setInstance(ctx, req, info, handler, verifier, headerName, translator, ignoredServices...)
}
}
func setInstance(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler, verifier authz.InstanceVerifier, headerName string, ignoredServices ...string) (_ interface{}, err error) {
func setInstance(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler, verifier authz.InstanceVerifier, headerName string, translator *i18n.Translator, ignoredServices ...string) (_ interface{}, err error) {
interceptorCtx, span := tracing.NewServerInterceptorSpan(ctx)
defer func() { span.EndWithError(err) }()
for _, service := range ignoredServices {
@ -42,11 +49,15 @@ func setInstance(ctx context.Context, req interface{}, info *grpc.UnaryServerInf
host, err := hostFromContext(interceptorCtx, headerName)
if err != nil {
return nil, status.Error(codes.PermissionDenied, err.Error())
return nil, status.Error(codes.NotFound, err.Error())
}
instance, err := verifier.InstanceByHost(interceptorCtx, host)
if err != nil {
return nil, status.Error(codes.PermissionDenied, err.Error())
caosErr := new(caos_errors.NotFoundError)
if errors.As(err, &caosErr) {
caosErr.Message = translator.LocalizeFromCtx(ctx, caosErr.GetMessage(), nil)
}
return nil, status.Error(codes.NotFound, err.Error())
}
span.End()
return handler(authz.WithInstance(ctx, instance), req)

View File

@ -135,7 +135,7 @@ func Test_setInstance(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := setInstance(tt.args.ctx, tt.args.req, tt.args.info, tt.args.handler, tt.args.verifier, tt.args.headerName)
got, err := setInstance(tt.args.ctx, tt.args.req, tt.args.info, tt.args.handler, tt.args.verifier, tt.args.headerName, nil)
if (err != nil) != tt.res.err {
t.Errorf("setInstance() error = %v, wantErr %v", err, tt.res.err)
return

View File

@ -2,11 +2,18 @@ package middleware
import (
"context"
"errors"
"fmt"
"net/http"
"strings"
"github.com/rakyll/statik/fs"
"github.com/zitadel/logging"
"golang.org/x/text/language"
"github.com/zitadel/zitadel/internal/api/authz"
caos_errors "github.com/zitadel/zitadel/internal/errors"
"github.com/zitadel/zitadel/internal/i18n"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
)
@ -14,6 +21,7 @@ type instanceInterceptor struct {
verifier authz.InstanceVerifier
headerName string
ignoredPrefixes []string
translator *i18n.Translator
}
func InstanceInterceptor(verifier authz.InstanceVerifier, headerName string, ignoredPrefixes ...string) *instanceInterceptor {
@ -21,43 +29,40 @@ func InstanceInterceptor(verifier authz.InstanceVerifier, headerName string, ign
verifier: verifier,
headerName: headerName,
ignoredPrefixes: ignoredPrefixes,
translator: newZitadelTranslator(),
}
}
func (a *instanceInterceptor) Handler(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
for _, prefix := range a.ignoredPrefixes {
if strings.HasPrefix(r.URL.Path, prefix) {
next.ServeHTTP(w, r)
return
}
}
ctx, err := setInstance(r, a.verifier, a.headerName)
if err != nil {
http.Error(w, err.Error(), http.StatusUnauthorized)
return
}
r = r.WithContext(ctx)
next.ServeHTTP(w, r)
a.handleInstance(w, r, next)
})
}
func (a *instanceInterceptor) HandlerFunc(next http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
for _, prefix := range a.ignoredPrefixes {
if strings.HasPrefix(r.URL.Path, prefix) {
next.ServeHTTP(w, r)
return
}
}
ctx, err := setInstance(r, a.verifier, a.headerName)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
a.handleInstance(w, r, next)
}
}
func (a *instanceInterceptor) handleInstance(w http.ResponseWriter, r *http.Request, next http.Handler) {
for _, prefix := range a.ignoredPrefixes {
if strings.HasPrefix(r.URL.Path, prefix) {
next.ServeHTTP(w, r)
return
}
r = r.WithContext(ctx)
next.ServeHTTP(w, r)
}
ctx, err := setInstance(r, a.verifier, a.headerName)
if err != nil {
caosErr := new(caos_errors.NotFoundError)
if errors.As(err, &caosErr) {
caosErr.Message = a.translator.LocalizeFromRequest(r, caosErr.GetMessage(), nil)
}
http.Error(w, err.Error(), http.StatusNotFound)
return
}
r = r.WithContext(ctx)
next.ServeHTTP(w, r)
}
func setInstance(r *http.Request, verifier authz.InstanceVerifier, headerName string) (_ context.Context, err error) {
@ -89,3 +94,12 @@ func HostFromRequest(r *http.Request, headerName string) (string, error) {
}
return host, nil
}
func newZitadelTranslator() *i18n.Translator {
dir, err := fs.NewWithNamespace("zitadel")
logging.WithFields("namespace", "zitadel").OnError(err).Panic("unable to get namespace")
translator, err := i18n.NewTranslator(dir, language.English, "")
logging.OnError(err).Panic("unable to get translator")
return translator
}

View File

@ -42,7 +42,7 @@ func Test_instanceInterceptor_Handler(t *testing.T) {
request: httptest.NewRequest("", "/url", nil),
},
res{
statusCode: 403,
statusCode: 404,
context: nil,
},
},
@ -109,7 +109,7 @@ func Test_instanceInterceptor_HandlerFunc(t *testing.T) {
request: httptest.NewRequest("", "/url", nil),
},
res{
statusCode: 403,
statusCode: 404,
context: nil,
},
},
@ -229,7 +229,7 @@ type testHandler struct {
context context.Context
}
func (t *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
func (t *testHandler) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
t.context = r.Context()
}
@ -237,7 +237,7 @@ type mockInstanceVerifier struct {
host string
}
func (m *mockInstanceVerifier) InstanceByHost(ctx context.Context, host string) (authz.Instance, error) {
func (m *mockInstanceVerifier) InstanceByHost(_ context.Context, host string) (authz.Instance, error) {
if host != m.host {
return nil, fmt.Errorf("invalid host")
}

View File

@ -98,22 +98,18 @@ func Start(config Config, externalSecure bool, issuer op.IssuerFromRequest, inst
security := middleware.SecurityHeaders(csp(), nil)
handler := mux.NewRouter()
handler.Use(security)
handler.Handle(envRequestPath, middleware.TelemetryHandler()(instanceHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
instance := authz.GetInstance(r.Context())
if instance.InstanceID() == "" {
http.Error(w, "empty instanceID", http.StatusInternalServerError)
return
}
handler.Use(security, instanceHandler)
handler.Handle(envRequestPath, middleware.TelemetryHandler()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
url := http_util.BuildOrigin(r.Host, externalSecure)
environmentJSON, err := createEnvironmentJSON(url, issuer(r), instance.ConsoleClientID(), customerPortal)
environmentJSON, err := createEnvironmentJSON(url, issuer(r), authz.GetInstance(r.Context()).ConsoleClientID(), customerPortal)
if err != nil {
http.Error(w, fmt.Sprintf("unable to marshal env for console: %v", err), http.StatusInternalServerError)
return
}
_, err = w.Write(environmentJSON)
logging.OnError(err).Error("error serving environment.json")
}))))
})))
handler.SkipClean(true).PathPrefix("").Handler(cache(http.FileServer(&spaHandler{http.FS(fSys)})))
return handler, nil
}

View File

@ -400,9 +400,6 @@ func prepareInstanceDomainQuery(host string) (sq.SelectBuilder, func(*sql.Rows)
&sequence,
)
if err != nil {
if errs.Is(err, sql.ErrNoRows) {
return nil, errors.ThrowNotFound(err, "QUERY-n0wng", "Errors.IAM.NotFound")
}
return nil, errors.ThrowInternal(err, "QUERY-d9nw", "Errors.Internal")
}
if !domain.Valid {
@ -418,6 +415,9 @@ func prepareInstanceDomainQuery(host string) (sq.SelectBuilder, func(*sql.Rows)
InstanceID: instance.ID,
})
}
if instance.ID == "" {
return nil, errors.ThrowNotFound(nil, "QUERY-n0wng", "Errors.IAM.NotFound")
}
instance.DefaultLang = language.Make(lang)
if err := rows.Close(); err != nil {
return nil, errors.ThrowInternal(err, "QUERY-Dfbe2", "Errors.Query.CloseRows")

View File

@ -265,6 +265,7 @@ Errors:
NotActive: Projekt Grant ist nicht aktiv
NotInactive: Projekt Grant ist nicht inaktiv
IAM:
NotFound: Instanz nicht gefunden
Member:
RolesNotChanged: Rollen wurden nicht verändert
MemberInvalid: Member ist ungültig

View File

@ -265,6 +265,7 @@ Errors:
NotActive: Project grant is not active
NotInactive: Project grant is not inactive
IAM:
NotFound: Instance not found
Member:
RolesNotChanged: Roles have not been changed
MemberInvalid: Member is invalid

View File

@ -265,6 +265,7 @@ Errors:
NotActive: La subvention de projet n'est pas active
NotInactive: La subvention du projet n'est pas inactive
IAM:
NotFound: Instance non trouvée
Member:
RolesNotChanged: Les rôles n'ont pas été modifiés
MemberInvalid: Le membre n'est pas valide

View File

@ -265,6 +265,7 @@ Errors:
NotActive: Grant del progetto non è attivo
NotInactive: Grant del progetto non è inattivo
IAM:
NotFound: Istanza non trovata
Member:
RolesNotChanged: I ruoli non sono stati cambiati
MemberInvalid: Il membro non è valido

View File

@ -9,8 +9,8 @@ import (
io "io"
reflect "reflect"
static "github.com/zitadel/zitadel/internal/static"
gomock "github.com/golang/mock/gomock"
static "github.com/zitadel/zitadel/internal/static"
)
// MockStorage is a mock of Storage interface.