Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions changelog/unreleased/fix-proxy-createhome-dedup.md

This file was deleted.

1 change: 0 additions & 1 deletion services/proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 0 additions & 1 deletion services/proxy/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
}
Expand Down
107 changes: 30 additions & 77 deletions services/proxy/pkg/middleware/create_home.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -16,26 +14,20 @@
"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,
}
}
}
Expand All @@ -45,96 +37,57 @@
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")

Check warning on line 48 in services/proxy/pkg/middleware/create_home.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

non-canonical header "x-access-token", instead use: "X-Access-Token"

See more on https://sonarcloud.io/project/issues?id=owncloud_ocis&issues=AZ0Ac0MkXYukZyFkR7jW&open=AZ0Ac0MkXYukZyFkR7jW&pullRequest=12127

// 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)
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.next.ServeHTTP(w, req)
}

func (m *createHome) homeKnown(userID string) bool {
if m.cacheDisabled {
return false
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))
}
}
_, ok := m.knownHomes.Load(userID)
return ok
}

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
} 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")
}
}
}

return true, nil
m.next.ServeHTTP(w, req)
}

func (m *createHome) shouldServe(req *http.Request) bool {
return req.Header.Get("X-Access-Token") != ""
func (m createHome) shouldServe(req *http.Request) bool {
return req.Header.Get("x-access-token") != ""

Check warning on line 87 in services/proxy/pkg/middleware/create_home.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

non-canonical header "x-access-token", instead use: "X-Access-Token"

See more on https://sonarcloud.io/project/issues?id=owncloud_ocis&issues=AZ0Ac0MkXYukZyFkR7jX&open=AZ0Ac0MkXYukZyFkR7jX&pullRequest=12127
}

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
Expand All @@ -152,7 +105,7 @@
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
}
Expand Down
9 changes: 0 additions & 9 deletions services/proxy/pkg/middleware/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down