Make encoding user groups in access tokens configurable (#2085)

changelog/unreleased/config-encode-groups-in-tokens.md

@@ -0,0 +1,3 @@
+Enhancement: Make encoding user groups in access tokens configurable
+
+https://github.com/cs3org/reva/pull/2085

internal/grpc/interceptors/auth/auth.go

@@ -20,12 +20,15 @@ package auth
 
 import (
 	"context"
+	"time"
 
+	"github.com/bluele/gcache"
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/auth/scope"
 	ctxpkg "github.com/cs3org/reva/pkg/ctx"
 	"github.com/cs3org/reva/pkg/errtypes"
+	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/token"
 	tokenmgr "github.com/cs3org/reva/pkg/token/manager/registry"
@@ -37,6 +40,8 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+var userGroupsCache gcache.Cache
+
 type config struct {
 	// TODO(labkode): access a map is more performant as uri as fixed in length
 	// for SkipMethods.
@@ -68,6 +73,8 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI
 	}
 	conf.GatewayAddr = sharedconf.GetGatewaySVC(conf.GatewayAddr)
 
+	userGroupsCache = gcache.New(1000000).LFU().Build()
+
 	h, ok := tokenmgr.NewFuncs[conf.TokenManager]
 	if !ok {
 		return nil, errtypes.NotFound("auth: token manager does not exist: " + conf.TokenManager)
@@ -88,7 +95,7 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI
 			// to decide the storage provider.
 			tkn, ok := ctxpkg.ContextGetToken(ctx)
 			if ok {
-				u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr)
+				u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, false)
 				if err == nil {
 					ctx = ctxpkg.ContextSetUser(ctx, u)
 				}
@@ -104,7 +111,7 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI
 		}
 
 		// validate the token and ensure access to the resource is allowed
-		u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr)
+		u, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, true)
 		if err != nil {
 			log.Warn().Err(err).Msg("access token is invalid")
 			return nil, status.Errorf(codes.PermissionDenied, "auth: core access token is invalid")
@@ -128,6 +135,8 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe
 		conf.TokenManager = "jwt"
 	}
 
+	userGroupsCache = gcache.New(1000000).LFU().Build()
+
 	h, ok := tokenmgr.NewFuncs[conf.TokenManager]
 	if !ok {
 		return nil, errtypes.NotFound("auth: token manager not found: " + conf.TokenManager)
@@ -149,7 +158,7 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe
 			// to decide the storage provider.
 			tkn, ok := ctxpkg.ContextGetToken(ctx)
 			if ok {
-				u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr)
+				u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, false)
 				if err == nil {
 					ctx = ctxpkg.ContextSetUser(ctx, u)
 					ss = newWrappedServerStream(ctx, ss)
@@ -167,7 +176,7 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe
 		}
 
 		// validate the token and ensure access to the resource is allowed
-		u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr)
+		u, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, true)
 		if err != nil {
 			log.Warn().Err(err).Msg("access token is invalid")
 			return status.Errorf(codes.PermissionDenied, "auth: core access token is invalid")
@@ -194,12 +203,20 @@ func (ss *wrappedServerStream) Context() context.Context {
 	return ss.newCtx
 }
 
-func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string) (*userpb.User, error) {
+func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.Manager, gatewayAddr string, fetchUserGroups bool) (*userpb.User, error) {
 	u, tokenScope, err := mgr.DismantleToken(ctx, tkn)
 	if err != nil {
 		return nil, err
 	}
 
+	if sharedconf.SkipUserGroupsInToken() && fetchUserGroups {
+		groups, err := getUserGroups(ctx, u, gatewayAddr)
+		if err != nil {
+			return nil, err
+		}
+		u.Groups = groups
+	}
+
 	// Check if access to the resource is in the scope of the token
 	ok, err := scope.VerifyScope(tokenScope, req)
 	if err != nil {
@@ -215,3 +232,24 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.
 
 	return u, nil
 }
+
+func getUserGroups(ctx context.Context, u *userpb.User, gatewayAddr string) ([]string, error) {
+	if groupsIf, err := userGroupsCache.Get(u.Id.OpaqueId); err == nil {
+		log := appctx.GetLogger(ctx)
+		log.Info().Msgf("user groups found in cache %s", u.Id.OpaqueId)
+		return groupsIf.([]string), nil
+	}
+
+	client, err := pool.GetGatewayServiceClient(gatewayAddr)
+	if err != nil {
+		return nil, err
+	}
+
+	res, err := client.GetUserGroups(ctx, &userpb.GetUserGroupsRequest{UserId: u.Id})
+	if err != nil {
+		return nil, errors.Wrap(err, "gateway: error calling GetUserGroups")
+	}
+	_ = userGroupsCache.SetWithExpire(u.Id.OpaqueId, res.Groups, 3600*time.Second)
+
+	return res.Groups, nil
+}

internal/grpc/interceptors/auth/scope.go

@@ -107,6 +107,7 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s
 				}
 			}
 		}
+
 	} else if ref, ok := extractShareRef(req); ok {
 		// It's a share ref
 		// The request might be coming from a share created for a lightweight account

internal/grpc/services/gateway/authprovider.go

@@ -38,6 +38,7 @@ import (
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/utils"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc/metadata"
@@ -98,12 +99,17 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest
 		}, nil
 	}
 
+	u := res.User
+	if sharedconf.SkipUserGroupsInToken() {
+		u.Groups = []string{}
+	}
+
 	// We need to expand the scopes of lightweight accounts, user shares and
 	// public shares, for which we need to retrieve the receieved shares and stat
 	// the resources referenced by these. Since the current scope can do that,
 	// mint a temporary token based on that and expand the scope. Then set the
 	// token obtained from the updated scope in the context.
-	token, err := s.tokenmgr.MintToken(ctx, res.User, res.TokenScope)
+	token, err := s.tokenmgr.MintToken(ctx, u, res.TokenScope)
 	if err != nil {
 		err = errors.Wrap(err, "authsvc: error in MintToken")
 		res := &gateway.AuthenticateResponse{
@@ -123,7 +129,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest
 		}, nil
 	}
 
-	token, err = s.tokenmgr.MintToken(ctx, res.User, scope)
+	token, err = s.tokenmgr.MintToken(ctx, u, scope)
 	if err != nil {
 		err = errors.Wrap(err, "authsvc: error in MintToken")
 		res := &gateway.AuthenticateResponse{
@@ -181,6 +187,14 @@ func (s *svc) WhoAmI(ctx context.Context, req *gateway.WhoAmIRequest) (*gateway.
 		}, nil
 	}
 
+	if sharedconf.SkipUserGroupsInToken() {
+		groupsRes, err := s.GetUserGroups(ctx, &userpb.GetUserGroupsRequest{UserId: u.Id})
+		if err != nil {
+			return nil, err
+		}
+		u.Groups = groupsRes.Groups
+	}
+
 	res := &gateway.WhoAmIResponse{
 		Status: status.NewOK(ctx),
 		User:   u,

internal/grpc/services/userprovider/userprovider.go

@@ -117,7 +117,7 @@ func (s *service) Close() error {
 }
 
 func (s *service) UnprotectedEndpoints() []string {
-	return []string{"/cs3.identity.user.v1beta1.UserAPI/GetUser", "/cs3.identity.user.v1beta1.UserAPI/GetUserByClaim"}
+	return []string{"/cs3.identity.user.v1beta1.UserAPI/GetUser", "/cs3.identity.user.v1beta1.UserAPI/GetUserByClaim", "/cs3.identity.user.v1beta1.UserAPI/GetUserGroups"}
 }
 
 func (s *service) Register(ss *grpc.Server) {

internal/http/interceptors/auth/auth.go

@@ -21,8 +21,11 @@ package auth
 import (
 	"fmt"
 	"net/http"
+	"time"
 
+	"github.com/bluele/gcache"
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
+	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	"github.com/cs3org/reva/internal/http/interceptors/auth/credential/registry"
 	tokenregistry "github.com/cs3org/reva/internal/http/interceptors/auth/token/registry"
@@ -42,6 +45,8 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
+var userGroupsCache gcache.Cache
+
 type config struct {
 	Priority   int    `mapstructure:"priority"`
 	GatewaySvc string `mapstructure:"gatewaysvc"`
@@ -97,6 +102,8 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err
 		conf.CredentialsByUserAgent = map[string]string{}
 	}
 
+	userGroupsCache = gcache.New(1000000).LFU().Build()
+
 	credChain := map[string]auth.CredentialStrategy{}
 	for i, key := range conf.CredentialChain {
 		f, ok := registry.NewCredentialFuncs[conf.CredentialChain[i]]
@@ -154,6 +161,13 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err
 
 			log := appctx.GetLogger(ctx)
 
+			client, err := pool.GetGatewayServiceClient(conf.GatewaySvc)
+			if err != nil {
+				log.Error().Err(err).Msg("error getting the authsvc client")
+				w.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+
 			// skip auth for urls set in the config.
 			// TODO(labkode): maybe use method:url to bypass auth.
 			if utils.Skip(r.URL.Path, unprotected) {
@@ -203,13 +217,6 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err
 
 				log.Debug().Msgf("AuthenticateRequest: type: %s, client_id: %s against %s", req.Type, req.ClientId, conf.GatewaySvc)
 
-				client, err := pool.GetGatewayServiceClient(conf.GatewaySvc)
-				if err != nil {
-					log.Error().Err(err).Msg("error getting the authsvc client")
-					w.WriteHeader(http.StatusUnauthorized)
-					return
-				}
-
 				res, err := client.Authenticate(ctx, req)
 				if err != nil {
 					log.Error().Err(err).Msg("error calling Authenticate")
@@ -239,6 +246,24 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err
 				w.WriteHeader(http.StatusUnauthorized)
 				return
 			}
+
+			if sharedconf.SkipUserGroupsInToken() {
+				var groups []string
+				if groupsIf, err := userGroupsCache.Get(u.Id.OpaqueId); err == nil {
+					groups = groupsIf.([]string)
+				} else {
+					groupsRes, err := client.GetUserGroups(ctx, &userpb.GetUserGroupsRequest{UserId: u.Id})
+					if err != nil {
+						log.Error().Err(err).Msg("error retrieving user groups")
+						w.WriteHeader(http.StatusInternalServerError)
+						return
+					}
+					groups = groupsRes.Groups
+					_ = userGroupsCache.SetWithExpire(u.Id.OpaqueId, groupsRes.Groups, 3600*time.Second)
+				}
+				u.Groups = groups
+			}
+
 			// ensure access to the resource is allowed
 			ok, err := scope.VerifyScope(tokenScope, r.URL.Path)
 			if err != nil {

pkg/ocm/share/manager/json/json.go

@@ -596,14 +596,6 @@ func (m *mgr) ListReceivedShares(ctx context.Context) ([]*ocm.ReceivedShare, err
 		}
 		if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER && utils.UserEqual(user.Id, share.Grantee.GetUserId()) {
 			rss = append(rss, &rs)
-		} else if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP {
-			// check if all user groups match this share; TODO(labkode): filter shares created by us.
-			for _, g := range user.Groups {
-				if g == share.Grantee.GetGroupId().OpaqueId {
-					rss = append(rss, &rs)
-					break
-				}
-			}
 		}
 	}
 	return rss, nil
@@ -632,12 +624,6 @@ func (m *mgr) getReceived(ctx context.Context, ref *ocm.ShareReference) (*ocm.Re
 		if sharesEqual(ref, share) {
 			if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER && utils.UserEqual(user.Id, share.Grantee.GetUserId()) {
 				return &rs, nil
-			} else if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP {
-				for _, g := range user.Groups {
-					if share.Grantee.GetGroupId().OpaqueId == g {
-						return &rs, nil
-					}
-				}
 			}
 		}
 	}

pkg/sharedconf/sharedconf.go

@@ -28,9 +28,10 @@ import (
 var sharedConf = &conf{}
 
 type conf struct {
-	JWTSecret   string `mapstructure:"jwt_secret"`
-	GatewaySVC  string `mapstructure:"gatewaysvc"`
-	DataGateway string `mapstructure:"datagateway"`
+	JWTSecret             string `mapstructure:"jwt_secret"`
+	GatewaySVC            string `mapstructure:"gatewaysvc"`
+	DataGateway           string `mapstructure:"datagateway"`
+	SkipUserGroupsInToken bool   `mapstructure:"skip_user_groups_in_token"`
 }
 
 // Decode decodes the configuration.
@@ -86,3 +87,8 @@ func GetDataGateway(val string) string {
 	}
 	return val
 }
+
+// SkipUserGroupsInToken returns whether to skip encoding user groups in the access tokens.
+func SkipUserGroupsInToken() bool {
+	return sharedConf.SkipUserGroupsInToken
+}