refactor: replace signIn callback with signIn event, adjust getUserByEmail in adapter to check provider (#1223)

* refactor: replace signIn callback with signIn event, adjust getUserByEmail in adapter to check provider

* test: adjusting tests for adapter and events

* docs: add comments for unknown auth provider

* fix: missing dayjs import

packages/auth/providers/test/basic-authorization.spec.ts

@@ -25,7 +25,6 @@ describe("authorizeWithBasicCredentials", () => {
     const result = await authorizeWithBasicCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "basic",
     });
 
     // Assert
@@ -47,7 +46,6 @@ describe("authorizeWithBasicCredentials", () => {
     const result = await authorizeWithBasicCredentialsAsync(db, {
       name: "test",
       password: "wrong",
-      credentialType: "basic",
     });
 
     // Assert
@@ -69,7 +67,6 @@ describe("authorizeWithBasicCredentials", () => {
     const result = await authorizeWithBasicCredentialsAsync(db, {
       name: "wrong",
       password: "test",
-      credentialType: "basic",
     });
 
     // Assert
@@ -88,7 +85,6 @@ describe("authorizeWithBasicCredentials", () => {
     const result = await authorizeWithBasicCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "basic",
     });
 
     // Assert

packages/auth/providers/test/ldap-authorization.spec.ts

@@ -3,7 +3,7 @@ import { describe, expect, test, vi } from "vitest";
 
 import type { Database } from "@homarr/db";
 import { and, createId, eq } from "@homarr/db";
-import { groupMembers, groups, users } from "@homarr/db/schema/sqlite";
+import { groups, users } from "@homarr/db/schema/sqlite";
 import { createDb } from "@homarr/db/test";
 
 import { authorizeWithLdapCredentialsAsync } from "../credentials/authorization/ldap-authorization";
@@ -34,7 +34,6 @@ describe("authorizeWithLdapCredentials", () => {
       authorizeWithLdapCredentialsAsync(null as unknown as Database, {
         name: "test",
         password: "test",
-        credentialType: "ldap",
       });
 
     // Assert
@@ -57,7 +56,6 @@ describe("authorizeWithLdapCredentials", () => {
       authorizeWithLdapCredentialsAsync(null as unknown as Database, {
         name: "test",
         password: "test",
-        credentialType: "ldap",
       });
 
     // Assert
@@ -87,7 +85,6 @@ describe("authorizeWithLdapCredentials", () => {
       authorizeWithLdapCredentialsAsync(null as unknown as Database, {
         name: "test",
         password: "test",
-        credentialType: "ldap",
       });
 
     // Assert
@@ -120,7 +117,6 @@ describe("authorizeWithLdapCredentials", () => {
       authorizeWithLdapCredentialsAsync(null as unknown as Database, {
         name: "test",
         password: "test",
-        credentialType: "ldap",
       });
 
     // Assert
@@ -152,11 +148,11 @@ describe("authorizeWithLdapCredentials", () => {
     const result = await authorizeWithLdapCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "ldap",
     });
 
     // Assert
     expect(result.name).toBe("test");
+    expect(result.groups).toHaveLength(0); // Groups are needed in signIn events callback
     const dbUser = await db.query.users.findFirst({
       where: eq(users.name, "test"),
     });
@@ -197,11 +193,11 @@ describe("authorizeWithLdapCredentials", () => {
     const result = await authorizeWithLdapCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "ldap",
     });
 
     // Assert
     expect(result.name).toBe("test");
+    expect(result.groups).toHaveLength(0); // Groups are needed in signIn events callback
     const dbUser = await db.query.users.findFirst({
       where: and(eq(users.name, "test"), eq(users.provider, "ldap")),
     });
@@ -219,7 +215,8 @@ describe("authorizeWithLdapCredentials", () => {
     expect(credentialsUser?.id).not.toBe(result.id);
   });
 
-  test("should authorize user with correct credentials and update name", async () => {
+  // The name update occurs in the signIn event callback
+  test("should authorize user with correct credentials and return updated name", async () => {
     // Arrange
     const spy = vi.spyOn(ldapClient, "LdapClient");
     spy.mockImplementation(
@@ -251,11 +248,10 @@ describe("authorizeWithLdapCredentials", () => {
     const result = await authorizeWithLdapCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "ldap",
     });
 
     // Assert
-    expect(result).toEqual({ id: userId, name: "test" });
+    expect(result).toEqual({ id: userId, name: "test", groups: [] });
 
     const dbUser = await db.query.users.findFirst({
       where: eq(users.id, userId),
@@ -263,12 +259,12 @@ describe("authorizeWithLdapCredentials", () => {
 
     expect(dbUser).toBeDefined();
     expect(dbUser?.id).toBe(userId);
-    expect(dbUser?.name).toBe("test");
+    expect(dbUser?.name).toBe("test-old");
     expect(dbUser?.email).toBe("test@gmail.com");
     expect(dbUser?.provider).toBe("ldap");
   });
 
-  test("should authorize user with correct credentials and add him to the groups that he is in LDAP but not in Homar", async () => {
+  test("should authorize user with correct credentials and return his groups", async () => {
     // Arrange
     const spy = vi.spyOn(ldapClient, "LdapClient");
     spy.mockImplementation(
@@ -311,83 +307,9 @@ describe("authorizeWithLdapCredentials", () => {
     const result = await authorizeWithLdapCredentialsAsync(db, {
       name: "test",
       password: "test",
-      credentialType: "ldap",
     });
 
     // Assert
-    expect(result).toEqual({ id: userId, name: "test" });
-
-    const dbGroupMembers = await db.query.groupMembers.findMany();
-    expect(dbGroupMembers).toHaveLength(1);
-  });
-
-  test("should authorize user with correct credentials and remove him from groups he is in Homarr but not in LDAP", async () => {
-    // Arrange
-    const spy = vi.spyOn(ldapClient, "LdapClient");
-    spy.mockImplementation(
-      () =>
-        ({
-          bindAsync: vi.fn(() => Promise.resolve()),
-          searchAsync: vi.fn((argument: { options: { filter: string } }) =>
-            argument.options.filter.includes("group")
-              ? Promise.resolve([
-                  {
-                    cn: "homarr_example",
-                  },
-                ])
-              : Promise.resolve([
-                  {
-                    dn: "test55",
-                    mail: "test@gmail.com",
-                  },
-                ]),
-          ),
-          disconnectAsync: vi.fn(),
-        }) as unknown as ldapClient.LdapClient,
-    );
-    const db = createDb();
-    const userId = createId();
-    await db.insert(users).values({
-      id: userId,
-      name: "test",
-      email: "test@gmail.com",
-      provider: "ldap",
-    });
-
-    const groupIds = [createId(), createId()] as const;
-    await db.insert(groups).values([
-      {
-        id: groupIds[0],
-        name: "homarr_example",
-      },
-      {
-        id: groupIds[1],
-        name: "homarr_no_longer_member",
-      },
-    ]);
-    await db.insert(groupMembers).values([
-      {
-        userId,
-        groupId: groupIds[0],
-      },
-      {
-        userId,
-        groupId: groupIds[1],
-      },
-    ]);
-
-    // Act
-    const result = await authorizeWithLdapCredentialsAsync(db, {
-      name: "test",
-      password: "test",
-      credentialType: "ldap",
-    });
-
-    // Assert
-    expect(result).toEqual({ id: userId, name: "test" });
-
-    const dbGroupMembers = await db.query.groupMembers.findMany();
-    expect(dbGroupMembers).toHaveLength(1);
-    expect(dbGroupMembers[0]?.groupId).toBe(groupIds[0]);
+    expect(result).toEqual({ id: userId, name: "test", groups: ["homarr_example"] });
   });
 });