27 add omniauth strategies for user authentication by YoungMame · Pull Request #54 · YoungMame/matcha

YoungMame · 2025-11-26T12:03:14Z

42 OAuth

Facebook OAuth

Test multiple app build => built once

Trust proxy true if not in test

…a tests)

Copilot

Pull request overview

This PR adds OAuth authentication support for 42 and Facebook providers, refactors the test suite to use a shared app instance for improved performance, and updates the database schema to track authentication providers.

Implements OAuth2 authentication flows for 42 and Facebook using @fastify/oauth2
Refactors test infrastructure to build the app once and share it across tests
Adds provider tracking to the user model to differentiate between local and OAuth users

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 22 comments.

Show a summary per file

File	Description
postgresql/init.sql	Adds `provider_` column to users table to track authentication method
fastify/conf/entrypoint.sh	Adds development mode logging
fastify/assets/test/setup.ts	Creates shared test setup with single app instance initialization
fastify/assets/test/unit/app.test.ts	Refactored to use shared app instance from setup
fastify/assets/test/integration/*/.test.ts	All integration tests refactored to use shared app instance
fastify/assets/srcs/utils/geoloc.ts	Adds rate limiting and test environment handling for geolocation API
fastify/assets/srcs/services/UserService.ts	Adds provider parameter support, new getUserByUsername method, and provider validation in login
fastify/assets/srcs/routes/private/ws/index.ts	Adds test environment handling for geolocation in WebSocket connections
fastify/assets/srcs/plugins/oAuth/facebook.ts	Implements Facebook OAuth2 authentication flow
fastify/assets/srcs/plugins/oAuth/42.ts	Implements 42 OAuth2 authentication flow
fastify/assets/srcs/models/User/index.ts	Updates insert and find methods to support provider field
fastify/assets/srcs/classes/User.ts	Adds provider property to User class
fastify/assets/srcs/app.ts	Registers OAuth plugins, adds health endpoint, enables trust proxy
fastify/assets/seed/seed.ts	Fixes typo in location insert query (longitude -> country)
fastify/assets/package.json	Adds `@fastify/oauth2` dependency and updates dev script
fastify/assets/@types/fastify.d.ts	Adds type definitions for OAuth namespaces and updated service methods
fastify/Dockerfile	Adds curl for health checks
docker-compose.*.yml	Adds OAuth credentials environment variables and health checks
.env.example	Documents OAuth credential environment variables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T12:10:59Z

+            while (isUsernameTaken) {
+                const suffix = Math.floor(Math.random() * 10000);
+                const tryUsername = `${baseUsername}${suffix}`;
+                const userByUsername = await this.userService.getUserByUsername(tryUsername);
+                if (!userByUsername) {
+                    baseUsername = tryUsername;
+                    isUsernameTaken = false;
+                }
+            }


Potential infinite loop risk. The username generation loop has no upper bound or timeout. If all possible username variations are taken (unlikely but possible), this could loop indefinitely. Consider adding a maximum retry counter.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-11-26T12:10:59Z

+            while (isUsernameTaken) {
+                const suffix = Math.floor(Math.random() * 10000);
+                const tryUsername = `${baseUsername}${suffix}`;
+                const userByUsername = await this.userService.getUserByUsername(tryUsername);
+                if (!userByUsername) {
+                    baseUsername = tryUsername;
+                    isUsernameTaken = false;
+                }
+            }


Potential infinite loop risk. The username generation loop has no upper bound or timeout. If all possible username variations are taken (unlikely but possible), this could loop indefinitely. Consider adding a maximum retry counter.

Copilot · 2025-11-26T12:11:00Z

+    {
+        if (process.env.NODE_ENV === 'test')
+            return { city: 'Test City', country: 'Test Country' };
+        return console.log('Google Maps API rate limit reached'), { city: '...', country: '...' };


Incorrect use of comma operator. The return statement uses a comma operator: return console.log(...), { city: '...', country: '...' }. While this works, it's confusing and non-idiomatic. The console.log should be on a separate line before the return statement for clarity.

Suggested change

return console.log('Google Maps API rate limit reached'), { city: '...', country: '...' };

console.log('Google Maps API rate limit reached');

return { city: '...', country: '...' };

Copilot · 2025-11-26T12:11:00Z

    const app = Fastify({
-        logger: (process.env.NODE_ENV == 'dev')
+        logger: (process.env.NODE_ENV == 'dev'),
+        trustProxy: (process.env.NODE_ENV != 'test')


Use strict inequality operator. Use !== instead of != for type-safe comparison.

Suggested change

trustProxy: (process.env.NODE_ENV != 'test')

trustProxy: (process.env.NODE_ENV !== 'test')

Copilot · 2025-11-26T12:11:00Z

+        if (!user || user.provider !== 'local')
            throw new UnauthorizedError();


The login method now checks if the provider is 'local', but this breaks existing OAuth users who might try to login directly. The error message thrown by UnauthorizedError doesn't explain why the login failed (e.g., "OAuth users cannot login with password" vs "Invalid credentials"). This could be confusing for users.

Copilot · 2025-11-26T12:11:04Z

@@ -1,6 +1,15 @@
 export async function getCityAndCountryFromCoords(latitude: number, longitude: number): Promise<{ city: string, country: string }> {
    const response = await fetch(`https://maps.googleapis.com/maps/api/geocode/json?latlng=${latitude},${longitude}&key=${process.env.GOOGLE_MAPS_API_KEY}`);
+    if (response.status == 429)


Use strict equality operator. Use === instead of == for type-safe comparison.

Suggested change

if (response.status == 429)

if (response.status === 429)

Copilot · 2025-11-26T12:11:04Z

+// after(async function () {
+//     this.timeout(10000);
+//     if (app) {
+//         await app.close();
+//     }
+// });


The commented-out cleanup code in the after hook could lead to resource leaks in tests. The app instance is never closed, which means database connections and other resources may not be properly cleaned up after test runs. This could cause issues in CI/CD environments or when running tests multiple times.

Copilot · 2025-11-26T12:11:04Z

 export const buildApp = () => {
    const app = Fastify({
-        logger: (process.env.NODE_ENV == 'dev')
+        logger: (process.env.NODE_ENV == 'dev'),


Use strict equality operator. Use === instead of == for type-safe comparison.

Suggested change

logger: (process.env.NODE_ENV == 'dev'),

logger: (process.env.NODE_ENV === 'dev'),

Copilot · 2025-11-26T12:11:04Z

            if (rows.rows.length === 0)
            {
-                await this.fastify.pg.query(
+                const result = await this.fastify.pg.query(


Unused variable result.

Suggested change

const result = await this.fastify.pg.query(

await this.fastify.pg.query(

Copilot · 2025-11-26T12:11:05Z

@@ -0,0 +1,102 @@
+import fp from 'fastify-plugin'
+import oauthPlugin, {  } from '@fastify/oauth2'


Unused import oauthPlugin.

Suggested change

import oauthPlugin, { } from '@fastify/oauth2'

Copilot · 2025-11-26T12:17:32Z

@YoungMame I've opened a new pull request, #55, to work on those changes. Once the pull request is ready, I'll request review from you.

…finite loop Co-authored-by: YoungMame <134452452+YoungMame@users.noreply.github.com>

Co-authored-by: YoungMame <134452452+YoungMame@users.noreply.github.com>

Added environment variables for Facebook and FT clients.

Add retry limit to OAuth username generation loops

Copilot

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T12:32:52Z

+        throw new Error('Missing FT_CLIENT_ID or FT_CLIENT_SECRET environment variable');
+
+    fastify.get('/auth/login/42', async function (request, reply) {
+        const url = `https://api.intra.42.fr/oauth/authorize?client_id=u-s4t2ud-164069d052052d412516d442f502a23c7662d19adb38400ed23e2f5a841eac37&redirect_uri=https%3A%2F%2Fmatcha.fr%2Fapi%2Fauth%2Flogin%2F42%2Fcallback%2F&response_type=code`;


Hardcoded client ID in the authorization URL instead of using the CLIENT_ID variable. This exposes the client ID in code and creates inconsistency. Replace with client_id=${CLIENT_ID} and use proper URL encoding for the redirect_uri parameter.

Suggested change

const url = `https://api.intra.42.fr/oauth/authorize?client_id=u-s4t2ud-164069d052052d412516d442f502a23c7662d19adb38400ed23e2f5a841eac37&redirect_uri=https%3A%2F%2Fmatcha.fr%2Fapi%2Fauth%2Flogin%2F42%2Fcallback%2F&response_type=code`;

const redirectUri = 'https://matcha.fr/api/auth/login/42/callback/';

const url = `https://api.intra.42.fr/oauth/authorize?client_id=${CLIENT_ID}&redirect_uri=${encodeURIComponent(redirectUri)}&response_type=code`;

Copilot · 2025-11-26T12:32:52Z

+    // The service provider redirect the user here after successful login
+    fastify.get('/auth/login/facebook/callback/', async function (request, reply) {
+        const { token } = await this.facebookOAuth2.getAccessTokenFromAuthorizationCodeFlow(request);
+        console.log('getNewAccessTokenUsingRefreshToken:', token.access_token);


Access token is being logged to console. This is a security risk as tokens should never be logged in production environments. Remove this console.log statement or conditionally log only in development mode.

Suggested change

console.log('getNewAccessTokenUsingRefreshToken:', token.access_token);

if (process.env.NODE_ENV === 'development') {

console.log('getNewAccessTokenUsingRefreshToken:', token.access_token);

}

Copilot · 2025-11-26T12:32:53Z

+            return reply.status(400).send({ error: 'Invalid user info received from Facebook' });
+        }
+
+        const existigUser = await this.userService.getUser(userInfos.email || '');


Corrected spelling of 'existigUser' to 'existingUser'.

Copilot · 2025-11-26T12:32:53Z

+        const existigUser = await this.userService.getUser(userInfo.email || '');
+        if (existigUser && existigUser.provider !== '42') {
+            return reply.status(400).send({ error: 'Email already used with another provider' });
+        } else if (existigUser && existigUser.provider === '42') {
+            const jwt = await fastify.jwt.sign({ id: existigUser.id, email: existigUser.email, username: existigUser.username, isVerified: existigUser.isVerified, isCompleted: existigUser.isProfileCompleted });


Corrected spelling of 'existigUser' to 'existingUser'.

Suggested change

const existigUser = await this.userService.getUser(userInfo.email || '');

if (existigUser && existigUser.provider !== '42') {

return reply.status(400).send({ error: 'Email already used with another provider' });

} else if (existigUser && existigUser.provider === '42') {

const jwt = await fastify.jwt.sign({ id: existigUser.id, email: existigUser.email, username: existigUser.username, isVerified: existigUser.isVerified, isCompleted: existigUser.isProfileCompleted });

const existingUser = await this.userService.getUser(userInfo.email || '');

if (existingUser && existingUser.provider !== '42') {

return reply.status(400).send({ error: 'Email already used with another provider' });

} else if (existingUser && existingUser.provider === '42') {

const jwt = await fastify.jwt.sign({ id: existingUser.id, email: existingUser.email, username: existingUser.username, isVerified: existingUser.isVerified, isCompleted: existingUser.isProfileCompleted });

Copilot · 2025-11-26T12:32:53Z

+            );
+
+            const existigUser = await this.userService.getUser(userInfos.email);
+            await this.userService.verifyEmail(Number(existigUser.id)!, undefined);


Potential null dereference. The getUser call could return null, but the code immediately accesses existigUser.id with the non-null assertion operator. Add a null check before accessing the user properties.

Suggested change

await this.userService.verifyEmail(Number(existigUser.id)!, undefined);

if (!existigUser) {

return reply.status(500).send({ error: 'Failed to retrieve newly created user' });

}

await this.userService.verifyEmail(Number(existigUser.id), undefined);

Copilot · 2025-11-26T12:32:54Z

+    fastify.get('/auth/login/42/callback/', async function (request: FastifyRequest, reply: FastifyReply) {
+        const q = request.query as { code?: string };
+        const code = q.code;
+


Missing validation for the authorization code. If the code parameter is missing or empty, the OAuth flow should fail gracefully with a clear error message instead of passing undefined to the token exchange endpoint.

Suggested change

if (!code || typeof code !== 'string' || code.trim() === '') {

return reply.status(400).send({ error: 'Missing or invalid authorization code' });

}

Copilot · 2025-11-26T12:32:55Z

+        // register a fastify url to start the redirect flow to the service provider's OAuth2 login
+        startRedirectPath: '/auth/login/facebook',
+        // service provider redirects here after user login
+        callbackUri: req => `https://matcha.fr/api/auth/login/facebook/callback/`,


Hardcoded domain in callback URI. Consider using process.env.DOMAIN or a configurable base URL to support different environments (development, staging, production).

Copilot · 2025-11-26T12:32:55Z

+// after(async function () {
+//     this.timeout(10000);
+//     if (app) {
+//         await app.close();
+//     }
+// });


Commented-out cleanup code. The app instance is never closed after tests complete, which could lead to resource leaks or port conflicts. Either uncomment this code or add a comment explaining why it's intentionally disabled.

Copilot · 2025-11-26T12:32:55Z

@@ -1,6 +1,15 @@
 export async function getCityAndCountryFromCoords(latitude: number, longitude: number): Promise<{ city: string, country: string }> {
    const response = await fetch(`https://maps.googleapis.com/maps/api/geocode/json?latlng=${latitude},${longitude}&key=${process.env.GOOGLE_MAPS_API_KEY}`);
+    if (response.status == 429)


Using loose equality (==) instead of strict equality (===) for status code comparison. Use === for type-safe comparison.

Suggested change

if (response.status == 429)

if (response.status === 429)

Copilot · 2025-11-26T12:32:55Z

+    {
+        if (process.env.NODE_ENV === 'test')
+            return { city: 'Test City', country: 'Test Country' };
+        return console.log('Google Maps API rate limit reached'), { city: '...', country: '...' };


Using comma operator to combine console.log and return statement. This is unclear and makes the code harder to read. Split into two separate statements for better clarity.

Suggested change

return console.log('Google Maps API rate limit reached'), { city: '...', country: '...' };

console.log('Google Maps API rate limit reached');

return { city: '...', country: '...' };

YoungMame added 7 commits November 25, 2025 15:20

fix:(dev env, ipstack 429 code, /user/me test, build app once in moch…

daf9ac7

…a tests)

fix:(pnpm dev tsx)

58e52fb

add:([WIP facebook auth])

ba3a3cb

add:(facebook auth)

9789d5d

add:(42 oaut)

f09f04a

remove:(trust proxy true)

d2865ed

add:(trust proxy true)

6884a3d

Copilot AI review requested due to automatic review settings November 26, 2025 12:03

YoungMame linked an issue Nov 26, 2025 that may be closed by this pull request

Add OmniAuth strategies for user authentication #27

Closed

5 tasks

Copilot started reviewing on behalf of YoungMame November 26, 2025 12:03 View session

Copilot finished reviewing on behalf of YoungMame November 26, 2025 12:06

Copilot AI reviewed Nov 26, 2025

View reviewed changes

YoungMame and others added 2 commits November 26, 2025 13:13

fix:(test docker compose db healthcheck)

e0bde0d

Initial plan

9978bc6

Copilot AI mentioned this pull request Nov 26, 2025

Add retry limit to OAuth username generation loops #55

Merged

Copilot AI and others added 4 commits November 26, 2025 12:20

fix: add max retry counter to username generation loops to prevent in…

a7ce854

…finite loop Co-authored-by: YoungMame <134452452+YoungMame@users.noreply.github.com>

refactor: move MAX_USERNAME_RETRIES constant to module level

523bf31

Co-authored-by: YoungMame <134452452+YoungMame@users.noreply.github.com>

Add Facebook and FT client secrets to workflow

f09c58d

Added environment variables for Facebook and FT clients.

Merge pull request #55 from YoungMame/copilot/sub-pr-54

b0d462f

Add retry limit to OAuth username generation loops

Copilot AI review requested due to automatic review settings November 26, 2025 12:27

Copilot started reviewing on behalf of YoungMame November 26, 2025 12:27 View session

Copilot finished reviewing on behalf of YoungMame November 26, 2025 12:28

YoungMame merged commit d2c379b into main Nov 26, 2025
7 checks passed

YoungMame deleted the 27-add-omniauth-strategies-for-user-authentication branch November 26, 2025 12:31

Copilot AI reviewed Nov 26, 2025

View reviewed changes

	return console.log('Google Maps API rate limit reached'), { city: '...', country: '...' };
	console.log('Google Maps API rate limit reached');
	return { city: '...', country: '...' };

	trustProxy: (process.env.NODE_ENV != 'test')
	trustProxy: (process.env.NODE_ENV !== 'test')

		if (!user \|\| user.provider !== 'local')
		throw new UnauthorizedError();

	logger: (process.env.NODE_ENV == 'dev'),
	logger: (process.env.NODE_ENV === 'dev'),

	const result = await this.fastify.pg.query(
	await this.fastify.pg.query(

		@@ -0,0 +1,102 @@
		import fp from 'fastify-plugin'
		import oauthPlugin, { } from '@fastify/oauth2'

	const url = `https://api.intra.42.fr/oauth/authorize?client_id=u-s4t2ud-164069d052052d412516d442f502a23c7662d19adb38400ed23e2f5a841eac37&redirect_uri=https%3A%2F%2Fmatcha.fr%2Fapi%2Fauth%2Flogin%2F42%2Fcallback%2F&response_type=code`;
	const redirectUri = 'https://matcha.fr/api/auth/login/42/callback/';
	const url = `https://api.intra.42.fr/oauth/authorize?client_id=${CLIENT_ID}&redirect_uri=${encodeURIComponent(redirectUri)}&response_type=code`;

-            await this.userService.verifyEmail(Number(existigUser.id)!, undefined);
+            if (!existigUser) {
+                return reply.status(500).send({ error: 'Failed to retrieve newly created user' });
+            }
+            await this.userService.verifyEmail(Number(existigUser.id), undefined);

+        if (!code || typeof code !== 'string' || code.trim() === '') {
+            return reply.status(400).send({ error: 'Missing or invalid authorization code' });
+        }

Conversation

YoungMame commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

YoungMame Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 26, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees