Merge 28af4e3f25 into 0aba66a820

src/lib/features/instance-stats/getReadOnlyUsers.e2e.test.ts

@@ -6,18 +6,133 @@ import dbInit, {
     type ITestDb,
 } from '../../../test/e2e/helpers/database-init.js';
 import getLogger from '../../../test/fixtures/no-logger.js';
+import { RoleName } from '../../types/model.js';
+import {
+    ALL_PROJECTS,
+    PROJECT_ROLE_TYPE,
+    ROOT_ROLE_TYPE,
+} from '../../util/constants.js';
+import { FEATURE_CREATED, FEATURE_FAVORITED } from '../../events/index.js';
 
 let db: ITestDb;
 let getReadOnlyUsers: GetReadOnlyUsers;
-let _viewerRoleId: number;
+let viewerRootRoleId: number;
+let adminRootRoleId: number;
+let ownerProjectRoleId: number;
+let userCounter = 0;
+let groupCounter = 0;
+
+const getRoleId = async (name: RoleName, type: string) => {
+    const role = await db
+        .rawDatabase('roles')
+        .where({ name, type })
+        .first('id');
+
+    if (!role) {
+        throw new Error(`Missing role ${name} (${type})`);
+    }
+
+    return role.id as number;
+};
+
+const insertUser = async (
+    overrides: Record<string, unknown> = {},
+): Promise<{ id: number }> => {
+    const [user] = await db
+        .rawDatabase('users')
+        .insert({
+            email: `viewer${++userCounter}@example.com`,
+            name: `Viewer ${userCounter}`,
+            is_system: false,
+            is_service: false,
+            ...overrides,
+        })
+        .returning('id');
+
+    return user as { id: number };
+};
+
+const assignRoleToUser = async (
+    userId: number,
+    roleId: number,
+    project: string = ALL_PROJECTS,
+) => {
+    await db.rawDatabase('role_user').insert({
+        role_id: roleId,
+        user_id: userId,
+        project,
+        created_at: new Date(),
+    });
+};
+
+const createViewer = async (overrides: Record<string, unknown> = {}) => {
+    const user = await insertUser(overrides);
+    await assignRoleToUser(user.id, viewerRootRoleId);
+    return user;
+};
+
+const createGroup = async (
+    overrides: Record<string, unknown> = {},
+): Promise<{ id: number }> => {
+    const [group] = await db
+        .rawDatabase('groups')
+        .insert({
+            name: `group-${++groupCounter}`,
+            created_at: new Date(),
+            ...overrides,
+        })
+        .returning('id');
+
+    return group as { id: number };
+};
+
+const addUserToGroup = async (userId: number, groupId: number) => {
+    await db.rawDatabase('group_user').insert({
+        group_id: groupId,
+        user_id: userId,
+        created_at: new Date(),
+    });
+};
+
+const assignGroupRole = async (
+    groupId: number,
+    roleId: number,
+    project: string = ALL_PROJECTS,
+) => {
+    await db.rawDatabase('group_role').insert({
+        group_id: groupId,
+        role_id: roleId,
+        project,
+        created_at: new Date(),
+    });
+};
+
+const addEventForUser = async (userId: number, type: string) => {
+    await db.rawDatabase('events').insert({
+        type,
+        created_by_user_id: userId,
+        created_by: 'readonly-test',
+        project: 'default',
+        environment: 'development',
+        data: '{}',
+        created_at: new Date(),
+    });
+};
 
 beforeAll(async () => {
     db = await dbInit('read_only_users_serial', getLogger);
     getReadOnlyUsers = createGetReadOnlyUsers(db.rawDatabase);
+
+    viewerRootRoleId = await getRoleId(RoleName.VIEWER, ROOT_ROLE_TYPE);
+    adminRootRoleId = await getRoleId(RoleName.ADMIN, ROOT_ROLE_TYPE);
+    ownerProjectRoleId = await getRoleId(RoleName.OWNER, PROJECT_ROLE_TYPE);
 });
 
 afterEach(async () => {
     await db.rawDatabase('events').delete();
+    await db.rawDatabase('group_role').delete();
+    await db.rawDatabase('group_user').delete();
+    await db.rawDatabase('groups').delete();
     await db.rawDatabase('role_user').delete();
     await db.rawDatabase('users').delete();
 });
@@ -26,22 +141,81 @@ afterAll(async () => {
     await db.destroy();
 });
 
-test('should count user with Viewer root role and no events', async () => {
-    const [user] = await db
-        .rawDatabase('users')
-        .insert({
-            email: 'viewer@example.com',
-            name: 'Viewer User',
-            is_system: false,
-            is_service: false,
-        })
-        .returning('id');
-
-    await db.rawDatabase('role_user').insert({
-        role_id: 3,
-        user_id: user.id,
-        project: '*',
-    });
+test('counts viewer without write events or permissions', async () => {
+    const viewer = await createViewer();
+    await addEventForUser(viewer.id, FEATURE_FAVORITED);
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('counts multiple viewers that satisfy read-only conditions', async () => {
+    await createViewer();
+    await createViewer();
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(2);
+});
+
+test('ignores deleted, system, and service viewers', async () => {
+    await createViewer();
+    await createViewer({ is_system: true });
+    await createViewer({ is_service: true });
+    await createViewer({ deleted_at: new Date() });
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('ignores viewers with additional root permissions', async () => {
+    await createViewer();
+    const userWithAdmin = await createViewer();
+    await assignRoleToUser(userWithAdmin.id, adminRootRoleId);
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('ignores viewers with project role permissions', async () => {
+    await createViewer();
+    const userWithProjectRole = await createViewer();
+    await assignRoleToUser(
+        userWithProjectRole.id,
+        ownerProjectRoleId,
+        'default',
+    );
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('ignores viewers with permissions inherited from group project roles', async () => {
+    await createViewer();
+    const userWithGroupProjectPermissions = await createViewer();
+    const group = await createGroup();
+    await addUserToGroup(userWithGroupProjectPermissions.id, group.id);
+    await assignGroupRole(group.id, ownerProjectRoleId, 'default');
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('ignores viewers with permissions inherited from group root roles', async () => {
+    await createViewer();
+    const userWithGroupRootPermissions = await createViewer();
+    const group = await createGroup({ root_role_id: adminRootRoleId });
+    await addUserToGroup(userWithGroupRootPermissions.id, group.id);
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(1);
+});
+
+test('counts viewers with no permissions inherited from group root roles', async () => {
+    await createViewer();
+    const userWithGroupRootPermissions = await createViewer();
+    const group = await createGroup({ root_role_id: viewerRootRoleId });
+    await addUserToGroup(userWithGroupRootPermissions.id, group.id);
+
+    await expect(getReadOnlyUsers()).resolves.toEqual(2);
+});
+
+test('ignores viewers with write events', async () => {
+    await createViewer();
+    const userWithWriteEvent = await createViewer();
+    await addEventForUser(userWithWriteEvent.id, FEATURE_CREATED);
 
     await expect(getReadOnlyUsers()).resolves.toEqual(1);
 });

src/lib/features/instance-stats/getReadOnlyUsers.ts

@@ -1,8 +1,20 @@
 import type { Db } from '../../types/index.js';
-import { FEATURE_FAVORITED } from '../../events/index.js';
+import {
+    FEATURE_FAVORITED,
+    FEATURE_UNFAVORITED,
+    PROJECT_FAVORITED,
+    PROJECT_UNFAVORITED,
+} from '../../events/index.js';
 import { RoleName } from '../../types/model.js';
 import { ROOT_ROLE_TYPE } from '../../util/constants.js';
 
+const READONLY_EVENTS = [
+    FEATURE_FAVORITED,
+    FEATURE_UNFAVORITED,
+    PROJECT_FAVORITED,
+    PROJECT_UNFAVORITED,
+];
+
 export type GetReadOnlyUsers = () => Promise<number>;
 
 export const createGetReadOnlyUsers =
@@ -12,16 +24,57 @@ export const createGetReadOnlyUsers =
             .countDistinct('users.id as readOnlyCount')
             .join('role_user', 'role_user.user_id', 'users.id')
             .join('roles', 'roles.id', 'role_user.role_id')
+            // Ensure valid user
             .whereNull('users.deleted_at')
             .where('users.is_system', false)
             .where('users.is_service', false)
+            // Ensure Viewer root role
             .where('roles.name', RoleName.VIEWER)
             .where('roles.type', ROOT_ROLE_TYPE)
+            // Ensure no permissions across all roles and groups
             .whereNotExists(function () {
-                this.select('*')
+                // Ensure no permissions across root and project roles
+                this.select(1)
+                    .from('role_permission as rp')
+                    .join('role_user as ur', 'ur.role_id', 'rp.role_id')
+                    .whereRaw('ur.user_id = users.id')
+                    // Ensure no permissions from group project roles
+                    .union(function () {
+                        this.select(db.raw('1'))
+                            .from('group_user as gu')
+                            .join('groups as g', 'g.id', 'gu.group_id')
+                            .join(
+                                'group_role as gr',
+                                'gr.group_id',
+                                'gu.group_id',
+                            )
+                            .join(
+                                'role_permission as rp',
+                                'rp.role_id',
+                                'gr.role_id',
+                            )
+                            .whereRaw('gu.user_id = users.id');
+                    })
+                    // Ensure no permissions from group root roles
+                    .union(function () {
+                        this.select(db.raw('1'))
+                            .from('group_user as gu')
+                            .join('groups as g', 'g.id', 'gu.group_id')
+                            .join(
+                                'role_permission as rp',
+                                'rp.role_id',
+                                'g.root_role_id',
+                            )
+                            .whereNotNull('g.root_role_id')
+                            .whereRaw('gu.user_id = users.id');
+                    });
+            })
+            // Ensure no write events
+            .whereNotExists(function () {
+                this.select(1)
                     .from('events')
                     .whereRaw('events.created_by_user_id = users.id')
-                    .whereNot('events.type', FEATURE_FAVORITED);
+                    .whereNotIn('events.type', READONLY_EVENTS);
             })
             .first();
 

src/lib/metrics.test.ts

@@ -365,9 +365,8 @@ test('should collect licensed_users metrics', async () => {
     expect(recordedMetric).toMatch(/licensed_users 0/);
 });
 
-test('should collect users_read_only_total metrics', async () => {
-    const recordedMetric = await prometheusRegister.getSingleMetricAsString(
-        'users_read_only_total',
-    );
-    expect(recordedMetric).toMatch(/users_read_only_total 0/);
+test('should collect read_only_users metrics', async () => {
+    const recordedMetric =
+        await prometheusRegister.getSingleMetricAsString('read_only_users');
+    expect(recordedMetric).toMatch(/read_only_users 0/);
 });

src/lib/metrics.ts

@@ -324,12 +324,6 @@ export function registerPrometheusMetrics(
         fetchValue: () => stores.userStore.count(),
         ttlMs: minutesToMilliseconds(15),
     });
-    const _usersReadOnly = createGauge({
-        name: 'users_read_only_total',
-        help: 'Number of read-only users (users with no events or only FEATURE_FAVORITED events)',
-        fetchValue: () => instanceStatsService.getReadOnlyUsers(),
-        ttlMs: hoursToMilliseconds(24),
-    });
     const trafficTotal = createGauge({
         name: 'traffic_total',
         help: 'Traffic used current month',
@@ -781,6 +775,18 @@ export function registerPrometheusMetrics(
         help: 'Number of unique unknown flag names reported in the last 24 hours, if any.',
     });
 
+    dbMetrics.registerGaugeDbMetric({
+        name: 'read_only_users',
+        help: 'Number of read-only users (viewers with no permissions or write events).',
+        query: () => {
+            if (flagResolver.isEnabled('readOnlyUsers')) {
+                return instanceStatsService.getReadOnlyUsers();
+            }
+            return Promise.resolve(0);
+        },
+        map: (result) => ({ value: result }),
+    });
+
     // register event listeners
     eventBus.on(
         events.EXCEEDS_LIMIT,