feat(multi-tenancy): verify tenant via OIDC claim
When multi-tenancy is enable we now allow to specify an OIDC claim against which the tenantid of the user resolved via CS3 apis is matched. Partial: #2310
This commit is contained in:
committed by
Ralf Haferkamp
parent
a1d4e86b96
commit
33f45fa965
@@ -42,6 +42,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl
|
||||
userProvider: options.UserProvider,
|
||||
userOIDCClaim: options.UserOIDCClaim,
|
||||
userCS3Claim: options.UserCS3Claim,
|
||||
tenantOIDCClaim: options.TenantOIDCClaim,
|
||||
userRoleAssigner: options.UserRoleAssigner,
|
||||
autoProvisionAccounts: options.AutoprovisionAccounts,
|
||||
multiTenantEnabled: options.MultiTenantEnabled,
|
||||
@@ -61,6 +62,7 @@ type accountResolver struct {
|
||||
multiTenantEnabled bool
|
||||
userOIDCClaim string
|
||||
userCS3Claim string
|
||||
tenantOIDCClaim string
|
||||
// lastGroupSyncCache is used to keep track of when the last sync of group
|
||||
// memberships was done for a specific user. This is used to trigger a sync
|
||||
// with every single request.
|
||||
@@ -68,7 +70,7 @@ type accountResolver struct {
|
||||
eventsPublisher events.Publisher
|
||||
}
|
||||
|
||||
func readUserIDClaim(path string, claims map[string]interface{}) (string, error) {
|
||||
func readStringClaim(path string, claims map[string]interface{}) (string, error) {
|
||||
// happy path
|
||||
value, _ := claims[path].(string)
|
||||
if value != "" {
|
||||
@@ -118,7 +120,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
}
|
||||
|
||||
if user == nil && claims != nil {
|
||||
value, err := readUserIDClaim(m.userOIDCClaim, claims)
|
||||
value, err := readStringClaim(m.userOIDCClaim, claims)
|
||||
if err != nil {
|
||||
m.logger.Error().Err(err).Msg("could not read user id claim")
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
@@ -169,6 +171,15 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// if a tenant claim is configured, verify it matches the tenant id on the resolved user
|
||||
if m.tenantOIDCClaim != "" {
|
||||
if err = m.verifyTenantClaim(user.GetId().GetTenantId(), claims); err != nil {
|
||||
m.logger.Error().Err(err).Str("userid", user.GetId().GetOpaqueId()).Msg("Tenant claim mismatch")
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// update user if needed
|
||||
if m.autoProvisionAccounts {
|
||||
if err = m.userProvider.UpdateUserIfNeeded(req.Context(), user, claims); err != nil {
|
||||
@@ -248,3 +259,14 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
span.End()
|
||||
m.next.ServeHTTP(w, req)
|
||||
}
|
||||
|
||||
func (m accountResolver) verifyTenantClaim(userTenantID string, claims map[string]interface{}) error {
|
||||
claimTenantID, err := readStringClaim(m.tenantOIDCClaim, claims)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not read tenant claim: %w", err)
|
||||
}
|
||||
if claimTenantID != userTenantID {
|
||||
return fmt.Errorf("tenant id from claim %q does not match user tenant id %q", claimTenantID, userTenantID)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -20,14 +20,21 @@ import (
|
||||
"github.com/stretchr/testify/mock"
|
||||
)
|
||||
|
||||
const (
|
||||
testIdP = "https://idx.example.com"
|
||||
testTenantA = "tenant-a"
|
||||
testTenantB = "tenant-b"
|
||||
testJWTSecret = "change-me"
|
||||
)
|
||||
|
||||
func TestTokenIsAddedWithMailClaim(t *testing.T) {
|
||||
sut := newMockAccountResolver(&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
|
||||
Id: &userv1beta1.UserId{Idp: testIdP, OpaqueId: "123"},
|
||||
Mail: "foo@example.com",
|
||||
}, nil, oidc.Email, "mail", false)
|
||||
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.Email: "foo@example.com",
|
||||
})
|
||||
|
||||
@@ -40,12 +47,12 @@ func TestTokenIsAddedWithMailClaim(t *testing.T) {
|
||||
|
||||
func TestTokenIsAddedWithUsernameClaim(t *testing.T) {
|
||||
sut := newMockAccountResolver(&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
|
||||
Id: &userv1beta1.UserId{Idp: testIdP, OpaqueId: "123"},
|
||||
Mail: "foo@example.com",
|
||||
}, nil, oidc.PreferredUsername, "username", false)
|
||||
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
})
|
||||
|
||||
@@ -59,13 +66,13 @@ func TestTokenIsAddedWithUsernameClaim(t *testing.T) {
|
||||
|
||||
func TestTokenIsAddedWithDotUsernamePathClaim(t *testing.T) {
|
||||
sut := newMockAccountResolver(&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
|
||||
Id: &userv1beta1.UserId{Idp: testIdP, OpaqueId: "123"},
|
||||
Mail: "foo@example.com",
|
||||
}, nil, "li.un", "username", false)
|
||||
|
||||
// This is how lico adds the username to the access token
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
"li": map[string]interface{}{
|
||||
"un": "foo",
|
||||
},
|
||||
@@ -79,44 +86,44 @@ func TestTokenIsAddedWithDotUsernamePathClaim(t *testing.T) {
|
||||
assert.Contains(t, token, "eyJ")
|
||||
}
|
||||
|
||||
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", false)
|
||||
func TestTokenIsAddedWithDottedUsernameClaim(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
oidcClaim string
|
||||
// comment describing what the claim exercises
|
||||
desc string
|
||||
}{
|
||||
{
|
||||
name: "escaped dot treated as literal key",
|
||||
oidcClaim: "li\\.un",
|
||||
desc: "li\\.un escapes the dot so the claim is looked up as the literal key \"li.un\"",
|
||||
},
|
||||
{
|
||||
name: "dotted path falls back to literal key",
|
||||
oidcClaim: "li.un",
|
||||
desc: "li.un is first tried as a nested path; when \"un\" is absent under \"li\", it falls back to the literal key \"li.un\"",
|
||||
},
|
||||
}
|
||||
|
||||
// This tests the . escaping of the readUserIDClaim
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
"li.un": "foo",
|
||||
})
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
sut := newMockAccountResolver(&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{Idp: testIdP, OpaqueId: "123"},
|
||||
Mail: "foo@example.com",
|
||||
}, nil, tc.oidcClaim, "username", false)
|
||||
|
||||
sut.ServeHTTP(rw, req)
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: testIdP,
|
||||
"li.un": "foo",
|
||||
})
|
||||
|
||||
token := req.Header.Get(revactx.TokenHeader)
|
||||
assert.NotEmpty(t, token)
|
||||
sut.ServeHTTP(rw, req)
|
||||
|
||||
assert.Contains(t, token, "eyJ")
|
||||
}
|
||||
|
||||
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", false)
|
||||
|
||||
// This tests the . escaping fallback of the readUserIDClaim
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
"li.un": "foo",
|
||||
})
|
||||
|
||||
sut.ServeHTTP(rw, req)
|
||||
|
||||
token := req.Header.Get(revactx.TokenHeader)
|
||||
assert.NotEmpty(t, token)
|
||||
|
||||
assert.Contains(t, token, "eyJ")
|
||||
token := req.Header.Get(revactx.TokenHeader)
|
||||
assert.NotEmpty(t, token)
|
||||
assert.Contains(t, token, "eyJ")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNSkipOnNoClaims(t *testing.T) {
|
||||
@@ -133,7 +140,7 @@ func TestNSkipOnNoClaims(t *testing.T) {
|
||||
func TestUnauthorizedOnUserNotFound(t *testing.T) {
|
||||
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.PreferredUsername, "username", false)
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
})
|
||||
|
||||
@@ -147,7 +154,7 @@ func TestUnauthorizedOnUserNotFound(t *testing.T) {
|
||||
func TestUnauthorizedOnUserDisabled(t *testing.T) {
|
||||
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.PreferredUsername, "username", false)
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
})
|
||||
|
||||
@@ -161,7 +168,7 @@ func TestUnauthorizedOnUserDisabled(t *testing.T) {
|
||||
func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
|
||||
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.Email, "mail", false)
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
})
|
||||
|
||||
sut.ServeHTTP(rw, req)
|
||||
@@ -174,12 +181,12 @@ func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
|
||||
func TestUnauthorizedOnMissingTenantId(t *testing.T) {
|
||||
sut := newMockAccountResolver(
|
||||
&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
|
||||
Id: &userv1beta1.UserId{Idp: testIdP, OpaqueId: "123"},
|
||||
Username: "foo",
|
||||
},
|
||||
nil, oidc.PreferredUsername, "username", true)
|
||||
req, rw := mockRequest(map[string]any{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
})
|
||||
|
||||
@@ -194,7 +201,7 @@ func TestTokenIsAddedWhenUserHasTenantId(t *testing.T) {
|
||||
sut := newMockAccountResolver(
|
||||
&userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{
|
||||
Idp: "https://idx.example.com",
|
||||
Idp: testIdP,
|
||||
OpaqueId: "123",
|
||||
TenantId: "tenant1",
|
||||
},
|
||||
@@ -202,7 +209,7 @@ func TestTokenIsAddedWhenUserHasTenantId(t *testing.T) {
|
||||
},
|
||||
nil, oidc.PreferredUsername, "username", true)
|
||||
req, rw := mockRequest(map[string]any{
|
||||
oidc.Iss: "https://idx.example.com",
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
})
|
||||
|
||||
@@ -213,9 +220,78 @@ func TestTokenIsAddedWhenUserHasTenantId(t *testing.T) {
|
||||
assert.Contains(t, token, "eyJ")
|
||||
}
|
||||
|
||||
func TestTenantClaimValidation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
requestTenant string
|
||||
wantToken bool
|
||||
wantStatusCode int
|
||||
}{
|
||||
{
|
||||
name: "token added when tenant claim matches",
|
||||
requestTenant: testTenantA,
|
||||
wantToken: true,
|
||||
wantStatusCode: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "unauthorized when tenant claim does not match",
|
||||
requestTenant: testTenantB,
|
||||
wantToken: false,
|
||||
wantStatusCode: http.StatusUnauthorized,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
user := &userv1beta1.User{
|
||||
Id: &userv1beta1.UserId{
|
||||
Idp: testIdP,
|
||||
OpaqueId: "123",
|
||||
TenantId: testTenantA,
|
||||
},
|
||||
Username: "foo",
|
||||
}
|
||||
|
||||
tokenManager, _ := jwt.New(map[string]interface{}{"secret": testJWTSecret, "expires": int64(60)})
|
||||
s, _ := scope.AddOwnerScope(nil)
|
||||
token, _ := tokenManager.MintToken(context.Background(), user, s)
|
||||
|
||||
ub := mocks.UserBackend{}
|
||||
ub.On("GetUserByClaims", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(user, token, nil)
|
||||
ra := userRoleMocks.UserRoleAssigner{}
|
||||
ra.On("UpdateUserRoleAssignment", mock.Anything, mock.Anything, mock.Anything).Return(user, nil)
|
||||
|
||||
sut := AccountResolver(
|
||||
Logger(log.NewLogger()),
|
||||
UserProvider(&ub),
|
||||
UserRoleAssigner(&ra),
|
||||
UserOIDCClaim(oidc.PreferredUsername),
|
||||
UserCS3Claim("username"),
|
||||
TenantOIDCClaim("tenant_id"),
|
||||
MultiTenantEnabled(true),
|
||||
)(mockHandler{})
|
||||
|
||||
req, rw := mockRequest(map[string]interface{}{
|
||||
oidc.Iss: testIdP,
|
||||
oidc.PreferredUsername: "foo",
|
||||
"tenant_id": tc.requestTenant,
|
||||
})
|
||||
|
||||
sut.ServeHTTP(rw, req)
|
||||
|
||||
if tc.wantToken {
|
||||
assert.NotEmpty(t, req.Header.Get(revactx.TokenHeader))
|
||||
} else {
|
||||
assert.Empty(t, req.Header.Get(revactx.TokenHeader))
|
||||
}
|
||||
assert.Equal(t, tc.wantStatusCode, rw.Code)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error, oidcclaim, cs3claim string, multiTenant bool) http.Handler {
|
||||
tokenManager, _ := jwt.New(map[string]interface{}{
|
||||
"secret": "change-me",
|
||||
"secret": testJWTSecret,
|
||||
"expires": int64(60),
|
||||
})
|
||||
|
||||
|
||||
@@ -49,6 +49,9 @@ type Options struct {
|
||||
UserOIDCClaim string
|
||||
// UserCS3Claim to use when looking up a user in the CS3 API
|
||||
UserCS3Claim string
|
||||
// TenantOIDCClaim is a JMESPath expression to extract the tenant ID from the OIDC claims.
|
||||
// When set, the extracted value is verified against the tenant ID on the resolved user.
|
||||
TenantOIDCClaim string
|
||||
// AutoprovisionAccounts when an accountResolver does not exist.
|
||||
AutoprovisionAccounts bool
|
||||
// EnableBasicAuth to allow basic auth
|
||||
@@ -171,6 +174,13 @@ func UserCS3Claim(val string) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// TenantOIDCClaim provides a function to set the TenantOIDCClaim config
|
||||
func TenantOIDCClaim(val string) Option {
|
||||
return func(o *Options) {
|
||||
o.TenantOIDCClaim = val
|
||||
}
|
||||
}
|
||||
|
||||
// AutoprovisionAccounts provides a function to set the AutoprovisionAccounts config
|
||||
func AutoprovisionAccounts(val bool) Option {
|
||||
return func(o *Options) {
|
||||
|
||||
Reference in New Issue
Block a user