Skip to content

Commit

Permalink
refactor(auth): implement post-login tasks for user settings manageme…
Browse files Browse the repository at this point in the history
…nt and remove UserSettingsInterceptor
  • Loading branch information
Junjiequan committed Sep 18, 2024
1 parent e476c40 commit f0afcaa
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 129 deletions.
65 changes: 65 additions & 0 deletions src/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import { flattenObject, parseBoolean } from "src/common/utils";
import { Issuer } from "openid-client";
import { ReturnedAuthLoginDto } from "./dto/returnedLogin.dto";
import { ReturnedUserDto } from "src/users/dto/returned-user.dto";
import { CreateUserSettingsDto } from "src/users/dto/create-user-settings.dto";
import {
FilterComponentType,
UserSettings,
} from "src/users/schemas/user-settings.schema";
import { UpdateUserSettingsDto } from "src/users/dto/update-user-settings.dto";

@Injectable()
export class AuthService {
Expand Down Expand Up @@ -43,6 +49,7 @@ export class AuthService {
async login(user: Omit<User, "password">): Promise<ReturnedAuthLoginDto> {
const expiresIn = this.configService.get<number>("jwt.expiresIn");
const accessToken = this.jwtService.sign(user, { expiresIn });
await this.postLoginTasks(user);
return {
access_token: accessToken,
id: accessToken,
Expand Down Expand Up @@ -122,4 +129,62 @@ export class AuthService {

return { logout: "successful" };
}
/**
* postLoginTasks: Executes additional tasks after user login.
*
* - Checks if the user has userSettings record.
* - If user has no userSetting, it creates default userSetting for the user.
* - If userSetting exist but are invalid (filters does not belong to FilterComponentType), it resets the filters to default - empty array.
*
* @param user - The logged-in user (without password).
*/
async postLoginTasks(user: Omit<User, "password">) {
if (!user) return;

const userId = user._id;

const userSettings = await this.usersService.findByIdUserSettings(userId);

if (!userSettings) {
Logger.log(
`Adding default settings to user ${user.username}`,
"UserSettingsInterceptor",
);
const createUserSettingsDto: CreateUserSettingsDto = {
userId,
filters: [],
conditions: [],
columns: [],
};
await this.usersService.createUserSettings(userId, createUserSettingsDto);
} else {
const isValidFilters = (userSettings: UserSettings): boolean => {
if (userSettings.filters.length === 0) {
return true;
}
return userSettings.filters.every((filter) => {
const [key, value] = Object.entries(filter)[0];
return (
Object.keys(FilterComponentType).includes(key) &&
typeof value === "boolean"
);
});
};

if (!isValidFilters) {
Logger.log(
`Reset default settings to user ${user.username}`,
"UserSettingsInterceptor",
);
const updateUserSettingsDto: UpdateUserSettingsDto = {
...userSettings,
filters: [],
};
await this.usersService.findOneAndUpdateUserSettings(
userId,
updateUserSettingsDto,
);
}
}
}
}
2 changes: 1 addition & 1 deletion src/config/frontend.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"jsonMetadataEnabled": true,
"jupyterHubUrl": "",
"landingPage": "doi.ess.eu/detail/",
"lbBaseURL": "http://127.0.0.1:3000",
"lbBaseURL": "http://localhost:3000",
"logbookEnabled": true,
"loginFormEnabled": true,
"maxDirectDownloadSize": 5000000000,
Expand Down
93 changes: 0 additions & 93 deletions src/users/interceptors/user-settings.interceptor.ts

This file was deleted.

58 changes: 33 additions & 25 deletions src/users/users.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { CaslModule } from "src/casl/casl.module";
import { UsersController } from "./users.controller";
import { UsersService } from "./users.service";
import { UpdateUserSettingsDto } from "./dto/update-user-settings.dto";
import { Request } from "express";

class UsersServiceMock {
findByIdUserIdentity(id: string) {
Expand All @@ -28,10 +29,7 @@ const mockUserSettings = {
columns: [],
datasetCount: 25,
jobCount: 25,
filters: [
{ type: "LocationFilterComponent", visible: true },
{ type: "PidFilterComponent", visible: true },
],
filters: [{ LocationFilter: true }, { PidFilter: true }],
conditions: [{ field: "status", value: "active", operator: "equals" }],
};

Expand All @@ -54,7 +52,6 @@ describe("UsersController", () => {
controller = module.get<UsersController>(UsersController);
usersService = module.get<UsersService>(UsersService);

// bypass authorization
jest
.spyOn(controller as UsersController, "checkUserAuthorization")
.mockImplementation(() => Promise.resolve());
Expand All @@ -65,45 +62,56 @@ describe("UsersController", () => {
});

it("should return user settings with filters and conditions", async () => {
jest
.spyOn(usersService, "findByIdUserSettings")
.mockResolvedValue(mockUserSettings);

const userId = "user1";
const result = await controller.getSettings(
{ user: { _id: userId } },
userId,
);
mockUserSettings._id = userId;

const mockRequest: Partial<Request> = {
user: { _id: userId },
};

const result = await controller.getSettings(mockRequest as Request, userId);

// Assert
expect(result).toEqual(mockUserSettings);
expect(result.filters).toBeDefined();
expect(result.filters.length).toBeGreaterThan(0);
expect(result.conditions).toBeDefined();
expect(result.conditions.length).toBeGreaterThan(0);
expect(result?.filters).toBeDefined();
expect(result?.filters.length).toBeGreaterThan(0);
expect(result?.conditions).toBeDefined();
expect(result?.conditions.length).toBeGreaterThan(0);
});

it("should update user settings with filters and conditions", async () => {
const userId = "user-id";
mockUserSettings._id = userId;

const updatedSettings = {
...mockUserSettings,
filters: [{ type: "PidFilterContainsComponent", visible: false }],
filters: [{ PidFilter: true }],
conditions: [{ field: "status", value: "inactive", operator: "equals" }],
};

const mockRequest: Partial<Request> = {
user: { _id: userId },
};

const expectedResponse = {
...updatedSettings,
_id: userId,
};

jest
.spyOn(usersService, "findOneAndUpdateUserSettings")
.mockResolvedValue(updatedSettings);
.mockResolvedValue(expectedResponse);

const userId = "user-id";
const result = await controller.updateSettings(
{ user: { _id: userId } },
mockRequest as Request,
userId,
updatedSettings,
);

expect(result).toEqual(updatedSettings);
expect(result.filters).toBeDefined();
expect(result.filters.length).toBe(1);
expect(result.conditions).toBeDefined();
expect(result.conditions.length).toBe(1);
expect(result?.filters).toBeDefined();
expect(result?.filters.length).toBe(1);
expect(result?.conditions).toBeDefined();
expect(result?.conditions.length).toBe(1);
});
});
17 changes: 7 additions & 10 deletions src/users/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
Body,
ForbiddenException,
HttpCode,
UseInterceptors,
} from "@nestjs/common";
import {
ApiBearerAuth,
Expand Down Expand Up @@ -43,7 +42,6 @@ import { CreateCustomJwt } from "./dto/create-custom-jwt.dto";
import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard";
import { ReturnedUserDto } from "./dto/returned-user.dto";
import { ReturnedAuthLoginDto } from "src/auth/dto/returnedLogin.dto";
import { UserSettingsInterceptor } from "./interceptors/user-settings.interceptor";

@ApiBearerAuth()
@ApiTags("users")
Expand Down Expand Up @@ -94,23 +92,22 @@ export class UsersController {

@ApiBody({ type: CredentialsDto })
@AllowAny()
@UseInterceptors(UserSettingsInterceptor)
@UseGuards(LocalAuthGuard)
@Post("login")
@ApiOperation({
summary: "Functional accounts login.",
description: "It allows to login with functional (local) accounts.",
summary:
"This endpoint is deprecated and will be removed soon. Use /auth/login instead",
description:
"This endpoint is deprecated and will be removed soon. Use /auth/login instead",
})
@ApiResponse({
status: 201,
type: ReturnedAuthLoginDto,
description:
"Create a new JWT token for anonymous or the user that is currently logged in",
"This endpoint is deprecated and will be removed soon. Use /auth/login instead",
})
async login(
@Req() req: Record<string, unknown>,
): Promise<ReturnedAuthLoginDto> {
return await this.authService.login(req.user as Omit<User, "password">);
async login(@Req() req: Record<string, unknown>): Promise<null> {
return null;
}

@UseGuards(AuthenticatedPoliciesGuard)
Expand Down

0 comments on commit f0afcaa

Please sign in to comment.