Merge pull request #285 from weaviate/fix/false-on-conflict

fix: revert breaking change and add deprecation notices

-  Bring back roles.assignedUserIds -- I'd got a false impression that this method was removed in the Python client and replaced it with userAssignments rather than deprecating it.
- Return false on HTTP 409 Conflict from /activate and /deactivate endpoints.
- Add deprecation notices to users.assignRoles, users.revokeRoles, and roles.assignedUserIds

Author: bevzzz

.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 }}

src/roles/index.ts

@@ -34,12 +34,25 @@ export interface Roles {
    * @returns {Promise<Role | null>} The role if it exists, or null if it does not.
    */
   byName: (roleName: string) => Promise<Role | null>;
+
+  /**
+   * 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<string[]>} The user IDs assigned to the role.
+   *
+   * @deprecated: Use `userAssignments` instead.
+   */
+  assignedUserIds: (roleName: string) => Promise<string[]>;
   /**
    * 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<string[]>} The user IDs assigned to the role.
+   * @returns {Promise<UserAssignment[]>} User IDs and user types assigned to the role.
    */
   userAssignments: (roleName: string) => Promise<UserAssignment[]>;
   /**
@@ -95,6 +108,7 @@ const roles = (connection: ConnectionREST): Roles => {
     listAll: () => connection.get<WeaviateRole[]>('/authz/roles').then(Map.roles),
     byName: (roleName: string) =>
       connection.get<WeaviateRole>(`/authz/roles/${roleName}`).then(Map.roleFromWeaviate),
+    assignedUserIds: (roleName: string) => connection.get<string[]>(`/authz/roles/${roleName}/users`),
     userAssignments: (roleName: string) =>
       connection
         .get<WeaviateAssignedUser[]>(`/authz/roles/${roleName}/user-assignments`, true)

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');
     });

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<void>;
+  /** @deprecated: Use `users.db.revokeRoles` or `users.oidc.revokeRoles` instead. */
+  revokeRoles: (roleNames: string | string[], userId: string) => Promise<void>;
+
   /**
    * Retrieve the information relevant to the currently authenticated user.
    *
@@ -45,6 +50,8 @@ export interface Users extends UsersBase {
    *
    * @param {string} userId The ID of the user to retrieve the assigned roles for.
    * @returns {Promise<Record<string, Role>>} A map of role names to their respective roles.
+   *
+   * @deprecated: Use `users.db.getAssignedRoles` or `users.oidc.getAssignedRoles` instead.
    */
   getAssignedRoles: (userId: string) => Promise<Record<string, Role>>;
 
@@ -147,11 +154,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;
     };

src/users/integration.test.ts

@@ -79,16 +79,22 @@ 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 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(true);
+      await expect(client.users.db.deactivate('dynamic-dave', { revokeKey: true })).resolves.toEqual(false);
+
+      // Re-activate
+      await expect(client.users.db.activate('dynamic-dave')).resolves.toEqual(true);
 
-      await client.users.db.delete('dynamic-dave');
+      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 () => {