From f6be8c98565f113c8ceb11a9e3e19f0a3ed80c9f Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 4 May 2021 15:33:31 +0200 Subject: [PATCH] Move token verification to auth interceptors --- internal/grpc/interceptors/auth/auth.go | 114 ++++++++---------- .../grpc/services/gateway/authprovider.go | 2 +- internal/http/interceptors/auth/auth.go | 14 ++- pkg/auth/scope/publicsharepath.go | 64 ---------- pkg/auth/scope/scope.go | 5 +- pkg/token/manager/demo/demo.go | 11 +- pkg/token/manager/demo/demo_test.go | 2 +- pkg/token/manager/jwt/jwt.go | 32 +---- pkg/token/token.go | 3 +- 9 files changed, 72 insertions(+), 175 deletions(-) delete mode 100644 pkg/auth/scope/publicsharepath.go diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index 60a81d39d5f..9e793a560ce 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -23,14 +23,13 @@ import ( "fmt" "strings" - authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/appctx" + "github.com/cs3org/reva/pkg/auth/scope" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/sharedconf" @@ -117,7 +116,7 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI return nil, status.Errorf(codes.Unauthenticated, "auth: core access token not found") } - // validate the token + // validate the token and ensure access to the resource is allowed u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr) if err != nil { log.Warn().Err(err).Msg("access token is invalid") @@ -188,7 +187,7 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe return status.Errorf(codes.Unauthenticated, "auth: core access token not found") } - // validate the token + // validate the token and ensure access to the resource is allowed u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr) if err != nil { log.Warn().Err(err).Msg("access token is invalid") @@ -217,74 +216,65 @@ func (ss *wrappedServerStream) Context() context.Context { } func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string) (*userpb.User, error) { - u, scope, err := mgr.DismantleToken(ctx, tkn, req) log := appctx.GetLogger(ctx) + u, tokenScope, err := mgr.DismantleToken(ctx, tkn) + if err != nil { + return nil, err + } - // Check if the err returned is PermissionDenied - if _, ok := err.(errtypes.PermissionDenied); ok { - // Check if req is of type *provider.Reference_Path - // If yes, the request might be coming from a share where the accessor is - // trying to impersonate the owner, since the share manager doesn't know the - // share path. - if ref, ok := extractRef(req); ok { - if ref.GetPath() != "" { - - // Try to extract the resource ID from the scope resource. - // Currently, we only check for public shares, but this will be extended - // for OCM shares, guest accounts, etc. - log.Info().Msgf("resolving path reference to ID to check token scope %+v", ref.GetPath()) - var share link.PublicShare - err = utils.UnmarshalJSONToProtoV1(scope["publicshare"].Resource.Value, &share) - if err != nil { - return nil, err - } + // Check if access to the resource is in the scope of the token + ok, err := scope.VerifyScope(tokenScope, req) + if err != nil { + return nil, errtypes.InternalError("error verifying scope of access token") + } + if ok { + return u, nil + } - client, err := pool.GetGatewayServiceClient(gatewayAddr) - if err != nil { - return nil, err - } + // Check if req is of type *provider.Reference_Path + // If yes, the request might be coming from a share where the accessor is + // trying to impersonate the owner, since the share manager doesn't know the + // share path. + if ref, ok := extractRef(req); ok { + if ref.GetPath() != "" { + + // Try to extract the resource ID from the scope resource. + // Currently, we only check for public shares, but this will be extended + // for OCM shares, guest accounts, etc. + log.Info().Msgf("resolving path reference to ID to check token scope %+v", ref.GetPath()) + var share link.PublicShare + err = utils.UnmarshalJSONToProtoV1(tokenScope["publicshare"].Resource.Value, &share) + if err != nil { + return nil, err + } - // Since the public share is obtained from the scope, the current token - // has access to it. - statReq := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{Id: share.ResourceId}, - }, - } + client, err := pool.GetGatewayServiceClient(gatewayAddr) + if err != nil { + return nil, err + } - statResponse, err := client.Stat(ctx, statReq) - if err != nil || statResponse.Status.Code != rpc.Code_CODE_OK { - return nil, err - } + // Since the public share is obtained from the scope, the current token + // has access to it. + statReq := &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{Id: share.ResourceId}, + }, + } - if strings.HasPrefix(ref.GetPath(), statResponse.Info.Path) { - // The path corresponds to the resource to which the token has access. - // Add it to the scope map - val, err := utils.MarshalProtoV1ToJSON(&provider.Reference{ - Spec: &provider.Reference_Path{Path: statResponse.Info.Path}, - }) - if err != nil { - return nil, err - } - scopeVal := &authpb.Scope{ - Resource: &types.OpaqueEntry{ - Decoder: "json", - Value: val, - }, - Role: scope["publicshare"].Role, - } - - tkn, err = mgr.AddScopeToToken(ctx, tkn, "publicshare:path", scopeVal) - if err != nil { - return nil, err - } - return dismantleToken(ctx, tkn, req, mgr, gatewayAddr) - } + statResponse, err := client.Stat(ctx, statReq) + if err != nil || statResponse.Status.Code != rpc.Code_CODE_OK { + return nil, err + } + + if strings.HasPrefix(ref.GetPath(), statResponse.Info.Path) { + // The path corresponds to the resource to which the token has access. + // We allow access to it. + return u, nil } } } - return u, err + return nil, err } func extractRef(req interface{}) (*provider.Reference, bool) { diff --git a/internal/grpc/services/gateway/authprovider.go b/internal/grpc/services/gateway/authprovider.go index 6cdd0432dba..4354e8fc731 100644 --- a/internal/grpc/services/gateway/authprovider.go +++ b/internal/grpc/services/gateway/authprovider.go @@ -143,7 +143,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest } func (s *svc) WhoAmI(ctx context.Context, req *gateway.WhoAmIRequest) (*gateway.WhoAmIResponse, error) { - u, _, err := s.tokenmgr.DismantleToken(ctx, req.Token, nil) + u, _, err := s.tokenmgr.DismantleToken(ctx, req.Token) if err != nil { err = errors.Wrap(err, "gateway: error getting user from token") return &gateway.WhoAmIResponse{ diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 89d7edddf4d..a8e8d90e2bc 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -29,6 +29,7 @@ import ( tokenwriterregistry "github.com/cs3org/reva/internal/http/interceptors/auth/tokenwriter/registry" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/auth" + "github.com/cs3org/reva/pkg/auth/scope" "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/rhttp/global" @@ -233,12 +234,23 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err } // validate token - u, _, err := tokenManager.DismantleToken(r.Context(), tkn, r.URL.Path) + u, tokenScope, err := tokenManager.DismantleToken(r.Context(), tkn) if err != nil { log.Error().Err(err).Msg("error dismantling token") w.WriteHeader(http.StatusUnauthorized) return } + // ensure access to the resource is allowed + ok, err := scope.VerifyScope(tokenScope, r.URL.Path) + if err != nil { + log.Error().Err(err).Msg("error verifying scope of access token") + w.WriteHeader(http.StatusInternalServerError) + } + if !ok { + log.Error().Err(err).Msg("access to resource not allowed") + w.WriteHeader(http.StatusUnauthorized) + return + } // store user and core access token in context. ctx = user.ContextSetUser(ctx, u) diff --git a/pkg/auth/scope/publicsharepath.go b/pkg/auth/scope/publicsharepath.go deleted file mode 100644 index 118d8fc8877..00000000000 --- a/pkg/auth/scope/publicsharepath.go +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2018-2021 CERN -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// In applying this license, CERN does not waive the privileges and immunities -// granted to it by virtue of its status as an Intergovernmental Organization -// or submit itself to any jurisdiction. - -package scope - -import ( - "fmt" - "strings" - - authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" - "github.com/cs3org/reva/pkg/errtypes" - "github.com/cs3org/reva/pkg/utils" -) - -func publicsharepathScope(scope *authpb.Scope, resource interface{}) (bool, error) { - var ref provider.Reference - err := utils.UnmarshalJSONToProtoV1(scope.Resource.Value, &ref) - if err != nil { - return false, err - } - - switch v := resource.(type) { - // Viewer role - case *registry.GetStorageProvidersRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - case *provider.StatRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - case *provider.ListContainerRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - case *provider.InitiateFileDownloadRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - - // Editor role - // TODO(ishank011): Add role checks, - // need to return appropriate status codes in the ocs/ocdav layers. - case *provider.CreateContainerRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - case *provider.DeleteRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - case *provider.MoveRequest: - return strings.HasPrefix(v.GetSource().GetPath(), ref.GetPath()) && strings.HasPrefix(v.GetDestination().GetPath(), ref.GetPath()), nil - case *provider.InitiateFileUploadRequest: - return strings.HasPrefix(v.GetRef().GetPath(), ref.GetPath()), nil - } - - return false, errtypes.InternalError(fmt.Sprintf("resource type assertion failed: %+v", resource)) -} diff --git a/pkg/auth/scope/scope.go b/pkg/auth/scope/scope.go index 27e42059f61..42ba65c11f7 100644 --- a/pkg/auth/scope/scope.go +++ b/pkg/auth/scope/scope.go @@ -29,9 +29,8 @@ import ( type Verifier func(*authpb.Scope, interface{}) (bool, error) var supportedScopes = map[string]Verifier{ - "user": userScope, - "publicshare": publicshareScope, - "publicshare:path": publicsharepathScope, + "user": userScope, + "publicshare": publicshareScope, } // VerifyScope is the function to be called when dismantling tokens to check if diff --git a/pkg/token/manager/demo/demo.go b/pkg/token/manager/demo/demo.go index 07a95b50007..a200de0898c 100644 --- a/pkg/token/manager/demo/demo.go +++ b/pkg/token/manager/demo/demo.go @@ -56,7 +56,7 @@ func (m *manager) MintToken(ctx context.Context, u *user.User, scope map[string] return token, nil } -func (m *manager) DismantleToken(ctx context.Context, token string, resource interface{}) (*user.User, map[string]*auth.Scope, error) { +func (m *manager) DismantleToken(ctx context.Context, token string) (*user.User, map[string]*auth.Scope, error) { c, err := decode(token) if err != nil { return nil, nil, errors.Wrap(err, "error decoding claims") @@ -64,15 +64,6 @@ func (m *manager) DismantleToken(ctx context.Context, token string, resource int return c.User, c.Scope, nil } -func (m *manager) AddScopeToToken(ctx context.Context, token string, scopeKey string, scope *auth.Scope) (string, error) { - c, err := decode(token) - if err != nil { - return "", errors.Wrap(err, "error decoding claims") - } - c.Scope[scopeKey] = scope - return encode(c) -} - // from https://stackoverflow.com/questions/28020070/golang-serialize-and-deserialize-back // go binary encoder func encode(c *claims) (string, error) { diff --git a/pkg/token/manager/demo/demo_test.go b/pkg/token/manager/demo/demo_test.go index ce36946072a..964b49fe472 100644 --- a/pkg/token/manager/demo/demo_test.go +++ b/pkg/token/manager/demo/demo_test.go @@ -61,7 +61,7 @@ func TestEncodeDecode(t *testing.T) { t.Fatal(err) } - decodedUser, decodedScope, err := m.DismantleToken(ctx, encoded, nil) + decodedUser, decodedScope, err := m.DismantleToken(ctx, encoded) if err != nil { t.Fatal(err) } diff --git a/pkg/token/manager/jwt/jwt.go b/pkg/token/manager/jwt/jwt.go index 11b1e3d5768..d31cbfafc65 100644 --- a/pkg/token/manager/jwt/jwt.go +++ b/pkg/token/manager/jwt/jwt.go @@ -24,7 +24,6 @@ import ( auth "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - "github.com/cs3org/reva/pkg/auth/scope" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/sharedconf" "github.com/cs3org/reva/pkg/token" @@ -109,7 +108,7 @@ func (m *manager) MintToken(ctx context.Context, u *user.User, scope map[string] return tkn, nil } -func (m *manager) DismantleToken(ctx context.Context, tkn string, resource interface{}) (*user.User, map[string]*auth.Scope, error) { +func (m *manager) DismantleToken(ctx context.Context, tkn string) (*user.User, map[string]*auth.Scope, error) { token, err := jwt.ParseWithClaims(tkn, &claims{}, func(token *jwt.Token) (interface{}, error) { return []byte(m.conf.Secret), nil }) @@ -119,37 +118,8 @@ func (m *manager) DismantleToken(ctx context.Context, tkn string, resource inter } if claims, ok := token.Claims.(*claims); ok && token.Valid { - ok, err = scope.VerifyScope(claims.Scope, resource) - if err != nil { - return nil, nil, errtypes.InternalError("error verifying scope of access token") - } - if !ok { - // Pass the allowed scope of the token. This might be needed for the - // path/resource ID resolution, because when the token was minted, the auth provider - // might be aware of only one of these references. In such cases, it's expectec that - // the caller will resolve the reference and pass the expected resource. - return nil, claims.Scope, errtypes.PermissionDenied("token missing necessary scope access") - } return claims.User, claims.Scope, nil } return nil, nil, errtypes.InvalidCredentials("invalid token") } - -func (m *manager) AddScopeToToken(ctx context.Context, tkn string, scopeKey string, scope *auth.Scope) (string, error) { - token, err := jwt.ParseWithClaims(tkn, &claims{}, func(token *jwt.Token) (interface{}, error) { - return []byte(m.conf.Secret), nil - }) - - if err != nil { - return "", errors.Wrap(err, "error parsing token") - } - - if claims, ok := token.Claims.(*claims); ok && token.Valid { - claims.Scope[scopeKey] = scope - return m.MintToken(ctx, claims.User, claims.Scope) - } - - return "", errtypes.InvalidCredentials("invalid token") - -} diff --git a/pkg/token/token.go b/pkg/token/token.go index 3738dec53f3..61b81533e77 100644 --- a/pkg/token/token.go +++ b/pkg/token/token.go @@ -36,8 +36,7 @@ const tokenKey key = iota // Manager is the interface to implement to sign and verify tokens type Manager interface { MintToken(ctx context.Context, u *user.User, scope map[string]*auth.Scope) (string, error) - DismantleToken(ctx context.Context, token string, resource interface{}) (*user.User, map[string]*auth.Scope, error) - AddScopeToToken(ctx context.Context, token string, scopeKey string, scope *auth.Scope) (string, error) + DismantleToken(ctx context.Context, token string) (*user.User, map[string]*auth.Scope, error) } // ContextGetToken returns the token if set in the given context.