Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show deleted user sessions #8749

Merged
merged 10 commits into from
Nov 14, 2024
14 changes: 13 additions & 1 deletion frontend/src/component/user/HostedAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { LOGIN_BUTTON, LOGIN_EMAIL_ID, LOGIN_PASSWORD_ID } from 'utils/testIds';
import type { IAuthEndpointDetailsResponse } from 'hooks/api/getters/useAuth/useAuthEndpoint';
import { BadRequestError, NotFoundError } from 'utils/apiUtils';
import { contentSpacingY } from 'themes/themeStyles';
import useToast from 'hooks/useToast';

interface IHostedAuthProps {
authDetails: IAuthEndpointDetailsResponse;
Expand Down Expand Up @@ -47,6 +48,7 @@ const HostedAuth: VFC<IHostedAuthProps> = ({ authDetails, redirect }) => {
passwordError?: string;
apiError?: string;
}>({});
const { setToastData } = useToast();

const handleSubmit: FormEventHandler<HTMLFormElement> = async (evt) => {
evt.preventDefault();
Expand All @@ -69,7 +71,17 @@ const HostedAuth: VFC<IHostedAuthProps> = ({ authDetails, redirect }) => {
}

try {
await passwordAuth(authDetails.path, username, password);
const data = await passwordAuth(
authDetails.path,
username,
password,
);
if (data.deletedSessions) {
setToastData({
type: 'success',
title: `You have been logged out of ${data.deletedSessions} stale session(s)`,
});
}
refetchUser();
navigate(redirect, { replace: true });
} catch (error: any) {
Expand Down
16 changes: 15 additions & 1 deletion frontend/src/component/user/PasswordAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
NotFoundError,
} from 'utils/apiUtils';
import { contentSpacingY } from 'themes/themeStyles';
import useToast from 'hooks/useToast';

interface IPasswordAuthProps {
authDetails: IAuthEndpointDetailsResponse;
Expand Down Expand Up @@ -46,6 +47,7 @@ const PasswordAuth: VFC<IPasswordAuthProps> = ({ authDetails, redirect }) => {
passwordError?: string;
apiError?: string;
}>({});
const { setToastData } = useToast();

const handleSubmit: FormEventHandler<HTMLFormElement> = async (evt) => {
evt.preventDefault();
Expand All @@ -68,7 +70,19 @@ const PasswordAuth: VFC<IPasswordAuthProps> = ({ authDetails, redirect }) => {
}

try {
await passwordAuth(authDetails.path, username, password);
const data = await passwordAuth(
authDetails.path,
username,
password,
);
if (data.deletedSessions && data.activeSessions) {
setToastData({
type: 'success',
title: 'Maximum Session Limit Reached',
text: `You can have up to ${data.activeSessions} active sessions at a time. To allow this login, we’ve logged out ${data.deletedSessions} session(s) from other browsers.`,
});
}

refetchUser();
navigate(redirect, { replace: true });
} catch (error: any) {
Expand Down
14 changes: 11 additions & 3 deletions frontend/src/hooks/api/actions/useAuthApi/useAuthApi.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { headers } from 'utils/apiUtils';
import useAPI from '../useApi/useApi';
import type { UserSchema } from 'openapi';

type PasswordLogin = (
path: string,
username: string,
password: string,
) => Promise<Response>;
) => Promise<UserSchema>;

type EmailLogin = (path: string, email: string) => Promise<Response>;

Expand All @@ -21,7 +22,11 @@ export const useAuthApi = (): IUseAuthApiOutput => {
propagateErrors: true,
});

const passwordAuth = (path: string, username: string, password: string) => {
const passwordAuth = async (
path: string,
username: string,
password: string,
): Promise<UserSchema> => {
const req = {
caller: () => {
return fetch(path, {
Expand All @@ -33,7 +38,10 @@ export const useAuthApi = (): IUseAuthApiOutput => {
id: 'passwordAuth',
};

return makeRequest(req.caller, req.id);
const res = await makeRequest(req.caller, req.id);
const data = await res.json();

return data;
};

const emailAuth = (path: string, email: string) => {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/openapi/models/userSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,6 @@ export interface UserSchema {
* @nullable
*/
username?: string | null;
deletedSessions?: number;
activeSessions?: number;
}
6 changes: 6 additions & 0 deletions src/lib/openapi/spec/user-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ export const userSchema = {
nullable: true,
example: 2,
},
deletedSessions: {
description:
'Experimental. The number of deleted sessions after the last login',
type: 'number',
example: 1,
},
},
components: {},
} as const;
Expand Down
10 changes: 7 additions & 3 deletions src/lib/services/session-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ export default class SessionService {
async deleteStaleSessionsForUser(
userId: number,
maxSessions: number,
): Promise<void> {
const userSessions: ISession[] =
await this.sessionStore.getSessionsForUser(userId);
): Promise<number> {
let userSessions: ISession[] = [];
try {
// this method may throw errors when no session
userSessions = await this.sessionStore.getSessionsForUser(userId);
} catch (e) {}
const newestFirst = userSessions.sort((a, b) =>
compareDesc(a.createdAt, b.createdAt),
);
Expand All @@ -48,6 +51,7 @@ export default class SessionService {
this.sessionStore.delete(session.sid),
),
);
return sessionsToDelete.length;
}

async deleteSession(sid: string): Promise<void> {
Expand Down
11 changes: 7 additions & 4 deletions src/lib/services/user-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,13 @@ class UserService {
deleteStaleUserSessions.payload?.value || 30,
);
// subtract current user session that will be created
await this.sessionService.deleteStaleSessionsForUser(
user.id,
Math.max(allowedSessions - 1, 0),
);
const deletedSessionsCount =
await this.sessionService.deleteStaleSessionsForUser(
user.id,
Math.max(allowedSessions - 1, 0),
);
user.deletedSessions = deletedSessionsCount;
user.activeSessions = allowedSessions;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tymek I'm using the field that you added to user schema

}
this.eventBus.emit(USER_LOGIN, { loginOrder });
return user;
Expand Down
2 changes: 2 additions & 0 deletions src/lib/types/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export interface IUser {
imageUrl?: string;
accountType?: AccountType;
scimId?: string;
deletedSessions?: number;
activeSessions?: number;
}

export type MinimalUser = Pick<
Expand Down
75 changes: 66 additions & 9 deletions src/test/e2e/services/user-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import PasswordMismatch from '../../../lib/error/password-mismatch';
import type { EventService } from '../../../lib/services';
import {
CREATE_ADDON,
type IFlagResolver,
type IUnleashStores,
type IUserStore,
SYSTEM_USER_AUDIT,
Expand Down Expand Up @@ -45,6 +46,8 @@ let eventService: EventService;
let accessService: AccessService;
let eventBus: EventEmitter;

const allowedSessions = 2;

beforeAll(async () => {
db = await dbInit('user_service_serial', getLogger);
stores = db.stores;
Expand All @@ -63,14 +66,31 @@ beforeAll(async () => {
sessionService = new SessionService(stores, config);
settingService = new SettingService(stores, config, eventService);

userService = new UserService(stores, config, {
accessService,
resetTokenService,
emailService,
eventService,
sessionService,
settingService,
});
const flagResolver = {
isEnabled() {
return true;
},
getVariant() {
return {
feature_enabled: true,
payload: {
value: String(allowedSessions),
},
};
},
} as unknown as IFlagResolver;
userService = new UserService(
stores,
{ ...config, flagResolver },
{
accessService,
resetTokenService,
emailService,
eventService,
sessionService,
settingService,
},
);
userStore = stores.userStore;
const rootRoles = await accessService.getRootRoles();
adminRole = rootRoles.find((r) => r.name === RoleName.ADMIN)!;
Expand All @@ -95,8 +115,9 @@ afterAll(async () => {
await db.destroy();
});

afterEach(async () => {
beforeEach(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a better practice to clean data before not after since it allows debugging failed tests

await userStore.deleteAll();
await settingService.deleteAll();
});

test('should create initial admin user', async () => {
Expand Down Expand Up @@ -361,6 +382,42 @@ test("deleting a user should delete the user's sessions", async () => {
expect(noSessions.length).toBe(0);
});

test('user login should remove stale sessions', async () => {
const email = '[email protected]';
const user = await userService.createUser(
{
email,
password: 'A very strange P4ssw0rd_',
rootRole: adminRole.id,
},
TEST_AUDIT_USER,
);
const userSession = (index: number) => ({
sid: `sid${index}`,
sess: {
cookie: {
originalMaxAge: minutesToMilliseconds(48),
expires: addDays(Date.now(), 1).toDateString(),
secure: false,
httpOnly: true,
path: '/',
},
user,
},
});

for (let i = 0; i < allowedSessions; i++) {
await sessionService.insertSession(userSession(i));
}

const insertedUser = await userService.loginUser(
email,
'A very strange P4ssw0rd_',
);

expect(insertedUser.deletedSessions).toBe(1);
});

test('updating a user without an email should not strip the email', async () => {
const email = '[email protected]';
const user = await userService.createUser(
Expand Down
Loading