poc: Passport OAuth#38592
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| app.get('/auth/facebook/callback', passport.authenticate('facebook', { failureRedirect: '/login' }), async (req, res) => { | ||
| console.log('facebook/callback', req.user); | ||
|
|
||
| const oAuthUser = req.user; | ||
| console.log('oAuthUser', oAuthUser); | ||
|
|
||
| const user = await Accounts.updateOrCreateUserFromExternalService('facebook', { ...oAuthUser, id: oAuthUser.providerId }); | ||
| console.log('user', user); | ||
|
|
||
| if (!user?.userId) { | ||
| return res.redirect('/login'); | ||
| } | ||
|
|
||
| const userFromDB = await Users.findOneById(user?.userId as string); | ||
| console.log('userFromDB', userFromDB); | ||
|
|
||
| const stampedToken = Accounts._generateStampedLoginToken(); | ||
| Accounts._insertLoginToken(userFromDB?._id as string, stampedToken); | ||
|
|
||
| res.redirect(`http://localhost:3000/home?resumeToken=${stampedToken.token}`); | ||
|
|
||
| req.session.destroy((err) => { | ||
| if (err) { | ||
| console.error('Error destroying session', err); | ||
| } | ||
| }); | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the problem should be fixed by adding a rate-limiting middleware in front of HTTP routes that perform authentication/authorization or other expensive operations. In Express, this is typically achieved by using a library like express-rate-limit and applying it either globally or to specific routes. For OAuth callbacks, per-route limiting is often preferred to avoid impacting unrelated endpoints.
For this file, the minimal, non-breaking fix is to introduce express-rate-limit, define a conservative limiter (for example, allowing a small number of login attempts per IP over a time window), and apply it to both /auth/facebook/callback and /auth/github/callback. We should not change the semantics of the existing handlers: they must still authenticate via Passport and then perform the current Meteor Account operations. The rate limiter should be inserted into the middleware chain between app.get(path, ...) and the final handler, or as an additional middleware parameter to app.get. Because imports already use ES module style (import express from 'express';), we will import the limiter likewise. We will define a shared limiter constant near where the app is created, and then attach it to both callback routes. This keeps the change localized to apps/meteor/server/passport.ts and does not alter existing business logic.
Concretely:
- Add an import for
express-rate-limitat the top ofapps/meteor/server/passport.ts. - Define a
loginLimiter(or similar) afterconst app = express();and before the route definitions, with a reasonablewindowMsandmaxvalues. - Modify:
app.get('/auth/facebook/callback', passport.authenticate(...), async (req, res) => { ... });app.get('/auth/github/callback', passport.authenticate(...), async (req, res) => { ... });
so that each includesloginLimiteras an additional middleware in the chain (e.g.,app.get('/auth/facebook/callback', loginLimiter, passport.authenticate(...), async ...)).
| @@ -6,6 +6,7 @@ | ||
| import passport from 'passport'; | ||
| import { Strategy as FacebookStrategy } from 'passport-facebook'; | ||
| import { Strategy as GitHubStrategy } from 'passport-github2'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| passport.use( | ||
| new FacebookStrategy( | ||
| @@ -87,6 +88,12 @@ | ||
| }); | ||
|
|
||
| const app = express(); | ||
|
|
||
| const loginLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 login requests per windowMs | ||
| }); | ||
|
|
||
| app.use( | ||
| session({ | ||
| name: 'oauth.tmp', | ||
| @@ -106,7 +113,7 @@ | ||
|
|
||
| app.get('/auth/facebook', passport.authenticate('facebook', { scope: ['email'], failureRedirect: '/login', failureFlash: true })); | ||
|
|
||
| app.get('/auth/facebook/callback', passport.authenticate('facebook', { failureRedirect: '/login' }), async (req, res) => { | ||
| app.get('/auth/facebook/callback', loginLimiter, passport.authenticate('facebook', { failureRedirect: '/login' }), async (req, res) => { | ||
| console.log('facebook/callback', req.user); | ||
|
|
||
| const oAuthUser = req.user; | ||
| @@ -136,7 +143,7 @@ | ||
|
|
||
| app.get('/auth/github', passport.authenticate('github', { scope: ['user:email'] })); | ||
|
|
||
| app.get('/auth/github/callback', passport.authenticate('github', { failureRedirect: '/login' }), async (req, res) => { | ||
| app.get('/auth/github/callback', loginLimiter, passport.authenticate('github', { failureRedirect: '/login' }), async (req, res) => { | ||
| console.log('github/callback', req.user); | ||
|
|
||
| const oAuthUser = req.user; |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38592 +/- ##
===========================================
+ Coverage 70.39% 70.50% +0.10%
===========================================
Files 3161 3163 +2
Lines 110654 110931 +277
Branches 19892 19988 +96
===========================================
+ Hits 77895 78207 +312
+ Misses 30731 30698 -33
+ Partials 2028 2026 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/12 22:57", "02/13 22:38", "02/16 14:04", "02/16 17:27 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 16 Feb 2026 17:27:03 GMT |
3 participants
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments