From 361d27412bd835052d3376823a969b9a999b4614 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 12:47:38 +0200 Subject: [PATCH 1/6] fix(users): return false on 409 in /activate and /deactivate --- src/users/index.ts | 7 ++++--- src/users/integration.test.ts | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/users/index.ts b/src/users/index.ts index 7b2c608a..07983bed 100644 --- a/src/users/index.ts +++ b/src/users/index.ts @@ -147,11 +147,12 @@ const users = (connection: ConnectionREST): Users => { const db = (connection: ConnectionREST): DBUsers => { const ns = namespacedUsers(connection); - /** expectCode returns true if the error contained an expected status code. */ + /** expectCode returns false if the contained WeaviateUnexpectedStatusCodeError + * has an known error code and rethrows the error otherwise. */ const expectCode = (code: number): ((_: any) => boolean) => { return (error) => { - if (error instanceof WeaviateUnexpectedStatusCodeError) { - return error.code === code; + if (error instanceof WeaviateUnexpectedStatusCodeError && error.code === code) { + return false; } throw error; }; diff --git a/src/users/integration.test.ts b/src/users/integration.test.ts index 0c442bfe..4d5862db 100644 --- a/src/users/integration.test.ts +++ b/src/users/integration.test.ts @@ -79,13 +79,13 @@ requireAtLeast( await expectDave().toHaveProperty('active', true); // Second activation is a no-op - await expect(client.users.db.activate('dynamic-dave')).resolves.toEqual(true); + await expect(client.users.db.activate('dynamic-dave')).resolves.toEqual(false); await client.users.db.deactivate('dynamic-dave'); await expectDave().toHaveProperty('active', false); // Second deactivation is a no-op - await expect(client.users.db.deactivate('dynamic-dave', { revokeKey: true })).resolves.toEqual(true); + await expect(client.users.db.deactivate('dynamic-dave', { revokeKey: true })).resolves.toEqual(false); await client.users.db.delete('dynamic-dave'); await expectDave(false).toHaveProperty('code', 404); From e257301e196f0533e25226924768fd28cd4f4844 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 13:06:44 +0200 Subject: [PATCH 2/6] chore: add deprecationnotice for assign/revokeRoles without namespace --- src/users/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/users/index.ts b/src/users/index.ts index 07983bed..9b72ea90 100644 --- a/src/users/index.ts +++ b/src/users/index.ts @@ -34,6 +34,11 @@ interface UsersBase { } export interface Users extends UsersBase { + /** @deprecated: Use `users.db.assignRoles` or `users.oidc.assignRoles` instead. */ + assignRoles: (roleNames: string | string[], userId: string) => Promise; + /** @deprecated: Use `users.db.revokeRoles` or `users.oidc.revokeRoles` instead. */ + revokeRoles: (roleNames: string | string[], userId: string) => Promise; + /** * Retrieve the information relevant to the currently authenticated user. * From 99b1b80069c3ffb425ec402903c1bdfc2710c606 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 13:07:13 +0200 Subject: [PATCH 3/6] fix(roles): bring back assignedUserIds method to avoid breaking changes Add deprecation notice to it. --- src/roles/index.ts | 16 +++++++++++++++- src/roles/integration.test.ts | 6 +++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/roles/index.ts b/src/roles/index.ts index 6971cf4f..68dd7e4f 100644 --- a/src/roles/index.ts +++ b/src/roles/index.ts @@ -34,12 +34,25 @@ export interface Roles { * @returns {Promise} The role if it exists, or null if it does not. */ byName: (roleName: string) => Promise; + + /** + * Retrieve the user IDs assigned to a role. + * + * @param {string} roleName The name of the role to retrieve the assigned user IDs for. + * @returns {Promise} The user IDs assigned to the role. + * + * @deprecated: Use `userAssignments` instead. + */ + assignedUserIds: (roleName: string) => Promise; /** * Retrieve the user IDs assigned to a role. Each user has a qualifying user type, * e.g. `'db_user' | 'db_env_user' | 'oidc'`. * + * Note, unlike `assignedUserIds`, this method may return multiple entries for the same username, + * if OIDC authentication is enabled: once with 'db_*' and once with 'oidc' user type. + * * @param {string} roleName The name of the role to retrieve the assigned user IDs for. - * @returns {Promise} The user IDs assigned to the role. + * @returns {Promise} User IDs and user types assigned to the role. */ userAssignments: (roleName: string) => Promise; /** @@ -95,6 +108,7 @@ const roles = (connection: ConnectionREST): Roles => { listAll: () => connection.get('/authz/roles').then(Map.roles), byName: (roleName: string) => connection.get(`/authz/roles/${roleName}`).then(Map.roleFromWeaviate), + assignedUserIds: (roleName: string) => connection.get(`/authz/roles/${roleName}/users`), userAssignments: (roleName: string) => connection .get(`/authz/roles/${roleName}/user-assignments`, true) diff --git a/src/roles/integration.test.ts b/src/roles/integration.test.ts index 1d335c9f..52540d63 100644 --- a/src/roles/integration.test.ts +++ b/src/roles/integration.test.ts @@ -322,7 +322,7 @@ requireAtLeast( 30, 0 )('namespaced users', () => { - it('retrieves assigned users with namespace', async () => { + it('retrieves assigned users with/without namespace', async () => { await client.roles.create('landlord', { collection: 'Buildings', tenant: 'john-doe', @@ -342,6 +342,10 @@ requireAtLeast( ]) ); + // Legacy + const assignedUsers = await client.roles.assignedUserIds('landlord'); + expect(assignedUsers).toEqual(['Innkeeper', 'custom-user']); + await client.users.db.delete('Innkeeper'); await client.roles.delete('landlord'); }); From d9071d41cdc3ff934f8d8c92c5e3234da2dbfdbb Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 13:14:55 +0200 Subject: [PATCH 4/6] ci: target 1.30.0 (not rc) --- .github/workflows/main.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 4db465ef..fb393a03 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -13,7 +13,7 @@ env: WEAVIATE_127: 1.27.15 WEAVIATE_128: 1.28.11 WEAVIATE_129: 1.29.1 - WEAVIATE_130: 1.30.0-rc.0-6b9a01c + WEAVIATE_130: 1.30.0 concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} From 15a56a48fa0493d2985d8242844c07b598be3891 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 13:37:51 +0200 Subject: [PATCH 5/6] chore: deprecate getAssignedRoles --- src/users/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/users/index.ts b/src/users/index.ts index 9b72ea90..bc11ef2b 100644 --- a/src/users/index.ts +++ b/src/users/index.ts @@ -50,6 +50,8 @@ export interface Users extends UsersBase { * * @param {string} userId The ID of the user to retrieve the assigned roles for. * @returns {Promise>} A map of role names to their respective roles. + * + * @deprecated: Use `users.db.getAssignedRoles` or `users.oidc.getAssignedRoles` instead. */ getAssignedRoles: (userId: string) => Promise>; From 7d9310ab0621162d4bde50c1c09bbf7c222ef58c Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 9 Apr 2025 13:42:27 +0200 Subject: [PATCH 6/6] test: extend checks, verify boolean returns on delete/activate/deactivate --- src/users/integration.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/users/integration.test.ts b/src/users/integration.test.ts index 4d5862db..809b74e7 100644 --- a/src/users/integration.test.ts +++ b/src/users/integration.test.ts @@ -81,14 +81,20 @@ requireAtLeast( // Second activation is a no-op await expect(client.users.db.activate('dynamic-dave')).resolves.toEqual(false); - await client.users.db.deactivate('dynamic-dave'); + await expect(client.users.db.deactivate('dynamic-dave')).resolves.toEqual(true); await expectDave().toHaveProperty('active', false); // Second deactivation is a no-op await expect(client.users.db.deactivate('dynamic-dave', { revokeKey: true })).resolves.toEqual(false); - await client.users.db.delete('dynamic-dave'); + // Re-activate + await expect(client.users.db.activate('dynamic-dave')).resolves.toEqual(true); + + await expect(client.users.db.delete('dynamic-dave')).resolves.toEqual(true); await expectDave(false).toHaveProperty('code', 404); + + // Second deletion is a no-op + await expect(client.users.db.delete('dynamic-dave')).resolves.toEqual(false); }); it('should be able to obtain and rotate api keys', async () => {