refined auth

src/auth/auth.service.spec.ts

@@ -7,13 +7,9 @@ import { User } from '../users/users.entity';
 describe('AuthService', () => {
   let service: AuthService;
 
-  const mockFindByKeycloakSub = jest.fn();
-  const mockUpdateFromToken = jest.fn();
   const mockCreateFromToken = jest.fn();
 
   const mockUsersService = {
-    findByKeycloakSub: mockFindByKeycloakSub,
-    updateFromToken: mockUpdateFromToken,
     createFromToken: mockCreateFromToken,
   };
 
@@ -62,13 +58,11 @@ describe('AuthService', () => {
 
   describe('syncUserFromToken', () => {
     it('should create a new user if user does not exist', async () => {
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue(mockUser);
 
       const result = await service.syncUserFromToken(mockAuthUser);
 
       expect(result).toEqual(mockUser);
-      expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123');
       expect(mockCreateFromToken).toHaveBeenCalledWith({
         keycloakSub: 'f:realm:user123',
         email: 'test@example.com',
@@ -77,32 +71,23 @@ describe('AuthService', () => {
         picture: 'https://example.com/avatar.jpg',
         roles: ['user', 'premium'],
       });
-      expect(mockUpdateFromToken).not.toHaveBeenCalled();
     });
 
-    it('should update existing user if user exists', async () => {
+    it('should handle existing user via upsert', async () => {
       const updatedUser = { ...mockUser, lastLoginAt: new Date('2024-02-01') };
-      mockFindByKeycloakSub.mockReturnValue(mockUser);
-      mockUpdateFromToken.mockReturnValue(updatedUser);
+      mockCreateFromToken.mockReturnValue(updatedUser);
 
       const result = await service.syncUserFromToken(mockAuthUser);
 
       expect(result).toEqual(updatedUser);
-      expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123');
-
-      expect(mockUpdateFromToken).toHaveBeenCalledWith(
-        'f:realm:user123',
-        expect.objectContaining({
-          email: 'test@example.com',
-          name: 'Test User',
-          username: 'testuser',
-          picture: 'https://example.com/avatar.jpg',
-          roles: ['user', 'premium'],
-
-          lastLoginAt: expect.any(Date),
-        }),
-      );
-      expect(mockCreateFromToken).not.toHaveBeenCalled();
+      expect(mockCreateFromToken).toHaveBeenCalledWith({
+        keycloakSub: 'f:realm:user123',
+        email: 'test@example.com',
+        name: 'Test User',
+        username: 'testuser',
+        picture: 'https://example.com/avatar.jpg',
+        roles: ['user', 'premium'],
+      });
     });
 
     it('should handle user with no email by using empty string', async () => {
@@ -111,7 +96,6 @@ describe('AuthService', () => {
         name: 'No Email User',
       };
 
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue({
         ...mockUser,
         email: '',
@@ -131,30 +115,28 @@ describe('AuthService', () => {
     it('should handle user with no name by using username or fallback', async () => {
       const authUserNoName: AuthenticatedUser = {
         keycloakSub: 'f:realm:user789',
-        username: 'someusername',
+        username: 'fallbackuser',
       };
 
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue({
         ...mockUser,
-        name: 'someusername',
+        name: 'fallbackuser',
       });
 
       await service.syncUserFromToken(authUserNoName);
 
       expect(mockCreateFromToken).toHaveBeenCalledWith(
         expect.objectContaining({
-          name: 'someusername',
+          name: 'fallbackuser',
         }),
       );
     });
 
     it('should use "Unknown User" when no name or username is available', async () => {
       const authUserMinimal: AuthenticatedUser = {
-        keycloakSub: 'f:realm:user000',
+        keycloakSub: 'f:realm:minimal',
       };
 
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue({
         ...mockUser,
         name: 'Unknown User',
@@ -176,7 +158,6 @@ describe('AuthService', () => {
         name: 'Empty Sub User',
       };
 
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue({
         ...mockUser,
         keycloakSub: '',
@@ -186,7 +167,6 @@ describe('AuthService', () => {
 
       const result = await service.syncUserFromToken(authUserEmptySub);
 
-      expect(mockFindByKeycloakSub).toHaveBeenCalledWith('');
       expect(mockCreateFromToken).toHaveBeenCalledWith(
         expect.objectContaining({
           keycloakSub: '',
@@ -203,7 +183,6 @@ describe('AuthService', () => {
         name: 'Malformed User',
       };
 
-      mockFindByKeycloakSub.mockReturnValue(null);
       mockCreateFromToken.mockReturnValue({
         ...mockUser,
         keycloakSub: 'invalid-format',
@@ -213,7 +192,6 @@ describe('AuthService', () => {
 
       const result = await service.syncUserFromToken(authUserMalformed);
 
-      expect(mockFindByKeycloakSub).toHaveBeenCalledWith('invalid-format');
       expect(mockCreateFromToken).toHaveBeenCalledWith(
         expect.objectContaining({
           keycloakSub: 'invalid-format',

src/auth/auth.service.ts

@@ -7,8 +7,23 @@ import { User } from '../users/users.entity';
  * Authentication Service
  *
  * Handles authentication-related business logic including:
- * - User synchronization from Keycloak tokens
- * - Profile updates for authenticated users
+ * - User login tracking from Keycloak tokens
+ * - Profile synchronization from Keycloak
+ * - Role-based authorization checks
+ *
+ * ## User Sync Strategy
+ *
+ * On every authentication:
+ * - Creates new users with full profile data from Keycloak
+ * - For existing users, compares Keycloak data with local database:
+ *   - If profile changed: Updates all fields (email, name, username, picture, roles, lastLoginAt)
+ *   - If profile unchanged: Only updates lastLoginAt (lightweight operation)
+ *
+ * This optimizes database performance since reads are cheaper than writes in PostgreSQL.
+ * Most logins only update lastLoginAt, but profile changes sync automatically.
+ *
+ * For explicit profile sync (webhooks, admin operations), use:
+ * - UsersService.syncProfileFromToken() - force sync regardless of changes
  */
 @Injectable()
 export class AuthService {
@@ -17,42 +32,31 @@ export class AuthService {
   constructor(private readonly usersService: UsersService) {}
 
   /**
-   * Synchronizes a user from Keycloak token to local database.
-   * Creates a new user if they don't exist, or updates their last login time.
+   * Handles user login and profile synchronization from Keycloak token.
+   * Creates new users with full profile data.
+   * For existing users, intelligently syncs only changed fields to optimize performance.
+   *
+   * The service compares Keycloak data with local database and:
+   * - Updates profile fields only if they changed from Keycloak
+   * - Always updates lastLoginAt to track login activity
    *
    * @param authenticatedUser - User data extracted from JWT token
-   * @returns The synchronized user entity
+   * @returns The user entity
    */
   async syncUserFromToken(authenticatedUser: AuthenticatedUser): Promise<User> {
     const { keycloakSub, email, name, username, picture, roles } =
       authenticatedUser;
 
-    // Try to find existing user by Keycloak subject
-    let user = await this.usersService.findByKeycloakSub(keycloakSub);
-
-    if (user) {
-      // User exists - update last login and sync profile data
-      this.logger.debug(`Syncing existing user: ${keycloakSub}`);
-      user = await this.usersService.updateFromToken(keycloakSub, {
-        email,
-        name,
-        username,
-        picture,
-        roles,
-        lastLoginAt: new Date(),
-      });
-    } else {
-      // New user - create from token data
-      this.logger.log(`Creating new user from token: ${keycloakSub}`);
-      user = await this.usersService.createFromToken({
-        keycloakSub,
-        email: email || '',
-        name: name || username || 'Unknown User',
-        username,
-        picture,
-        roles,
-      });
-    }
+    // Use createFromToken which handles upsert atomically
+    // This prevents race conditions when multiple requests arrive simultaneously
+    const user = await this.usersService.createFromToken({
+      keycloakSub,
+      email: email || '',
+      name: name || username || 'Unknown User',
+      username,
+      picture,
+      roles,
+    });
 
     return user;
   }