Revert "fix(proxy): [OCISDEV-460] deduplicate CreateHome calls in middleware"

changelog/unreleased/fix-proxy-createhome-dedup.md

@@ -1,11 +0,0 @@
-Bugfix: Deduplicate CreateHome calls in proxy middleware
-
-The CreateHome proxy middleware previously fired a CreateHome gRPC
-request on every authenticated HTTP request with no deduplication.
-On first login, the browser sends many parallel requests, each
-triggering a redundant CreateHome call. This change uses singleflight
-to collapse concurrent calls for the same user and caches successful
-results in-process so subsequent requests skip the gRPC call entirely.
-
-https://github.com/owncloud/ocis/pull/12115
-https://github.com/owncloud/ocis/issues/12068

services/proxy/pkg/command/server.go

@@ -373,7 +373,6 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config,
 			middleware.Logger(logger),
 			middleware.WithRevaGatewaySelector(gatewaySelector),
 			middleware.RoleQuotas(cfg.RoleQuotas),
-			middleware.CreateHomeCacheDisabled(cfg.CreateHomeCacheDisabled),
 		),
 		// trigger space assignment when a user logs in
 		middleware.SpaceManager(

services/proxy/pkg/config/config.go

@@ -48,7 +48,6 @@ type Config struct {
 	ClaimSpaceManagement      ClaimSpaceManagement `yaml:"claim_space_management"`
 	MultiFactorAuthentication MFAConfig            `yaml:"mfa"`
 	MultiInstance             MultiInstanceConfig  `yaml:"multi_instance"`
-	CreateHomeCacheDisabled   bool                 `yaml:"create_home_cache_disabled" env:"PROXY_CREATEHOME_CACHE_DISABLED" desc:"Disable the process-lifetime cache that tracks which users already have a personal space. When enabled, the proxy skips redundant CreateHome gRPC calls for returning users. Set this to true as a fallback if the cache causes unexpected behavior." introductionVersion:"Daledda"`
 
 	Context context.Context `json:"-" yaml:"-"`
 }

services/proxy/pkg/middleware/create_home.go

@@ -1,10 +1,8 @@
 package middleware
 
 import (
-	"context"
 	"net/http"
 	"strconv"
-	"sync"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -16,26 +14,20 @@ import (
 	"github.com/owncloud/reva/v2/pkg/rgrpc/status"
 	"github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/owncloud/reva/v2/pkg/utils"
-	"golang.org/x/sync/singleflight"
 	"google.golang.org/grpc/metadata"
 )
 
-// CreateHome provides a middleware which sends a CreateHome request to the reva gateway.
-// It deduplicates concurrent requests for the same user and caches successful results
-// for the lifetime of the process.
+// CreateHome provides a middleware which sends a CreateHome request to the reva gateway
 func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler {
 	options := newOptions(optionSetters...)
 	logger := options.Logger
 
-	cacheDisabled := options.CreateHomeCacheDisabled
-
 	return func(next http.Handler) http.Handler {
 		return &createHome{
 			next:                next,
 			logger:              logger,
 			revaGatewaySelector: options.RevaGatewaySelector,
 			roleQuotas:          options.RoleQuotas,
-			cacheDisabled:       cacheDisabled,
 		}
 	}
 }
@@ -45,96 +37,57 @@ type createHome struct {
 	logger              log.Logger
 	revaGatewaySelector pool.Selectable[gateway.GatewayAPIClient]
 	roleQuotas          map[string]uint64
-	cacheDisabled       bool
-
-	knownHomes sync.Map           // map[userID]struct{} — users whose home is confirmed
-	flight     singleflight.Group // collapses concurrent CreateHome calls per user
 }
 
-func (m *createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if !m.shouldServe(req) {
 		m.next.ServeHTTP(w, req)
 		return
 	}
 
-	token := req.Header.Get("X-Access-Token")
+	token := req.Header.Get("x-access-token")
 
 	// we need to pass the token to authenticate the CreateHome request.
+	//ctx := tokenpkg.ContextSetToken(r.Context(), token)
 	ctx := metadata.AppendToOutgoingContext(req.Context(), revactx.TokenHeader, token)
 
-	u, ok := revactx.ContextGetUser(ctx)
-	if !ok {
-		m.next.ServeHTTP(w, req)
-		return
-	}
-
-	userID := u.Id.OpaqueId
-
-	// Fast path: home already known to exist for this user.
-	if m.homeKnown(userID) {
-		m.next.ServeHTTP(w, req)
-		return
-	}
-
 	createHomeReq := &provider.CreateHomeRequest{}
-	roleIDs, err := m.getUserRoles(u)
+	u, ok := revactx.ContextGetUser(ctx)
+	if ok {
+		roleIDs, err := m.getUserRoles(u)
+		if err != nil {
+			m.logger.Error().Err(err).Str("userid", u.Id.OpaqueId).Msg("failed to get roles for user")
+			errorcode.GeneralException.Render(w, req, http.StatusInternalServerError, "Unauthorized")
+			return
+		}
+		if limit, hasLimit := m.checkRoleQuotaLimit(roleIDs); hasLimit {
+			createHomeReq.Opaque = utils.AppendPlainToOpaque(nil, "quota", strconv.FormatUint(limit, 10))
+		}
+	}
+
+	client, err := m.revaGatewaySelector.Next()
 	if err != nil {
-		m.logger.Error().Err(err).Str("userid", userID).Msg("failed to get roles for user")
-		errorcode.GeneralException.Render(w, req, http.StatusInternalServerError, "Unauthorized")
-		return
-	}
-	if limit, hasLimit := m.checkRoleQuotaLimit(roleIDs); hasLimit {
-		createHomeReq.Opaque = utils.AppendPlainToOpaque(nil, "quota", strconv.FormatUint(limit, 10))
-	}
-
-	// Deduplicate concurrent CreateHome calls for the same user.
-	_, err, _ = m.flight.Do(userID, func() (interface{}, error) {
-		return m.createHome(ctx, createHomeReq)
-	})
-
-	// Only cache on success — if CreateHome failed, retry on next request.
-	if err == nil && !m.cacheDisabled {
-		m.knownHomes.Store(userID, struct{}{})
+		m.logger.Err(err).Msg("error selecting next gateway client")
+	} else {
+		createHomeRes, err := client.CreateHome(ctx, createHomeReq)
+		if err != nil {
+			m.logger.Err(err).Msg("error calling CreateHome")
+		} else if createHomeRes.Status.Code != rpc.Code_CODE_OK {
+			err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway")
+			if createHomeRes.Status.Code != rpc.Code_CODE_ALREADY_EXISTS {
+				m.logger.Err(err).Msg("error when calling Createhome")
+			}
+		}
 	}
 
 	m.next.ServeHTTP(w, req)
 }
 
-func (m *createHome) homeKnown(userID string) bool {
-	if m.cacheDisabled {
-		return false
-	}
-	_, ok := m.knownHomes.Load(userID)
-	return ok
+func (m createHome) shouldServe(req *http.Request) bool {
+	return req.Header.Get("x-access-token") != ""
 }
 
-func (m *createHome) createHome(ctx context.Context, req *provider.CreateHomeRequest) (interface{}, error) {
-	client, err := m.revaGatewaySelector.Next()
-	if err != nil {
-		m.logger.Err(err).Msg("error selecting next gateway client")
-		return nil, err
-	}
-
-	createHomeRes, err := client.CreateHome(ctx, req)
-	if err != nil {
-		m.logger.Err(err).Msg("error calling CreateHome")
-		return nil, err
-	}
-
-	if createHomeRes.Status.Code != rpc.Code_CODE_OK && createHomeRes.Status.Code != rpc.Code_CODE_ALREADY_EXISTS {
-		err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway")
-		m.logger.Err(err).Msg("error when calling CreateHome")
-		return nil, err
-	}
-
-	return true, nil
-}
-
-func (m *createHome) shouldServe(req *http.Request) bool {
-	return req.Header.Get("X-Access-Token") != ""
-}
-
-func (m *createHome) getUserRoles(user *userv1beta1.User) ([]string, error) {
+func (m createHome) getUserRoles(user *userv1beta1.User) ([]string, error) {
 	var roleIDs []string
 	if err := utils.ReadJSONFromOpaque(user.Opaque, "roles", &roleIDs); err != nil {
 		return nil, err
@@ -152,7 +105,7 @@ func (m *createHome) getUserRoles(user *userv1beta1.User) ([]string, error) {
 	return dedup, nil
 }
 
-func (m *createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) {
+func (m createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) {
 	if len(roleIDs) == 0 {
 		return 0, false
 	}

services/proxy/pkg/middleware/options.go

@@ -77,8 +77,6 @@ type Options struct {
 	// Service Accounts
 	ServiceAccountID     string
 	ServiceAccountSecret string
-	// CreateHomeCacheDisabled disables the knownHomes cache in the CreateHome middleware
-	CreateHomeCacheDisabled bool
 	// Multi-Instance Options
 	MultiInstanceEnabled bool
 	InstanceID           string
@@ -238,13 +236,6 @@ func AccessTokenVerifyMethod(method string) Option {
 	}
 }
 
-// CreateHomeCacheDisabled provides a function to disable the CreateHome cache.
-func CreateHomeCacheDisabled(val bool) Option {
-	return func(o *Options) {
-		o.CreateHomeCacheDisabled = val
-	}
-}
-
 // RoleQuotas sets the role quota mapping setting
 func RoleQuotas(roleQuotas map[string]uint64) Option {
 	return func(o *Options) {