diff --git a/api/server/middleware/checkBan.js b/api/server/middleware/checkBan.js index 0e686fdadb..7194a555b0 100644 --- a/api/server/middleware/checkBan.js +++ b/api/server/middleware/checkBan.js @@ -1,9 +1,10 @@ const { Keyv } = require('keyv'); const uap = require('ua-parser-js'); const { logger } = require('@librechat/data-schemas'); -const { ViolationTypes } = require('librechat-data-provider'); +const { ErrorTypes, ViolationTypes } = require('librechat-data-provider'); const { isEnabled, keyvMongo, removePorts } = require('@librechat/api'); const { getLogStores } = require('~/cache'); +const { isOAuthNavigation, redirectOAuthFailure } = require('./oauthNavigation'); const denyRequest = require('./denyRequest'); const { findUser } = require('~/models'); @@ -50,9 +51,13 @@ const isInteractiveAgentChatRequest = (req) => { * @param {Object} req - Express Request object. * @param {Object} res - Express Response object. * - * @returns {Promise} - Returns a Promise which sends a JSON 403 unless this is an interactive browser agent chat request, in which case it calls `denyRequest()`. + * @returns {Promise} - Returns a Promise which sends a JSON 403, unless this is an interactive browser agent chat request (`denyRequest()`) or an OAuth browser navigation (redirect to the login page). */ const banResponse = async (req, res) => { + if (isOAuthNavigation(req)) { + return redirectOAuthFailure(res, ErrorTypes.AUTH_BANNED); + } + const ua = uap(req.headers['user-agent']); if (!ua.browser.name) { return res.status(403).json({ message }); diff --git a/api/server/middleware/index.js b/api/server/middleware/index.js index 775d140e24..8f39ea1ee8 100644 --- a/api/server/middleware/index.js +++ b/api/server/middleware/index.js @@ -10,6 +10,7 @@ const { sendValidationResponse, } = require('./messageValidation'); const checkDomainAllowed = require('./checkDomainAllowed'); +const { markOAuthNavigation } = require('./oauthNavigation'); const requireLocalAuth = require('./requireLocalAuth'); const canDeleteAccount = require('./canDeleteAccount'); const accessResources = require('./accessResources'); @@ -41,6 +42,7 @@ module.exports = { uaParser, setHeaders, logHeaders, + markOAuthNavigation, moderateText, validateModel, requireJwtAuth, diff --git a/api/server/middleware/limiters/loginLimiter.js b/api/server/middleware/limiters/loginLimiter.js index c178b68a25..9e82f622ba 100644 --- a/api/server/middleware/limiters/loginLimiter.js +++ b/api/server/middleware/limiters/loginLimiter.js @@ -1,6 +1,7 @@ const rateLimit = require('express-rate-limit'); -const { ViolationTypes } = require('librechat-data-provider'); +const { ErrorTypes, ViolationTypes } = require('librechat-data-provider'); const { limiterCache, removePorts } = require('@librechat/api'); +const { isOAuthNavigation, redirectOAuthFailure } = require('~/server/middleware/oauthNavigation'); const { logViolation } = require('~/cache'); const { LOGIN_WINDOW = 5, LOGIN_MAX = 7, LOGIN_VIOLATION_SCORE: score } = process.env; @@ -18,6 +19,12 @@ const handler = async (req, res) => { }; await logViolation(req, res, type, errorMessage, score); + + /** A browser navigation would render the JSON body as the page. */ + if (isOAuthNavigation(req)) { + return redirectOAuthFailure(res, ErrorTypes.AUTH_RATE_LIMITED); + } + return res.status(429).json({ message }); }; diff --git a/api/server/middleware/limiters/loginLimiter.spec.js b/api/server/middleware/limiters/loginLimiter.spec.js new file mode 100644 index 0000000000..82e08ed036 --- /dev/null +++ b/api/server/middleware/limiters/loginLimiter.spec.js @@ -0,0 +1,92 @@ +const express = require('express'); +const request = require('supertest'); + +const originalEnv = process.env; + +const createApp = () => { + jest.resetModules(); + process.env = { + ...originalEnv, + DOMAIN_CLIENT: 'http://client.test', + LOGIN_MAX: '1', + LOGIN_WINDOW: '5', + }; + + jest.doMock('@librechat/api', () => ({ + ...jest.requireActual('@librechat/api'), + limiterCache: jest.fn(() => undefined), + removePorts: (req) => req?.['ip'], + })); + jest.doMock('~/cache', () => ({ + logViolation: jest.fn().mockResolvedValue(undefined), + })); + + const { markOAuthNavigation } = require('../oauthNavigation'); + const loginLimiter = require('./loginLimiter'); + const { logViolation } = require('~/cache'); + + const app = express(); + app.set('trust proxy', 1); + app.use(express.json()); + app.post('/api/auth/login', loginLimiter, (_req, res) => res.status(204).end()); + app.use('/oauth', markOAuthNavigation, loginLimiter); + app.get('/oauth/openid', (_req, res) => res.status(204).end()); + + return { app, logViolation }; +}; + +describe('loginLimiter', () => { + afterEach(() => { + jest.dontMock('@librechat/api'); + jest.dontMock('~/cache'); + process.env = originalEnv; + }); + + it('answers API login requests with JSON', async () => { + const { app, logViolation } = createApp(); + + await request(app).post('/api/auth/login').set('x-forwarded-for', '198.51.100.4').expect(204); + + const response = await request(app) + .post('/api/auth/login') + .set('x-forwarded-for', '198.51.100.4') + .expect(429); + + expect(response.body).toEqual({ + message: 'Too many login attempts, please try again after 5 minutes.', + }); + expect(logViolation).toHaveBeenCalledTimes(1); + }); + + it('redirects a rate limited OAuth navigation to the login page', async () => { + const { app, logViolation } = createApp(); + + await request(app).get('/oauth/openid').set('x-forwarded-for', '203.0.113.7').expect(204); + + const response = await request(app) + .get('/oauth/openid') + .set('x-forwarded-for', '203.0.113.7') + .expect(302); + + expect(response.headers.location).toBe( + 'http://client.test/login?redirect=false&error=auth_rate_limited', + ); + expect(response.text).not.toContain('Too many login attempts'); + expect(logViolation).toHaveBeenCalledTimes(1); + }); + + it('shares one attempt budget between the API login and OAuth routes', async () => { + const { app } = createApp(); + + await request(app).post('/api/auth/login').set('x-forwarded-for', '203.0.113.9').expect(204); + + const response = await request(app) + .get('/oauth/openid') + .set('x-forwarded-for', '203.0.113.9') + .expect(302); + + expect(response.headers.location).toBe( + 'http://client.test/login?redirect=false&error=auth_rate_limited', + ); + }); +}); diff --git a/api/server/middleware/oauthNavigation.js b/api/server/middleware/oauthNavigation.js new file mode 100644 index 0000000000..8aa59e0377 --- /dev/null +++ b/api/server/middleware/oauthNavigation.js @@ -0,0 +1,36 @@ +const { redirectToAuthFailure } = require('@librechat/api'); + +/** + * Flags the request as a top-level browser navigation belonging to the OAuth flow. + * + * These routes are reached by the address bar (social button click, IdP redirect, SAML/Apple + * form post), never by fetch, so any middleware that rejects them with a JSON body renders that + * body as the page. Rejections must redirect back to the login page instead. + * + * @type {import('express').RequestHandler} + */ +const markOAuthNavigation = (req, _res, next) => { + req.isOAuthNavigation = true; + next(); +}; + +/** + * @param {import('express').Request} req + * @returns {boolean} Whether the request is a top-level browser navigation in the OAuth flow. + */ +const isOAuthNavigation = (req) => req?.isOAuthNavigation === true; + +/** + * Sends the browser back to the login page carrying an error code the client localizes. + * + * @param {import('express').Response} res + * @param {string} authFailedError - An `ErrorTypes` value. + */ +const redirectOAuthFailure = (res, authFailedError) => + redirectToAuthFailure(res, { clientDomain: process.env.DOMAIN_CLIENT, authFailedError }); + +module.exports = { + markOAuthNavigation, + isOAuthNavigation, + redirectOAuthFailure, +}; diff --git a/api/server/routes/oauth.js b/api/server/routes/oauth.js index d5e4c939ea..2d87bb43a0 100644 --- a/api/server/routes/oauth.js +++ b/api/server/routes/oauth.js @@ -11,7 +11,12 @@ const { getOAuthFailureMessage, redirectToAuthFailure, } = require('@librechat/api'); -const { checkDomainAllowed, loginLimiter, logHeaders } = require('~/server/middleware'); +const { + checkDomainAllowed, + loginLimiter, + logHeaders, + markOAuthNavigation, +} = require('~/server/middleware'); const { createOAuthHandler } = require('~/server/controllers/auth/oauth'); const { findBalanceByUser, upsertBalanceFields } = require('~/models'); const { getAppConfig } = require('~/server/services/Config'); @@ -35,6 +40,7 @@ const authFailureRedirectOptions = { }; router.use(logHeaders); +router.use(markOAuthNavigation); router.use(loginLimiter); const oauthHandler = createOAuthHandler(); diff --git a/api/server/routes/oauth.test.js b/api/server/routes/oauth.test.js index b6739ffdc5..9acf4d9278 100644 --- a/api/server/routes/oauth.test.js +++ b/api/server/routes/oauth.test.js @@ -77,10 +77,16 @@ jest.mock('@librechat/api', () => ({ redirectToAuthFailure: (...args) => mockRedirectToAuthFailure(...args), })); +const mockMarkOAuthNavigation = jest.fn((req, _res, next) => { + req.isOAuthNavigation = true; + next(); +}); + jest.mock('~/server/middleware', () => ({ checkDomainAllowed: jest.fn((_req, _res, next) => next()), loginLimiter: jest.fn((_req, _res, next) => next()), logHeaders: jest.fn((_req, _res, next) => next()), + markOAuthNavigation: (...args) => mockMarkOAuthNavigation(...args), })); jest.mock('~/server/controllers/auth/oauth', () => ({ @@ -136,11 +142,21 @@ describe('OAuth route failure logging', () => { mockGetOAuthFailureMessage.mockClear(); mockRedirectToAuthFailure.mockClear(); mockPassportAuthenticate.mockClear(); + mockMarkOAuthNavigation.mockClear(); mockOpenIDCallbackAuthenticatorOptions = undefined; mockPassportAuthenticate.mockImplementation(() => (_req, _res, next) => next()); mockOpenIDCallbackMiddleware.mockImplementation((_req, _res, next) => next()); }); + it('marks OAuth requests as browser navigations so rejections can redirect', async () => { + const app = createApp(); + + await request(app).get('/oauth/openid/callback?code=secret-code').expect(204); + + expect(mockMarkOAuthNavigation).toHaveBeenCalled(); + expect(mockOAuthHandler.mock.calls[0][0].isOAuthNavigation).toBe(true); + }); + it('wires the package OpenID callback middleware into the route', async () => { const app = createApp(); diff --git a/api/test/server/middleware/checkBan.test.js b/api/test/server/middleware/checkBan.test.js index 569fabdc71..237282d9d0 100644 --- a/api/test/server/middleware/checkBan.test.js +++ b/api/test/server/middleware/checkBan.test.js @@ -41,6 +41,8 @@ jest.mock('@librechat/api', () => ({ }, keyvMongo: {}, removePorts: jest.fn((req) => req.ip), + redirectToAuthFailure: (res, { clientDomain, authFailedError }) => + res.redirect(`${clientDomain}/login?redirect=false&error=${authFailedError}`), })); jest.mock('~/models', () => ({ @@ -72,6 +74,7 @@ const createReq = (overrides = {}) => ({ const createRes = () => ({ status: jest.fn().mockReturnThis(), json: jest.fn().mockReturnThis(), + redirect: jest.fn().mockReturnThis(), }); describe('checkBan middleware', () => { @@ -152,6 +155,26 @@ describe('checkBan middleware', () => { expect(res.status).toHaveBeenCalledWith(403); }); + it('redirects instead of sending JSON when the ban hits an OAuth navigation', async () => { + process.env.DOMAIN_CLIENT = 'http://client.test'; + mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 }); + const next = jest.fn(); + const req = createReq({ + isOAuthNavigation: true, + baseUrl: '/oauth', + originalUrl: '/oauth/openid/callback', + }); + const res = createRes(); + + await checkBan(req, res, next); + + expect(req.banned).toBe(true); + expect(res.json).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith( + 'http://client.test/login?redirect=false&error=auth_banned', + ); + }); + it('returns 403 when user ban is cached (IP miss)', async () => { mockBanCacheGet .mockResolvedValueOnce(undefined) diff --git a/client/src/components/Auth/Login.tsx b/client/src/components/Auth/Login.tsx index ef9e595509..30dd425bbc 100644 --- a/client/src/components/Auth/Login.tsx +++ b/client/src/components/Auth/Login.tsx @@ -3,6 +3,7 @@ import { OpenIDIcon, useToastContext } from '@librechat/client'; import { ErrorTypes, registerPage } from 'librechat-data-provider'; import { useOutletContext, useSearchParams, useLocation } from 'react-router-dom'; import type { TLoginLayoutContext } from '~/common'; +import type { TranslationKeys } from '~/hooks'; import { getLoginError, persistRedirectToSession } from '~/utils'; import { ErrorMessage } from '~/components/Auth/ErrorMessage'; import SocialButton from '~/components/Auth/SocialButton'; @@ -14,6 +15,13 @@ interface LoginLocationState { redirect_to?: string; } +/** Error codes the server appends to the login redirect when an OAuth navigation is rejected. */ +const oauthErrorKeys: Record = { + [ErrorTypes.AUTH_FAILED]: 'com_auth_error_oauth_failed', + [ErrorTypes.AUTH_RATE_LIMITED]: 'com_auth_error_login_rl', + [ErrorTypes.AUTH_BANNED]: 'com_auth_error_login_ban', +}; + function Login() { const localize = useLocalize(); const { showToast } = useToastContext(); @@ -38,9 +46,14 @@ function Login() { } const oauthError = searchParams?.get('error'); - if (oauthError && oauthError === ErrorTypes.AUTH_FAILED) { + /** `hasOwn` keeps an attacker-supplied `?error=toString` off the prototype chain. */ + const oauthErrorKey = + oauthError != null && Object.hasOwn(oauthErrorKeys, oauthError) + ? oauthErrorKeys[oauthError] + : undefined; + if (oauthErrorKey != null) { showToast({ - message: localize('com_auth_error_oauth_failed'), + message: localize(oauthErrorKey), status: 'error', }); const newParams = new URLSearchParams(searchParams); diff --git a/client/src/components/Auth/__tests__/Login.spec.tsx b/client/src/components/Auth/__tests__/Login.spec.tsx index 6f98fec8ca..a89e433716 100644 --- a/client/src/components/Auth/__tests__/Login.spec.tsx +++ b/client/src/components/Auth/__tests__/Login.spec.tsx @@ -11,6 +11,13 @@ import Login from '~/components/Auth/Login'; jest.mock('librechat-data-provider/react-query'); +const mockShowToast = jest.fn(); + +jest.mock('@librechat/client', () => ({ + ...jest.requireActual('@librechat/client'), + useToastContext: () => ({ showToast: mockShowToast }), +})); + const mockStartupConfig = { isFetching: false, isLoading: false, @@ -205,3 +212,43 @@ test('Navigates to / on successful login', async () => { waitFor(() => expect(window.location.pathname).toBe('/')); }); + +describe('OAuth rejection redirects', () => { + const enterAt = (search: string) => { + window.history.pushState({}, '', `/login${search}`); + }; + + afterEach(() => { + mockShowToast.mockClear(); + window.history.pushState({}, '', '/login'); + }); + + test.each([ + [ + 'auth_rate_limited', + 'Too many login attempts in a short amount of time. Please try again later.', + ], + ['auth_banned', 'Your account has been temporarily banned due to violations of our service.'], + ['auth_failed', 'Authentication failed. Please check your login method and try again.'], + ])('surfaces a localized toast for %s and clears the query', async (code, message) => { + enterAt(`?redirect=false&error=${code}`); + + setup(); + + await waitFor(() => expect(mockShowToast).toHaveBeenCalledWith({ message, status: 'error' })); + await waitFor(() => expect(window.location.search).not.toContain('error=')); + expect(window.location.search).not.toContain('redirect='); + }); + + test.each(['toString', 'constructor', 'unmapped_code'])( + 'ignores the unmapped error code %s', + async (code) => { + enterAt(`?error=${code}`); + + setup(); + + await waitFor(() => expect(getByTestId(document.body, 'login-button')).toBeInTheDocument()); + expect(mockShowToast).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 8f4bab1609..6549df3046 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -3082,6 +3082,14 @@ export enum ErrorTypes { * Generic Authentication failure */ AUTH_FAILED = 'auth_failed', + /** + * Authentication rejected by a rate limiter + */ + AUTH_RATE_LIMITED = 'auth_rate_limited', + /** + * Authentication rejected because the account or IP is banned + */ + AUTH_BANNED = 'auth_banned', /** * Model refused to respond (content policy violation) */