user sync optinization
This commit is contained in:
@@ -8,9 +8,11 @@ describe('AuthService', () => {
|
|||||||
let service: AuthService;
|
let service: AuthService;
|
||||||
|
|
||||||
const mockCreateFromToken = jest.fn();
|
const mockCreateFromToken = jest.fn();
|
||||||
|
const mockFindByKeycloakSub = jest.fn();
|
||||||
|
|
||||||
const mockUsersService = {
|
const mockUsersService = {
|
||||||
createFromToken: mockCreateFromToken,
|
createFromToken: mockCreateFromToken,
|
||||||
|
findByKeycloakSub: mockFindByKeycloakSub,
|
||||||
};
|
};
|
||||||
|
|
||||||
const mockAuthUser: AuthenticatedUser = {
|
const mockAuthUser: AuthenticatedUser = {
|
||||||
@@ -202,6 +204,81 @@ describe('AuthService', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('ensureUserExists', () => {
|
||||||
|
it('should return existing user without updating', async () => {
|
||||||
|
mockFindByKeycloakSub.mockResolvedValue(mockUser);
|
||||||
|
|
||||||
|
const result = await service.ensureUserExists(mockAuthUser);
|
||||||
|
|
||||||
|
expect(result).toEqual(mockUser);
|
||||||
|
expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123');
|
||||||
|
expect(mockCreateFromToken).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should create new user if user does not exist', async () => {
|
||||||
|
mockFindByKeycloakSub.mockResolvedValue(null);
|
||||||
|
mockCreateFromToken.mockResolvedValue(mockUser);
|
||||||
|
|
||||||
|
const result = await service.ensureUserExists(mockAuthUser);
|
||||||
|
|
||||||
|
expect(result).toEqual(mockUser);
|
||||||
|
expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123');
|
||||||
|
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 when creating new user', async () => {
|
||||||
|
const authUserNoEmail: AuthenticatedUser = {
|
||||||
|
keycloakSub: 'f:realm:user456',
|
||||||
|
name: 'No Email User',
|
||||||
|
};
|
||||||
|
|
||||||
|
mockFindByKeycloakSub.mockResolvedValue(null);
|
||||||
|
mockCreateFromToken.mockResolvedValue({
|
||||||
|
...mockUser,
|
||||||
|
email: '',
|
||||||
|
name: 'No Email User',
|
||||||
|
});
|
||||||
|
|
||||||
|
await service.ensureUserExists(authUserNoEmail);
|
||||||
|
|
||||||
|
expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user456');
|
||||||
|
expect(mockCreateFromToken).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
email: '',
|
||||||
|
name: 'No Email User',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use "Unknown User" when creating user with no name or username', async () => {
|
||||||
|
const authUserMinimal: AuthenticatedUser = {
|
||||||
|
keycloakSub: 'f:realm:minimal',
|
||||||
|
};
|
||||||
|
|
||||||
|
mockFindByKeycloakSub.mockResolvedValue(null);
|
||||||
|
mockCreateFromToken.mockResolvedValue({
|
||||||
|
...mockUser,
|
||||||
|
name: 'Unknown User',
|
||||||
|
});
|
||||||
|
|
||||||
|
await service.ensureUserExists(authUserMinimal);
|
||||||
|
|
||||||
|
expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:minimal');
|
||||||
|
expect(mockCreateFromToken).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
name: 'Unknown User',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('hasRole', () => {
|
describe('hasRole', () => {
|
||||||
it('should return true if user has the required role', () => {
|
it('should return true if user has the required role', () => {
|
||||||
const result = service.hasRole(mockAuthUser, 'user');
|
const result = service.hasRole(mockAuthUser, 'user');
|
||||||
|
|||||||
@@ -24,6 +24,11 @@ import { User } from '../users/users.entity';
|
|||||||
*
|
*
|
||||||
* For explicit profile sync (webhooks, admin operations), use:
|
* For explicit profile sync (webhooks, admin operations), use:
|
||||||
* - UsersService.syncProfileFromToken() - force sync regardless of changes
|
* - UsersService.syncProfileFromToken() - force sync regardless of changes
|
||||||
|
*
|
||||||
|
* ## Usage Guidelines
|
||||||
|
*
|
||||||
|
* - Use `syncUserFromToken()` for actual login events (WebSocket connections, /users/me)
|
||||||
|
* - Use `ensureUserExists()` for regular API calls that just need the user record
|
||||||
*/
|
*/
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class AuthService {
|
export class AuthService {
|
||||||
@@ -61,6 +66,49 @@ export class AuthService {
|
|||||||
return user;
|
return user;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Ensures a user exists in the local database without updating profile data.
|
||||||
|
* This is optimized for regular API calls that need the user record but don't
|
||||||
|
* need to sync profile data from Keycloak on every request.
|
||||||
|
*
|
||||||
|
* - For new users: Creates with full profile data
|
||||||
|
* - For existing users: Returns existing record WITHOUT updating
|
||||||
|
*
|
||||||
|
* Use this for most API endpoints. Only use syncUserFromToken() for actual
|
||||||
|
* login events (WebSocket connections, /users/me endpoint).
|
||||||
|
*
|
||||||
|
* @param authenticatedUser - User data extracted from JWT token
|
||||||
|
* @returns The user entity
|
||||||
|
*/
|
||||||
|
async ensureUserExists(authenticatedUser: AuthenticatedUser): Promise<User> {
|
||||||
|
const { keycloakSub, email, name, username, picture, roles } =
|
||||||
|
authenticatedUser;
|
||||||
|
|
||||||
|
// Check if user exists
|
||||||
|
const existingUser = await this.usersService.findByKeycloakSub(keycloakSub);
|
||||||
|
|
||||||
|
if (existingUser) {
|
||||||
|
// User exists - return without updating
|
||||||
|
return existingUser;
|
||||||
|
}
|
||||||
|
|
||||||
|
// New user - create with full profile data
|
||||||
|
this.logger.log(
|
||||||
|
`Creating new user from token: ${keycloakSub} (via ensureUserExists)`,
|
||||||
|
);
|
||||||
|
|
||||||
|
const user = await this.usersService.createFromToken({
|
||||||
|
keycloakSub,
|
||||||
|
email: email || '',
|
||||||
|
name: name || username || 'Unknown User',
|
||||||
|
username,
|
||||||
|
picture,
|
||||||
|
roles,
|
||||||
|
});
|
||||||
|
|
||||||
|
return user;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Validates if a user has a specific role.
|
* Validates if a user has a specific role.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -82,6 +82,7 @@ describe('FriendsController', () => {
|
|||||||
|
|
||||||
const mockAuthService = {
|
const mockAuthService = {
|
||||||
syncUserFromToken: jest.fn(),
|
syncUserFromToken: jest.fn(),
|
||||||
|
ensureUserExists: jest.fn(),
|
||||||
};
|
};
|
||||||
|
|
||||||
const mockStateGateway = {
|
const mockStateGateway = {
|
||||||
@@ -106,6 +107,7 @@ describe('FriendsController', () => {
|
|||||||
|
|
||||||
jest.clearAllMocks();
|
jest.clearAllMocks();
|
||||||
mockAuthService.syncUserFromToken.mockResolvedValue(mockUser1);
|
mockAuthService.syncUserFromToken.mockResolvedValue(mockUser1);
|
||||||
|
mockAuthService.ensureUserExists.mockResolvedValue(mockUser1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should be defined', () => {
|
it('should be defined', () => {
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ export class FriendsController {
|
|||||||
@Query() searchDto: SearchUsersDto,
|
@Query() searchDto: SearchUsersDto,
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<UserBasicDto[]> {
|
): Promise<UserBasicDto[]> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.debug(
|
this.logger.debug(
|
||||||
`Searching users with username: ${searchDto.username || 'all'}`,
|
`Searching users with username: ${searchDto.username || 'all'}`,
|
||||||
@@ -126,7 +126,7 @@ export class FriendsController {
|
|||||||
@Body() sendRequestDto: SendFriendRequestDto,
|
@Body() sendRequestDto: SendFriendRequestDto,
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendRequestResponseDto> {
|
): Promise<FriendRequestResponseDto> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.log(
|
this.logger.log(
|
||||||
`User ${user.id} sending friend request to ${sendRequestDto.receiverId}`,
|
`User ${user.id} sending friend request to ${sendRequestDto.receiverId}`,
|
||||||
@@ -161,7 +161,7 @@ export class FriendsController {
|
|||||||
async getReceivedRequests(
|
async getReceivedRequests(
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendRequestResponseDto[]> {
|
): Promise<FriendRequestResponseDto[]> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.debug(`Getting received friend requests for user ${user.id}`);
|
this.logger.debug(`Getting received friend requests for user ${user.id}`);
|
||||||
|
|
||||||
@@ -188,7 +188,7 @@ export class FriendsController {
|
|||||||
async getSentRequests(
|
async getSentRequests(
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendRequestResponseDto[]> {
|
): Promise<FriendRequestResponseDto[]> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.debug(`Getting sent friend requests for user ${user.id}`);
|
this.logger.debug(`Getting sent friend requests for user ${user.id}`);
|
||||||
|
|
||||||
@@ -227,7 +227,7 @@ export class FriendsController {
|
|||||||
@Param('id') requestId: string,
|
@Param('id') requestId: string,
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendRequestResponseDto> {
|
): Promise<FriendRequestResponseDto> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.log(`User ${user.id} accepting friend request ${requestId}`);
|
this.logger.log(`User ${user.id} accepting friend request ${requestId}`);
|
||||||
|
|
||||||
@@ -274,7 +274,7 @@ export class FriendsController {
|
|||||||
@Param('id') requestId: string,
|
@Param('id') requestId: string,
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendRequestResponseDto> {
|
): Promise<FriendRequestResponseDto> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.log(`User ${user.id} denying friend request ${requestId}`);
|
this.logger.log(`User ${user.id} denying friend request ${requestId}`);
|
||||||
|
|
||||||
@@ -307,7 +307,7 @@ export class FriendsController {
|
|||||||
async getFriends(
|
async getFriends(
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<FriendshipResponseDto[]> {
|
): Promise<FriendshipResponseDto[]> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.debug(`Getting friends list for user ${user.id}`);
|
this.logger.debug(`Getting friends list for user ${user.id}`);
|
||||||
|
|
||||||
@@ -355,7 +355,7 @@ export class FriendsController {
|
|||||||
@Param('friendId') friendId: string,
|
@Param('friendId') friendId: string,
|
||||||
@CurrentUser() authUser: AuthenticatedUser,
|
@CurrentUser() authUser: AuthenticatedUser,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
this.logger.log(`User ${user.id} unfriending user ${friendId}`);
|
this.logger.log(`User ${user.id} unfriending user ${friendId}`);
|
||||||
|
|
||||||
|
|||||||
@@ -24,8 +24,11 @@ describe('UsersController', () => {
|
|||||||
findByKeycloakSub: mockFindByKeycloakSub,
|
findByKeycloakSub: mockFindByKeycloakSub,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const mockEnsureUserExists = jest.fn();
|
||||||
|
|
||||||
const mockAuthService = {
|
const mockAuthService = {
|
||||||
syncUserFromToken: mockSyncUserFromToken,
|
syncUserFromToken: mockSyncUserFromToken,
|
||||||
|
ensureUserExists: mockEnsureUserExists,
|
||||||
};
|
};
|
||||||
|
|
||||||
const mockAuthUser: AuthenticatedUser = {
|
const mockAuthUser: AuthenticatedUser = {
|
||||||
@@ -90,7 +93,7 @@ describe('UsersController', () => {
|
|||||||
const updateDto: UpdateUserDto = { name: 'Updated Name' };
|
const updateDto: UpdateUserDto = { name: 'Updated Name' };
|
||||||
const updatedUser = { ...mockUser, name: 'Updated Name' };
|
const updatedUser = { ...mockUser, name: 'Updated Name' };
|
||||||
|
|
||||||
mockSyncUserFromToken.mockResolvedValue(mockUser);
|
mockEnsureUserExists.mockResolvedValue(mockUser);
|
||||||
mockUpdate.mockResolvedValue(updatedUser);
|
mockUpdate.mockResolvedValue(updatedUser);
|
||||||
|
|
||||||
const result = await controller.updateCurrentUser(
|
const result = await controller.updateCurrentUser(
|
||||||
@@ -99,7 +102,7 @@ describe('UsersController', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
expect(result).toBe(updatedUser);
|
expect(result).toBe(updatedUser);
|
||||||
expect(mockSyncUserFromToken).toHaveBeenCalledWith(mockAuthUser);
|
expect(mockEnsureUserExists).toHaveBeenCalledWith(mockAuthUser);
|
||||||
expect(mockUpdate).toHaveBeenCalledWith(
|
expect(mockUpdate).toHaveBeenCalledWith(
|
||||||
mockUser.id,
|
mockUser.id,
|
||||||
updateDto,
|
updateDto,
|
||||||
@@ -185,12 +188,12 @@ describe('UsersController', () => {
|
|||||||
|
|
||||||
describe('deleteCurrentUser', () => {
|
describe('deleteCurrentUser', () => {
|
||||||
it('should delete the current user account', async () => {
|
it('should delete the current user account', async () => {
|
||||||
mockSyncUserFromToken.mockResolvedValue(mockUser);
|
mockEnsureUserExists.mockResolvedValue(mockUser);
|
||||||
mockDelete.mockResolvedValue(undefined);
|
mockDelete.mockResolvedValue(undefined);
|
||||||
|
|
||||||
await controller.deleteCurrentUser(mockAuthUser);
|
await controller.deleteCurrentUser(mockAuthUser);
|
||||||
|
|
||||||
expect(mockSyncUserFromToken).toHaveBeenCalledWith(mockAuthUser);
|
expect(mockEnsureUserExists).toHaveBeenCalledWith(mockAuthUser);
|
||||||
expect(mockDelete).toHaveBeenCalledWith(
|
expect(mockDelete).toHaveBeenCalledWith(
|
||||||
mockUser.id,
|
mockUser.id,
|
||||||
mockAuthUser.keycloakSub,
|
mockAuthUser.keycloakSub,
|
||||||
|
|||||||
@@ -51,7 +51,8 @@ export class UsersController {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Get current authenticated user's profile.
|
* Get current authenticated user's profile.
|
||||||
* This endpoint syncs the user from Keycloak token on each request.
|
* This endpoint syncs the user from Keycloak token to ensure profile data
|
||||||
|
* is up-to-date when explicitly requested by the user.
|
||||||
*/
|
*/
|
||||||
@Get('me')
|
@Get('me')
|
||||||
@ApiOperation({
|
@ApiOperation({
|
||||||
@@ -72,7 +73,8 @@ export class UsersController {
|
|||||||
): Promise<User> {
|
): Promise<User> {
|
||||||
this.logger.debug(`Get current user: ${authUser.keycloakSub}`);
|
this.logger.debug(`Get current user: ${authUser.keycloakSub}`);
|
||||||
|
|
||||||
// Sync user from token (creates if doesn't exist, updates if exists)
|
// Sync user from token - this is one of the few endpoints that should
|
||||||
|
// actively sync profile data, as it's an explicit request for user info
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.syncUserFromToken(authUser);
|
||||||
|
|
||||||
return user;
|
return user;
|
||||||
@@ -106,7 +108,7 @@ export class UsersController {
|
|||||||
this.logger.log(`Update current user: ${authUser.keycloakSub}`);
|
this.logger.log(`Update current user: ${authUser.keycloakSub}`);
|
||||||
|
|
||||||
// First ensure user exists in our system
|
// First ensure user exists in our system
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
// Update the user's profile
|
// Update the user's profile
|
||||||
return this.usersService.update(
|
return this.usersService.update(
|
||||||
@@ -221,7 +223,7 @@ export class UsersController {
|
|||||||
this.logger.log(`Delete current user: ${authUser.keycloakSub}`);
|
this.logger.log(`Delete current user: ${authUser.keycloakSub}`);
|
||||||
|
|
||||||
// First ensure user exists in our system
|
// First ensure user exists in our system
|
||||||
const user = await this.authService.syncUserFromToken(authUser);
|
const user = await this.authService.ensureUserExists(authUser);
|
||||||
|
|
||||||
// Delete the user's account
|
// Delete the user's account
|
||||||
await this.usersService.delete(user.id, authUser.keycloakSub);
|
await this.usersService.delete(user.id, authUser.keycloakSub);
|
||||||
|
|||||||
Reference in New Issue
Block a user