Add test case to cover cross-bucket RBAC role grants for filtering

base/main_test_bucket_pool_util.go

@@ -51,8 +51,8 @@ func getTestBucketSpec(testBucketName tbpBucketName) BucketSpec {
 }
 
 // RequireNumTestBuckets skips the given test if there are not enough test buckets available to use.
-func RequireNumTestBuckets(t *testing.T, numRequired int) {
-	usable := GTestBucketPool.numUsableBuckets()
+func RequireNumTestBuckets(t testing.TB, numRequired int) {
+	usable := GTestBucketPool.NumUsableBuckets()
 	if usable < numRequired {
 		t.Skipf("Only had %d usable test buckets available (test requires %d)", usable, numRequired)
 	}
@@ -67,8 +67,8 @@ func RequireNumTestDataStores(t testing.TB, numRequired int) {
 	}
 }
 
-// numUsableBuckets returns the total number of buckets in the pool that can be used by a test.
-func (tbp *TestBucketPool) numUsableBuckets() int {
+// NumUsableBuckets returns the total number of buckets in the pool that can be used by a test.
+func (tbp *TestBucketPool) NumUsableBuckets() int {
 	if !tbp.integrationMode {
 		// we can create virtually endless walrus buckets,
 		// so report back 10 to match a fully available CBS bucket pool.

rest/audit_test.go

@@ -39,14 +39,15 @@ func TestAuditLoggingFields(t *testing.T) {
 	base.InitializeMemoryLoggers()
 
 	const (
-		requestInfoHeaderName      = "extra-audit-logging-header"
-		requestUser                = "alice"
-		filteredPublicUsername     = "bob"
-		filteredPublicRoleUsername = "charlie"
-		filteredPublicRoleName     = "observer"
-		filteredAdminUsername      = "TestAuditLoggingFields-charlie"
-		filteredAdminRoleUsername  = "TestAuditLoggingFields-bob"
-		unauthorizedAdminUsername  = "TestAuditLoggingFields-alice"
+		requestInfoHeaderName       = "extra-audit-logging-header"
+		requestUser                 = "alice"
+		filteredPublicUsername      = "bob"
+		filteredPublicRoleUsername  = "charlie"
+		filteredPublicRoleName      = "observer"
+		filteredAdminUsername       = "TestAuditLoggingFields-charlie"
+		unfilteredAdminRoleUsername = "TestAuditLoggingFields-diana"
+		filteredAdminRoleUsername   = "TestAuditLoggingFields-bob"
+		unauthorizedAdminUsername   = "TestAuditLoggingFields-alice"
 	)
 	var (
 		filteredAdminRoleName = BucketFullAccessRole.RoleName
@@ -102,13 +103,29 @@ func TestAuditLoggingFields(t *testing.T) {
 		eps, httpClient, err := rt.ServerContext().ObtainManagementEndpointsAndHTTPClient()
 		require.NoError(t, err)
 
-		MakeUser(t, httpClient, eps[0], filteredAdminUsername, RestTesterDefaultUserPassword, []string{fmt.Sprintf("%s[%s]", MobileSyncGatewayRole.RoleName, rt.Bucket().GetName())})
+		MakeUser(t, httpClient, eps[0], filteredAdminUsername, RestTesterDefaultUserPassword, []string{
+			fmt.Sprintf("%s[%s]", MobileSyncGatewayRole.RoleName, rt.Bucket().GetName()),
+		})
 		defer DeleteUser(t, httpClient, eps[0], filteredAdminUsername)
-		MakeUser(t, httpClient, eps[0], filteredAdminRoleUsername, RestTesterDefaultUserPassword, []string{fmt.Sprintf("%s[%s]", filteredAdminRoleName, rt.Bucket().GetName())})
+		MakeUser(t, httpClient, eps[0], filteredAdminRoleUsername, RestTesterDefaultUserPassword, []string{
+			fmt.Sprintf("%s[%s]", filteredAdminRoleName, rt.Bucket().GetName()),
+		})
 		defer DeleteUser(t, httpClient, eps[0], filteredAdminRoleUsername)
 		MakeUser(t, httpClient, eps[0], unauthorizedAdminUsername, RestTesterDefaultUserPassword, []string{})
 		defer DeleteUser(t, httpClient, eps[0], unauthorizedAdminUsername)
 
+		// if we have another bucket available, use it to test cross-bucket role filtering (to ensure it doesn't)
+		if base.GTestBucketPool.NumUsableBuckets() >= 2 {
+			differentBucket := base.GetTestBucket(t)
+			defer differentBucket.Close(base.TestCtx(t))
+			differentBucketName := differentBucket.GetName()
+
+			MakeUser(t, httpClient, eps[0], unfilteredAdminRoleUsername, RestTesterDefaultUserPassword, []string{
+				fmt.Sprintf("%s[%s]", filteredAdminRoleName, differentBucketName),
+				fmt.Sprintf("%s[%s]", MobileSyncGatewayRole.RoleName, rt.Bucket().GetName()),
+			})
+			defer DeleteUser(t, httpClient, eps[0], unfilteredAdminRoleUsername)
+		}
 	}
 
 	// auditFieldValueIgnored is a special value for an audit field to skip value-specific checks whilst still ensuring the field property is set
@@ -339,6 +356,30 @@ func TestAuditLoggingFields(t *testing.T) {
 				RequireStatus(t, rt.SendAdminRequestWithAuth(http.MethodGet, "/db/", "", filteredAdminRoleUsername, RestTesterDefaultUserPassword), http.StatusOK)
 			},
 		},
+		{
+			name: "authed admin request role filtered on different bucket",
+			auditableAction: func(t testing.TB) {
+				if !rt.AdminInterfaceAuthentication {
+					t.Skip("Skipping subtest that requires admin auth")
+				}
+				base.RequireNumTestBuckets(t, 2)
+				RequireStatus(t, rt.SendAdminRequestWithAuth(http.MethodGet, "/db/", "", unfilteredAdminRoleUsername, RestTesterDefaultUserPassword), http.StatusOK)
+			},
+			expectedAuditEventFields: map[base.AuditID]base.AuditFields{
+				base.AuditIDAdminUserAuthenticated: {
+					base.AuditFieldCorrelationID: auditFieldValueIgnored,
+					//	base.AuditFieldRealUserID:    map[string]any{"domain": "cbs", "user": unfilteredAdminRoleUsername},
+				},
+				base.AuditIDReadDatabase: {
+					base.AuditFieldCorrelationID: auditFieldValueIgnored,
+					base.AuditFieldRealUserID:    map[string]any{"domain": "cbs", "user": unfilteredAdminRoleUsername},
+				},
+				base.AuditIDAdminHTTPAPIRequest: {
+					base.AuditFieldHTTPMethod: http.MethodGet,
+					base.AuditFieldHTTPPath:   "/db/",
+				},
+			},
+		},
 	}
 	for _, testCase := range testCases {
 		rt.Run(testCase.name, func(t *testing.T) {

rest/handler.go

@@ -573,20 +573,8 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
 
 		// even though `checkAdminAuth` _can_ issue whoami to find user's roles, it doesn't always...
 		// to reduce code complexity, we'll potentially be making this whoami request twice if we need it for audit filtering
-		var auditRoleNames []string
-		if needRolesForAudit(dbContext, base.UserDomainCBServer) {
-			whoAmIResponse, whoAmIStatus, whoAmIErr := cbRBACWhoAmI(h.ctx(), httpClient, managementEndpoints, username, password)
-			if whoAmIErr != nil || whoAmIStatus != http.StatusOK {
-				base.WarnfCtx(h.ctx(), "An error occurred whilst fetching the user's roles for audit filtering - will not filter: %v", whoAmIErr)
-			} else {
-				for _, role := range whoAmIResponse.Roles {
-					// only filter roles applied to this bucket or cluster-scope
-					if bucketName == authScope || bucketName == "" {
-						auditRoleNames = append(auditRoleNames, role.RoleName)
-					}
-				}
-			}
-		}
+		auditRoleNames := getCBUserRolesForAudit(dbContext, authScope, h.ctx(), httpClient, managementEndpoints, username, password)
+
 		h.authorizedAdminUser = username
 		h.permissionsResults = permissions
 		h.rqCtx = base.UserLogCtx(h.ctx(), username, base.UserDomainCBServer, auditRoleNames)
@@ -678,6 +666,27 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
 	return nil
 }
 
+func getCBUserRolesForAudit(db *db.DatabaseContext, authScope string, ctx context.Context, httpClient *http.Client, managementEndpoints []string, username, password string) []string {
+	if !needRolesForAudit(db, base.UserDomainCBServer) {
+		return nil
+	}
+
+	whoAmIResponse, whoAmIStatus, whoAmIErr := cbRBACWhoAmI(ctx, httpClient, managementEndpoints, username, password)
+	if whoAmIErr != nil || whoAmIStatus != http.StatusOK {
+		base.WarnfCtx(ctx, "An error occurred whilst fetching the user's roles for audit filtering - will not filter: %v", whoAmIErr)
+		return nil
+	}
+
+	var auditRoleNames []string
+	for _, role := range whoAmIResponse.Roles {
+		// only filter roles applied to this bucket or cluster-scope
+		if role.BucketName == "" || role.BucketName == authScope {
+			auditRoleNames = append(auditRoleNames, role.RoleName)
+		}
+	}
+	return auditRoleNames
+}
+
 // removeCorruptConfigIfExists will remove the config from the bucket and remove it from the map if it exists on the invalid database config map
 func (h *handler) removeCorruptConfigIfExists(ctx context.Context, bucket, configGroupID, dbName string) error {
 	_, ok := h.server.invalidDatabaseConfigTracking.exists(dbName)