feat(multi-tenancy): verify tenant via OIDC claim

devtools/deployments/multi-tenancy/config/opencloud/csp.yaml

@@ -4,10 +4,11 @@ directives:
   connect-src:
     - '''self'''
     - 'blob:'
-    - 'https://${COMPANION_DOMAIN|companion.opencloud.test}/'
-    - 'wss://${COMPANION_DOMAIN|companion.opencloud.test}/'
+    - 'https://${COMPANION_DOMAIN|companion.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
+    - 'wss://${COMPANION_DOMAIN|companion.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
     - 'https://raw.githubusercontent.com/opencloud-eu/awesome-apps/'
-    - 'https://${IDP_DOMAIN|keycloak.opencloud.test}/'
+    - 'https://${IDP_DOMAIN|keycloak.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
+    - 'https://update.opencloud.eu/'
   default-src:
     - '''none'''
   font-src:
@@ -19,16 +20,17 @@ directives:
     - 'blob:'
     - 'https://embed.diagrams.net/'
     # In contrary to bash and docker the default is given after the | character
-    - 'https://${COLLABORA_DOMAIN|collabora.opencloud.test}/'
+    - 'https://${COLLABORA_DOMAIN|collabora.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
     # This is needed for the external-sites web extension when embedding sites
     - 'https://docs.opencloud.eu'
   img-src:
     - '''self'''
     - 'data:'
     - 'blob:'
     - 'https://raw.githubusercontent.com/opencloud-eu/awesome-apps/'
+    - 'https://tile.openstreetmap.org/'
     # In contrary to bash and docker the default is given after the | character
-    - 'https://${COLLABORA_DOMAIN|collabora.opencloud.test}/'
+    - 'https://${COLLABORA_DOMAIN|collabora.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
   manifest-src:
     - '''self'''
   media-src:
@@ -39,6 +41,7 @@ directives:
   script-src:
     - '''self'''
     - '''unsafe-inline'''
+    - 'https://${IDP_DOMAIN|keycloak.opencloud.test}${TRAEFIK_PORT_HTTPS}/'
   style-src:
     - '''self'''
     - '''unsafe-inline'''

services/proxy/pkg/command/server.go

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

services/proxy/pkg/config/config.go

@@ -34,6 +34,7 @@ type Config struct {
 	AccountBackend                string              `yaml:"account_backend" env:"PROXY_ACCOUNT_BACKEND_TYPE" desc:"Account backend the PROXY service should use. Currently only 'cs3' is possible here." introductionVersion:"1.0.0"`
 	UserOIDCClaim                 string              `yaml:"user_oidc_claim" env:"PROXY_USER_OIDC_CLAIM" desc:"The name of an OpenID Connect claim that is used for resolving users with the account backend. The value of the claim must hold a per user unique, stable and non re-assignable identifier. The availability of claims depends on your Identity Provider. There are common claims available for most Identity providers like 'email' or 'preferred_username' but you can also add your own claim." introductionVersion:"1.0.0"`
 	UserCS3Claim                  string              `yaml:"user_cs3_claim" env:"PROXY_USER_CS3_CLAIM" desc:"The name of a CS3 user attribute (claim) that should be mapped to the 'user_oidc_claim'. Supported values are 'username', 'mail' and 'userid'." introductionVersion:"1.0.0"`
+	TenantOIDCClaim               string              `yaml:"tenant_oidc_claim" env:"PROXY_TENANT_OIDC_CLAIM" desc:"JMESPath expression to extract the tenant ID from the OIDC token claims. When set, the extracted value is verified against the tenant ID returned by the user backend, rejecting requests where they do not match. Only relevant when multi-tenancy is enabled." introductionVersion:"%%NEXT%%"`
 	MachineAuthAPIKey             string              `yaml:"machine_auth_api_key" env:"OC_MACHINE_AUTH_API_KEY;PROXY_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary to access resources from other services." introductionVersion:"1.0.0" mask:"password"`
 	AutoprovisionAccounts         bool                `yaml:"auto_provision_accounts" env:"PROXY_AUTOPROVISION_ACCOUNTS" desc:"Set this to 'true' to automatically provision users that do not yet exist in the users service on-demand upon first sign-in. To use this a write-enabled libregraph user backend needs to be setup an running." introductionVersion:"1.0.0"`
 	AutoProvisionClaims           AutoProvisionClaims `yaml:"auto_provision_claims"`

services/proxy/pkg/middleware/account_resolver.go

@@ -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,14 +62,15 @@ 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.
 	lastGroupSyncCache *ttlcache.Cache[string, 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
+}

services/proxy/pkg/middleware/account_resolver_test.go

@@ -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)
-
-	// This tests the . escaping 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")
-}
-
-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)
+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 fallback 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")
+			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,15 +201,15 @@ func TestTokenIsAddedWhenUserHasTenantId(t *testing.T) {
 	sut := newMockAccountResolver(
 		&userv1beta1.User{
 			Id: &userv1beta1.UserId{
-				Idp:      "https://idx.example.com",
+				Idp:      testIdP,
 				OpaqueId: "123",
 				TenantId: "tenant1",
 			},
 			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",
 	})
 
@@ -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),
 	})