From ce23ab36cb58871e469c8fc4e69e2ea9973a7074 Mon Sep 17 00:00:00 2001 From: Ben Toogood Date: Thu, 2 Apr 2020 18:41:06 +0100 Subject: [PATCH] Improve Err Handling --- auth/service/service.go | 1 - service.go | 2 +- util/wrapper/wrapper.go | 36 +++++++++++++++++++++++++++++++----- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/auth/service/service.go b/auth/service/service.go index 166886d1..57d6082e 100644 --- a/auth/service/service.go +++ b/auth/service/service.go @@ -198,7 +198,6 @@ func (s *svc) Verify(acc *auth.Account, res *auth.Resource) error { // no rules were found for the resource, default to denying access log.Infof("%v:%v denied access to %v:%v:%v:%v by lack of rule (%v rules found for namespace)", acc.Namespace, logID, res.Namespace, res.Type, res.Name, res.Endpoint, len(s.listRules(res.Namespace))) - fmt.Println(s.rules) return auth.ErrForbidden } diff --git a/service.go b/service.go index a13b4e19..2feb5db4 100644 --- a/service.go +++ b/service.go @@ -35,7 +35,7 @@ func newService(opts ...Option) Service { serviceName := options.Server.Options().Name // TODO: better accessors - authFn := func() auth.Auth { return service.opts.Auth } + authFn := func() auth.Auth { return options.Auth } // wrap client to inject From-Service header on any calls options.Client = wrapper.FromService(serviceName, options.Client, authFn) diff --git a/util/wrapper/wrapper.go b/util/wrapper/wrapper.go index f795e705..a2878a20 100644 --- a/util/wrapper/wrapper.go +++ b/util/wrapper/wrapper.go @@ -2,6 +2,7 @@ package wrapper import ( "context" + "fmt" "strings" "github.com/micro/go-micro/v2/auth" @@ -9,6 +10,7 @@ import ( "github.com/micro/go-micro/v2/debug/stats" "github.com/micro/go-micro/v2/debug/trace" "github.com/micro/go-micro/v2/errors" + "github.com/micro/go-micro/v2/logger" "github.com/micro/go-micro/v2/metadata" "github.com/micro/go-micro/v2/server" ) @@ -165,24 +167,48 @@ func AuthHandler(fn func() auth.Auth) server.HandlerWrapper { if header, ok := metadata.Get(ctx, "Authorization"); ok { // Ensure the correct scheme is being used if !strings.HasPrefix(header, auth.BearerScheme) { - return errors.Unauthorized("go.micro.auth", "invalid authorization header. expected Bearer schema") + return errors.Unauthorized(req.Service(), "invalid authorization header. expected Bearer schema") } token = header[len(auth.BearerScheme):] } + // Get the namespace for the request + namespace, ok := metadata.Get(ctx, auth.NamespaceKey) + if !ok { + logger.Errorf("Missing request namespace") + namespace = auth.DefaultNamespace + } + fmt.Printf("Namespace is %v\n", namespace) + // Inspect the token and get the account account, err := a.Inspect(token) if err != nil { - account = &auth.Account{} + account = &auth.Account{Namespace: auth.DefaultNamespace} + } + + // Check the accounts namespace matches the namespace we're operating + // within. If not forbid the request and log the occurance. + if account.Namespace != namespace { + logger.Warnf("Cross namespace request forbidden: account %v (%v) requested access to %v %v in the %v namespace", + account.ID, account.Namespace, req.Service(), req.Endpoint(), namespace) + return errors.Forbidden(req.Service(), "cross namespace request") + } + + // construct the resource + res := &auth.Resource{ + Type: "service", + Name: req.Service(), + Endpoint: req.Endpoint(), + Namespace: namespace, } // Verify the caller has access to the resource - err = a.Verify(account, &auth.Resource{Type: "service", Name: req.Service(), Endpoint: req.Endpoint()}) + err = a.Verify(account, res) if err != nil && len(account.ID) > 0 { - return errors.Forbidden("go.micro.auth", "Forbidden call made to %v:%v by %v", req.Service(), req.Endpoint(), account.ID) + return errors.Forbidden(req.Service(), "Forbidden call made to %v:%v by %v", req.Service(), req.Endpoint(), account.ID) } else if err != nil { - return errors.Unauthorized("go.micro.auth", "Unauthorised call made to %v:%v", req.Service(), req.Endpoint()) + return errors.Unauthorized(req.Service(), "Unauthorised call made to %v:%v", req.Service(), req.Endpoint()) } // There is an account, set it in the context