feat(proxy): account_resolver multi-tenancy

Make the account resolve reject users without a tenantid, when
multi-tenancy is enabled.
This commit is contained in:
Ralf Haferkamp
2025-09-24 17:18:56 +02:00
committed by Ralf Haferkamp
parent b1c50ea5a0
commit 8cea8c8cfd
4 changed files with 82 additions and 12 deletions

View File

@@ -370,6 +370,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config,
middleware.UserOIDCClaim(cfg.UserOIDCClaim),
middleware.UserCS3Claim(cfg.UserCS3Claim),
middleware.AutoprovisionAccounts(cfg.AutoprovisionAccounts),
middleware.MultiTenantEnabled(cfg.Commons.MultiTenantEnabled),
middleware.EventsPublisher(publisher),
),
middleware.SelectorCookie(

View File

@@ -43,6 +43,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl
userCS3Claim: options.UserCS3Claim,
userRoleAssigner: options.UserRoleAssigner,
autoProvisionAccounts: options.AutoprovisionAccounts,
multiTenantEnabled: options.MultiTenantEnabled,
lastGroupSyncCache: lastGroupSyncCache,
eventsPublisher: options.EventsPublisher,
}
@@ -56,6 +57,7 @@ type accountResolver struct {
userProvider backend.UserBackend
userRoleAssigner userroles.UserRoleAssigner
autoProvisionAccounts bool
multiTenantEnabled bool
userOIDCClaim string
userCS3Claim string
// lastGroupSyncCache is used to keep track of when the last sync of group
@@ -159,6 +161,14 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}
// if this is a multi-tenant setup, make sure the resolved user has a tenant id set
if m.multiTenantEnabled && user.GetId().GetTenantId() == "" {
m.logger.Error().Str("userid", user.Id.OpaqueId).Msg("User does not have a tenantId assigned")
w.WriteHeader(http.StatusUnauthorized)
return
}
// update user if needed
if m.autoProvisionAccounts {
if err = m.userProvider.UpdateUserIfNeeded(req.Context(), user, claims); err != nil {
m.logger.Error().Err(err).Str("userid", user.GetId().GetOpaqueId()).Interface("claims", claims).Msg("Failed to update autoprovisioned user")
@@ -201,6 +211,13 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
m.logger.Debug().Interface("claims", claims).Interface("user", user).Msg("associated claims with user")
} else if user != nil && !hasToken {
// if this is a multi-tenant setup, make sure the resolved user has a tenant id set
if m.multiTenantEnabled && user.GetId().GetTenantId() == "" {
m.logger.Error().Str("userid", user.Id.OpaqueId).Msg("User does not have a tenantId assigned")
w.WriteHeader(http.StatusUnauthorized)
return
}
// If we already have a token (e.g. the app auth middleware adds the token to the context) there is no need
// to get yet another one here.
var err error

View File

@@ -24,7 +24,7 @@ func TestTokenIsAddedWithMailClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "foo@example.com",
}, nil, oidc.Email, "mail")
}, nil, oidc.Email, "mail", false)
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
@@ -42,7 +42,7 @@ func TestTokenIsAddedWithUsernameClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "foo@example.com",
}, nil, oidc.PreferredUsername, "username")
}, nil, oidc.PreferredUsername, "username", false)
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
@@ -61,7 +61,7 @@ func TestTokenIsAddedWithDotUsernamePathClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "foo@example.com",
}, nil, "li.un", "username")
}, nil, "li.un", "username", false)
// This is how lico adds the username to the access token
req, rw := mockRequest(map[string]interface{}{
@@ -83,7 +83,7 @@ func TestTokenIsAddedWithDotEscapedUsernameClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "foo@example.com",
}, nil, "li\\.un", "username")
}, nil, "li\\.un", "username", false)
// This tests the . escaping of the readUserIDClaim
req, rw := mockRequest(map[string]interface{}{
@@ -103,7 +103,7 @@ func TestTokenIsAddedWithDottedUsernameClaimFallback(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "foo@example.com",
}, nil, "li.un", "username")
}, nil, "li.un", "username", false)
// This tests the . escaping fallback of the readUserIDClaim
req, rw := mockRequest(map[string]interface{}{
@@ -120,7 +120,7 @@ func TestTokenIsAddedWithDottedUsernameClaimFallback(t *testing.T) {
}
func TestNSkipOnNoClaims(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.Email, "mail")
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.Email, "mail", false)
req, rw := mockRequest(nil)
sut.ServeHTTP(rw, req)
@@ -131,7 +131,7 @@ func TestNSkipOnNoClaims(t *testing.T) {
}
func TestUnauthorizedOnUserNotFound(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.PreferredUsername, "username")
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.PreferredUsername, "username", false)
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
@@ -145,7 +145,7 @@ func TestUnauthorizedOnUserNotFound(t *testing.T) {
}
func TestUnauthorizedOnUserDisabled(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.PreferredUsername, "username")
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.PreferredUsername, "username", false)
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
@@ -159,7 +159,7 @@ func TestUnauthorizedOnUserDisabled(t *testing.T) {
}
func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.Email, "mail")
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.Email, "mail", false)
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
})
@@ -171,7 +171,49 @@ func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
assert.Equal(t, http.StatusInternalServerError, rw.Code)
}
func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error, oidcclaim, cs3claim string) http.Handler {
func TestUnauthorizedOnMissingTenantId(t *testing.T) {
sut := newMockAccountResolver(
&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Username: "foo",
},
nil, oidc.PreferredUsername, "username", true)
req, rw := mockRequest(map[string]any{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
})
sut.ServeHTTP(rw, req)
token := req.Header.Get(revactx.TokenHeader)
assert.Empty(t, token)
assert.Equal(t, http.StatusUnauthorized, rw.Code)
}
func TestTokenIsAddedWhenUserHasTenantId(t *testing.T) {
sut := newMockAccountResolver(
&userv1beta1.User{
Id: &userv1beta1.UserId{
Idp: "https://idx.example.com",
OpaqueId: "123",
TenantId: "tenant1",
},
Username: "foo",
},
nil, oidc.PreferredUsername, "username", true)
req, rw := mockRequest(map[string]any{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
})
sut.ServeHTTP(rw, req)
token := req.Header.Get(revactx.TokenHeader)
assert.NotEmpty(t, token)
assert.Contains(t, token, "eyJ")
}
func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error, oidcclaim, cs3claim string, multiTenant bool) http.Handler {
tokenManager, _ := jwt.New(map[string]interface{}{
"secret": "change-me",
"expires": int64(60),
@@ -198,6 +240,7 @@ func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr
UserOIDCClaim(oidcclaim),
UserCS3Claim(cs3claim),
AutoprovisionAccounts(false),
MultiTenantEnabled(multiTenant),
)(mockHandler{})
}

View File

@@ -70,8 +70,10 @@ type Options struct {
// TraceProvider sets the tracing provider.
TraceProvider trace.TracerProvider
// SkipUserInfo prevents the oidc middleware from querying the userinfo endpoint and read any claims directly from the access token instead
SkipUserInfo bool
EventsPublisher events.Publisher
SkipUserInfo bool
// MultiTenantEnabled causes the account resolve middleware to reject users that don't have a tenant id assigned
MultiTenantEnabled bool
EventsPublisher events.Publisher
}
// newOptions initializes the available default options.
@@ -239,6 +241,13 @@ func SkipUserInfo(val bool) Option {
}
}
// MultiTenantEnabled sets the MultiTenantEnabled flag.
func MultiTenantEnabled(val bool) Option {
return func(o *Options) {
o.MultiTenantEnabled = val
}
}
// EventsPublisher sets the events publisher.
func EventsPublisher(ep events.Publisher) Option {
return func(o *Options) {