fix(gateway): avoid panics

Signed-off-by: jkoberg <[email protected]>
cs3org · Nov 18, 2024 · 7a43c69 · 7a43c69
1 parent 4ce61d4
commit 7a43c69
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 8 deletions.
diff --git a/changelog/unreleased/fix-gateway-panic.md b/changelog/unreleased/fix-gateway-panic.md
@@ -0,0 +1,5 @@
+Bugfix: Avoid gateway panics
+
+The gateway would panic if there is a missing user in the context. Now it errors instead.
+
+https://github.com/cs3org/reva/issues/4953
diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go
@@ -197,7 +197,11 @@ func buildOpenInAppRequest(ctx context.Context, ri *storageprovider.ResourceInfo
 		}
 
 		// build a fake user object for the token
-		currentuser := ctxpkg.ContextMustGetUser(ctx)
+		currentuser, ok := ctxpkg.ContextGetUser(ctx)
+		if !ok {
+			return nil, errors.New("user not found in context")
+		}
+
 		scopedUser := &userpb.User{
 			Id:          ri.GetOwner(), // the owner of the resource is always set, right?
 			DisplayName: "View Only user for " + currentuser.GetUsername(),

diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go
@@ -428,7 +428,10 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
 }
 
 func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provider.GetHomeResponse, error) {
-	currentUser := ctxpkg.ContextMustGetUser(ctx)
+	currentUser, ok := ctxpkg.ContextGetUser(ctx)
+	if !ok {
+		return nil, errors.New("user not found in context")
+	}
 
 	srClient, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint)
 	if err != nil {

diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go
@@ -28,6 +28,7 @@ import (
 	sdk "github.com/cs3org/reva/v2/pkg/sdk/common"
 	"github.com/cs3org/reva/v2/pkg/storage/cache"
 	"github.com/cs3org/reva/v2/pkg/utils"
+	"github.com/pkg/errors"
 	"google.golang.org/grpc"
 )
 
@@ -44,7 +45,12 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg
 
 	spaceID := sdk.DecodeOpaqueMap(in.Opaque)["space_id"]
 
-	key := c.cache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId(), spaceID)
+	u, ok := ctxpkg.ContextGetUser(ctx)
+	if !ok {
+		return nil, errors.New("user not found in context")
+	}
+
+	key := c.cache.GetKey(u.GetId(), spaceID)
 	if key != "" {
 		s := &registry.ListStorageProvidersResponse{}
 		if err := c.cache.PullFromCache(key, s); err == nil {
@@ -89,7 +95,12 @@ type cachedSpacesAPIClient struct {
 // CreateStorageSpace creates a storage space
 func (c *cachedSpacesAPIClient) CreateStorageSpace(ctx context.Context, in *provider.CreateStorageSpaceRequest, opts ...grpc.CallOption) (*provider.CreateStorageSpaceResponse, error) {
 	if in.Type == "personal" {
-		key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId())
+		u, ok := ctxpkg.ContextGetUser(ctx)
+		if !ok {
+			return nil, errors.New("user not found in context")
+		}
+
+		key := c.createPersonalSpaceCache.GetKey(u.GetId())
 		if key != "" {
 			s := &provider.CreateStorageSpaceResponse{}
 			if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil {
@@ -132,7 +143,12 @@ type cachedAPIClient struct {
 
 // CreateHome caches calls to CreateHome locally - anyways they only need to be called once per user
 func (c *cachedAPIClient) CreateHome(ctx context.Context, in *provider.CreateHomeRequest, opts ...grpc.CallOption) (*provider.CreateHomeResponse, error) {
-	key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId())
+	u, ok := ctxpkg.ContextGetUser(ctx)
+	if !ok {
+		return nil, errors.New("user not found in context")
+	}
+
+	key := c.createPersonalSpaceCache.GetKey(u.GetId())
 	if key != "" {
 		s := &provider.CreateHomeResponse{}
 		if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil {

diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go
@@ -139,7 +139,11 @@ func (s *svc) updateShare(ctx context.Context, req *collaboration.UpdateShareReq
 	}
 
 	if s.c.CommitShareToStorageGrant {
-		creator := ctxpkg.ContextMustGetUser(ctx)
+		creator, ok := ctxpkg.ContextGetUser(ctx)
+		if !ok {
+			return nil, errors.New("user not found in context")
+		}
+
 		grant := &provider.Grant{
 			Grantee:     res.GetShare().GetGrantee(),
 			Permissions: res.GetShare().GetPermissions().GetPermissions(),
@@ -198,11 +202,16 @@ func (s *svc) updateSpaceShare(ctx context.Context, req *collaboration.UpdateSha
 			req.Share.Expiration = existsGrant.GetExpiration()
 		}
 
+		u, ok := ctxpkg.ContextGetUser(ctx)
+		if !ok {
+			return nil, errors.New("user not found in context")
+		}
+
 		grant := &provider.Grant{
 			Grantee:     req.GetShare().GetGrantee(),
 			Permissions: req.GetShare().GetPermissions().GetPermissions(),
 			Expiration:  req.GetShare().GetExpiration(),
-			Creator:     ctxpkg.ContextMustGetUser(ctx).GetId(),
+			Creator:     u.GetId(),
 		}
 
 		if grant.GetPermissions() == nil {
@@ -410,7 +419,11 @@ func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider
 		ResourceId: id,
 	}
 
-	creator := ctxpkg.ContextMustGetUser(ctx)
+	creator, ok := ctxpkg.ContextGetUser(ctx)
+	if !ok {
+		return nil, errors.New("user not found in context")
+	}
+
 	grantReq := &provider.AddGrantRequest{
 		Ref: ref,
 		Grant: &provider.Grant{