Skip to content

Conversation

@Peu77
Copy link
Member

@Peu77 Peu77 commented Nov 16, 2025

No description provided.

@Peu77 Peu77 linked an issue Nov 16, 2025 that may be closed by this pull request
@Peu77 Peu77 requested a review from PaulicStudios November 16, 2025 20:23
@PaulicStudios
Copy link
Member

frontend build failed...

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates authentication from frontend-based NextAuth with GitHub OAuth to a backend-managed JWT authentication system using NestJS and Passport strategies.

Key Changes:

  • Implemented JWT-based authentication on the backend with GitHub and 42 School OAuth strategies
  • Replaced custom UserGuard with standard JwtAuthGuard across all protected endpoints
  • Updated frontend to consume backend authentication APIs and handle SSO redirects

Reviewed Changes

Copilot reviewed 36 out of 39 changed files in this pull request and generated 20 comments.

Show a summary per file
File Description
frontend/types/next-auth.d.ts Changed user type from image to profilePicture field
frontend/layouts/basic-navbar.tsx Updated to use profilePicture instead of image
frontend/hooks/use42Linking.ts Refactored to call backend API for 42 auth URL generation
frontend/components/github.tsx Changed to redirect to backend GitHub OAuth endpoint
frontend/app/utils/authOptions.ts Replaced GitHub provider with backend credentials provider using JWT
frontend/app/actions/axios.ts Updated interceptor to extract and send JWT token from cookies
frontend/app/auth/sso/page.tsx New SSO callback page to finalize authentication
api/src/auth/* New auth module with JWT strategy, GitHub/42 OAuth strategies, and controller
api/src/guards/UserGuard.ts Removed custom auth guard, replaced with JWT-based authentication
api/src/user/user.controller.ts Removed user creation/update endpoints, replaced guards with JwtAuthGuard
api/src/main.ts Updated CORS configuration for credential support
frontend/.env.example Removed GitHub/42 OAuth client credentials from frontend
api/.env.example Added OAuth credentials and JWT configuration to backend
Files not reviewed (1)
  • api/pnpm-lock.yaml: Language not supported

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

Peu77 and others added 4 commits November 16, 2025 22:49
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@PaulicStudios PaulicStudios left a comment

Choose a reason for hiding this comment

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

do it with better auth

Copy link
Member

@PaulicStudios PaulicStudios left a comment

Choose a reason for hiding this comment

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

other than the comments it is fine for now

) {}

@UseGuards(UserGuard)
@UseGuards(JwtAuthGuard)
Copy link
Member

Choose a reason for hiding this comment

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

Can we not reverse this logic?
So that we can whitelist requests that everyone should be able to access?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really know what you mean with "reverse this logic",
but we don't need to whitelist requests, so that everyone has access. We can just remove the UseGuards decorator. Except where we need the user id

Copy link
Member

Choose a reason for hiding this comment

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

Would not be the best case that just every request has the JwtAuthGuard on it and we would only define an @ for requests that are not protected?

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually, most requests are protected, so you can simply add the decorator to the controller and it will apply to every request. This is an edge case where one request is not protected. We could also create a new controller for this request

}

@Get("/42/callback")
@UseGuards(AuthGuard("42"))
Copy link
Member

Choose a reason for hiding this comment

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

42 auth does not work. Get the following error message:

[Nest] 98853  - 01/11/2026, 2:52:18 PM   ERROR [ExceptionsHandler] TokenError: Client authentication failed due to unknown client, no client authentication included, or unsupported authentication method.
    at OAuth2Strategy.parseErrorResponse (/Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/passport-oauth2@1.8.0/node_modules/passport-oauth2/lib/strategy.js:373:12)
    at OAuth2Strategy._createOAuthError (/Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/passport-oauth2@1.8.0/node_modules/passport-oauth2/lib/strategy.js:420:16)
    at /Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/passport-oauth2@1.8.0/node_modules/passport-oauth2/lib/strategy.js:177:45
    at /Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/oauth@0.10.2/node_modules/oauth/lib/oauth2.js:196:18
    at passBackControl (/Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/oauth@0.10.2/node_modules/oauth/lib/oauth2.js:132:9)
    at IncomingMessage.<anonymous> (/Users/paul/Documents/core/website_relaunch/api/node_modules/.pnpm/oauth@0.10.2/node_modules/oauth/lib/oauth2.js:157:7)
    at IncomingMessage.emit (node:events:520:35)
    at endReadableNT (node:internal/streams/readable:1701:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
  code: 'invalid_client',
  uri: undefined,
  status: 500

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like a you problem, are you sure you have the right FORTYTWO_CLIENT_ID and FORTYTWO_CLIENT_SECRET?

Copy link
Member

Choose a reason for hiding this comment

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

I can try again. But the error does not seem like the secret is wrong

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure it is an credentials error

Copy link
Member

@PaulicStudios PaulicStudios left a comment

Choose a reason for hiding this comment

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

And we also need to change the envs in the frontend deployment pipelines that are not necessary anymore and add the ones in the helm values config

@PaulicStudios PaulicStudios merged commit 5b05e1e into dev Jan 26, 2026
4 checks passed
@PaulicStudios PaulicStudios deleted the 390-migrate-authentication-to-the-backend branch January 26, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

migrate authentication to the backend

3 participants