Skip to content

Commit b07c850

Browse files
pyphiliakim
andauthored
feat(password): set login session in password login (#1944)
* feat(password): scope session login management in password login * refactor: remove redirection link for login with password * refactor: use post for logout, apply PR requested changes --------- Co-authored-by: kim <kim.phanhoang@epfl.ch>
1 parent 2cb7732 commit b07c850

6 files changed

Lines changed: 47 additions & 51 deletions

File tree

src/services/auth/plugins/magicLink/magicLink.controller.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,10 @@ describe('Auth routes tests', () => {
220220
});
221221
});
222222

223-
describe('GET /logout', () => {
223+
describe('POST /logout', () => {
224224
it('Authenticate successfully', async () => {
225225
const response = await app.inject({
226-
method: HttpMethod.Get,
226+
method: HttpMethod.Post,
227227
url: '/logout',
228228
});
229229
expect(response.statusCode).toEqual(StatusCodes.NO_CONTENT);

src/services/auth/plugins/magicLink/magicLink.controller.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { getRedirectionLink } from '../../utils';
1818
import captchaPreHandler from '../captcha/captcha';
1919
import { PassportStrategy } from '../passport/strategies';
2020
import type { PassportInfo } from '../passport/types';
21-
import { auth, login, register } from './magicLink.schemas';
21+
import { auth, login, register, signOut } from './magicLink.schemas';
2222
import { MagicLinkService } from './magicLink.service';
2323

2424
const ERROR_SEARCH_PARAM = 'error';
@@ -119,7 +119,7 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
119119
);
120120

121121
// logout
122-
fastify.get('/logout', async (request, reply) => {
122+
fastify.post('/logout', { schema: signOut }, async (request, reply) => {
123123
// logout user, so subsequent calls can not make use of the current user.
124124
request.logout();
125125
// remove session so the cookie is removed by the browser

src/services/auth/plugins/magicLink/magicLink.schemas.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,15 @@ export const auth = {
6363
'4xx': errorSchemaRef,
6464
},
6565
} as const satisfies FastifySchema;
66+
67+
export const signOut = {
68+
operationId: 'signOut',
69+
tags: ['authentication'],
70+
summary: 'Log out',
71+
description: 'Log out from current session',
72+
73+
response: {
74+
[StatusCodes.NO_CONTENT]: Type.Null({ description: 'Successful Response' }),
75+
'4xx': errorSchemaRef,
76+
},
77+
} as const satisfies FastifySchema;

src/services/auth/plugins/password/password.controller.test.ts

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { faker } from '@faker-js/faker';
22
import { compare } from 'bcrypt';
3+
import { compareAsc } from 'date-fns';
34
import { eq } from 'drizzle-orm';
45
import { ReasonPhrases, StatusCodes } from 'http-status-codes';
56
import { Redis } from 'ioredis';
@@ -24,7 +25,7 @@ import { REDIS_CONNECTION } from '../../../../config/redis';
2425
import { PASSWORD_RESET_JWT_EXPIRATION_IN_MINUTES } from '../../../../config/secrets';
2526
import { resolveDependency } from '../../../../di/utils';
2627
import { db } from '../../../../drizzle/db';
27-
import { memberPasswordsTable } from '../../../../drizzle/schema';
28+
import { accountsTable, memberPasswordsTable } from '../../../../drizzle/schema';
2829
import type { MemberRaw } from '../../../../drizzle/types';
2930
import { MailerService } from '../../../../plugins/mailer/mailer.service';
3031
import { assertIsDefined } from '../../../../utils/assertions';
@@ -94,8 +95,17 @@ describe('Password', () => {
9495
captcha: MOCK_CAPTCHA,
9596
},
9697
});
97-
expect(response.statusCode).toEqual(StatusCodes.SEE_OTHER);
98-
expect(response.json()).toHaveProperty('resource');
98+
expect(response.statusCode).toEqual(StatusCodes.NO_CONTENT);
99+
expect(response.headers['set-cookie']).toContain('session=');
100+
101+
// last authenticated at should be updated
102+
const m = await db.query.accountsTable.findFirst({
103+
where: eq(accountsTable.email, actor.email!),
104+
});
105+
assertIsDefined(m);
106+
expect(
107+
compareAsc(new Date(m.lastAuthenticatedAt!), new Date(actor.lastAuthenticatedAt!)),
108+
).toEqual(1);
99109
});
100110

101111
it('Sign In successfully with weak password', async () => {
@@ -114,8 +124,7 @@ describe('Password', () => {
114124
captcha: MOCK_CAPTCHA,
115125
},
116126
});
117-
expect(response.statusCode).toEqual(StatusCodes.SEE_OTHER);
118-
expect(response.json()).toHaveProperty('resource');
127+
expect(response.statusCode).toEqual(StatusCodes.NO_CONTENT);
119128
});
120129

121130
it('Sign In successfully with captcha score = 0', async () => {
@@ -139,8 +148,7 @@ describe('Password', () => {
139148
captcha: MOCK_CAPTCHA,
140149
},
141150
});
142-
expect(response.statusCode).toEqual(StatusCodes.SEE_OTHER);
143-
expect(response.json()).toHaveProperty('resource');
151+
expect(response.statusCode).toEqual(StatusCodes.NO_CONTENT);
144152
});
145153

146154
it('Sign In successfully with captcha score < 0.5', async () => {
@@ -164,8 +172,7 @@ describe('Password', () => {
164172
captcha: MOCK_CAPTCHA,
165173
},
166174
});
167-
expect(response.statusCode).toEqual(StatusCodes.SEE_OTHER);
168-
expect(response.json()).toHaveProperty('resource');
175+
expect(response.statusCode).toEqual(StatusCodes.NO_CONTENT);
169176
});
170177

171178
it('Sign In does send unauthorized error for wrong password', async () => {
@@ -365,7 +372,7 @@ describe('Password', () => {
365372

366373
// Try to login with the new password
367374
const responseLogin = await login(app, member.email, newPassword);
368-
expect(responseLogin.statusCode).toBe(StatusCodes.SEE_OTHER);
375+
expect(responseLogin.statusCode).toBe(StatusCodes.NO_CONTENT);
369376

370377
// Try to login with the old password
371378
const responseLoginOld = await login(app, member.email, password);
@@ -389,7 +396,7 @@ describe('Password', () => {
389396
assertIsDefined(anotherMember);
390397
assertIsMemberForTest(anotherMember);
391398
const responseLoginDifferent = await login(app, anotherMember.email, anotherPassword);
392-
expect(responseLoginDifferent.statusCode).toBe(StatusCodes.SEE_OTHER);
399+
expect(responseLoginDifferent.statusCode).toBe(StatusCodes.NO_CONTENT);
393400

394401
// token should be single use
395402
const responseSecondReset = await app.inject({
@@ -729,16 +736,8 @@ describe('Password', () => {
729736
url: '/login-password',
730737
payload: { email: actor.email, password: pwd, captcha: MOCK_CAPTCHA },
731738
});
732-
733-
expect(loginResponse.statusCode).toEqual(StatusCodes.SEE_OTHER);
734-
735-
const response = await app.inject({
736-
method: HttpMethod.Get,
737-
url: loginResponse.json().resource,
738-
});
739-
740-
expect(response.statusCode).toEqual(StatusCodes.SEE_OTHER);
741-
expect(response.headers['set-cookie']).toContain('session=');
739+
expect(loginResponse.statusCode).toEqual(StatusCodes.NO_CONTENT);
740+
expect(loginResponse.headers['set-cookie']).toContain('session=');
742741
});
743742
});
744743
});

src/services/auth/plugins/password/password.controller.ts

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,16 @@ import type { FastifyPluginAsyncTypebox } from '@fastify/type-provider-typebox';
44

55
import { ActionTriggers, Context, RecaptchaAction } from '@graasp/sdk';
66

7-
import { LOGIN_TOKEN_EXPIRATION_IN_MINUTES } from '../../../../config/secrets';
87
import { resolveDependency } from '../../../../di/utils';
98
import { db } from '../../../../drizzle/db';
109
import type { ActionInsertDTO } from '../../../../drizzle/types';
1110
import { asDefined } from '../../../../utils/assertions';
12-
import { PUBLIC_URL } from '../../../../utils/config';
1311
import { ActionService } from '../../../action/action.service';
1412
import { View } from '../../../item/plugins/action/itemAction.schemas';
13+
import { MemberService } from '../../../member/member.service';
1514
import { validatedMemberAccountRole } from '../../../member/strategies/validatedMemberAccountRole';
16-
import { getRedirectionLink } from '../../utils';
1715
import captchaPreHandler from '../captcha/captcha';
1816
import {
19-
SHORT_TOKEN_PARAM,
2017
authenticatePassword,
2118
authenticatePasswordReset,
2219
isAuthenticated,
@@ -32,11 +29,9 @@ import {
3229
} from './password.schemas';
3330
import { MemberPasswordService } from './password.service';
3431

35-
const REDIRECTION_URL_PARAM = 'url';
36-
const AUTHENTICATION_FALLBACK_ROUTE = '/auth';
37-
3832
const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
3933
const actionService = resolveDependency(ActionService);
34+
const memberService = resolveDependency(MemberService);
4035
const memberPasswordService = resolveDependency(MemberPasswordService);
4136

4237
// login with password
@@ -52,23 +47,16 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
5247
],
5348
},
5449
async (request, reply) => {
55-
const { body, log, user } = request;
56-
const { url } = body;
57-
const member = asDefined(user?.account);
58-
59-
const token = memberPasswordService.generateToken(
60-
{ sub: member.id },
61-
`${LOGIN_TOKEN_EXPIRATION_IN_MINUTES}m`,
62-
);
63-
const redirectionUrl = getRedirectionLink(log, url);
50+
const { user } = request;
51+
request.logIn(user, { session: true });
6452

65-
const target = new URL(AUTHENTICATION_FALLBACK_ROUTE, PUBLIC_URL);
66-
target.searchParams.set(SHORT_TOKEN_PARAM, token);
67-
target.searchParams.set(REDIRECTION_URL_PARAM, redirectionUrl);
68-
const resource = target.toString();
53+
// update last authenticated date
54+
const member = asDefined(user?.account);
55+
await db.transaction(async (tx) => {
56+
await memberService.refreshLastAuthenticatedAt(tx, member.id);
57+
});
6958

70-
reply.status(StatusCodes.SEE_OTHER);
71-
return { resource };
59+
reply.status(StatusCodes.NO_CONTENT);
7260
},
7361
);
7462

src/services/auth/plugins/password/password.schemas.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,12 @@ export const signInWithPassword = {
1616
email: Type.String({ format: 'email' }),
1717
password: Type.String(),
1818
captcha: Type.String(),
19-
url: Type.Optional(Type.String({ format: 'uri' })),
2019
}),
2120
querystring: customType.StrictObject({
2221
lang: Type.Optional(Type.String()),
2322
}),
2423
response: {
25-
[StatusCodes.OK]: customType.StrictObject({
26-
resource: Type.String({ description: 'Redirection link' }),
27-
}),
24+
[StatusCodes.NO_CONTENT]: Type.Null({ description: 'Successful Response' }),
2825
'4xx': errorSchemaRef,
2926
'5xx': errorSchemaRef,
3027
},

0 commit comments

Comments
 (0)