Skip to content

refactor: replace console.log with logger.debug in OAuth2 server #41563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rajkaran-122 wants to merge 1 commit into
base: develop
Choose a base branch
from
+18 −17
Open

refactor: replace console.log with logger.debug in OAuth2 server #41563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions apps/meteor/server/oauth2-server/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Logger } from '@rocket.chat/logger';

import type {
AuthorizationCode,
AuthorizationCodeModel,
Expand All @@ -14,7 +16,9 @@ export type ModelConfig = {
debug?: boolean;
};

export class Model implements AuthorizationCodeModel, RefreshTokenModel {
export const logger = new Logger('OAuth2Server');

class Model implements AuthorizationCodeModel, RefreshTokenModel {
private debug: boolean;

private grants = ['authorization_code', 'refresh_token'];
Expand All @@ -25,7 +29,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async verifyScope(token: Token, scope: string | string[]): Promise<boolean> {
if (this.debug === true) {
console.log('[OAuth2Server]', 'in grantTypeAllowed (clientId:', token.client.id, ', grantType:', `${scope})`);
logger.debug('in grantTypeAllowed (clientId:', token.client.id, ', grantType:', `${scope})`);
}

if (!Array.isArray(scope)) {
Expand All @@ -38,7 +42,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async getAccessToken(accessToken: string): Promise<Token | Falsey> {
if (this.debug === true) {
console.log('[OAuth2Server]', 'in getAccessToken (bearerToken:', accessToken, ')');
logger.debug('in getAccessToken (bearerToken:', accessToken, ')');
Comment on lines 43 to +45

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Prevent OAuth secrets from reaching centralized logs.

The model logs tokens, authorization codes, client secrets, and user data, while the middleware logs query strings that may contain the same credentials. Redact sensitive fields and log only request paths or safe metadata.

  • apps/meteor/server/oauth2-server/model.ts#L43-L45: remove or redact the access token.
  • apps/meteor/server/oauth2-server/model.ts#L76-L79: never log clientSecret.
  • apps/meteor/server/oauth2-server/model.ts#L101-L104: remove or redact the authorization code.
  • apps/meteor/server/oauth2-server/model.ts#L133-L148: avoid logging the full user object.
  • apps/meteor/server/oauth2-server/model.ts#L174-L189: remove access and refresh token values.
  • apps/meteor/server/oauth2-server/model.ts#L216-L219: remove or redact the refresh token.
  • apps/meteor/server/oauth2-server/model.ts#L259-L262: remove or redact the access token.
  • apps/meteor/server/oauth2-server/model.ts#L278-L281: remove or redact the authorization code.
  • apps/meteor/server/oauth2-server/oauth.ts#L49-L52: log the path instead of the full URL/query string.
📍 Affects 2 files
  • apps/meteor/server/oauth2-server/model.ts#L43-L45 (this comment)
  • apps/meteor/server/oauth2-server/model.ts#L76-L79
  • apps/meteor/server/oauth2-server/model.ts#L101-L104
  • apps/meteor/server/oauth2-server/model.ts#L133-L148
  • apps/meteor/server/oauth2-server/model.ts#L174-L189
  • apps/meteor/server/oauth2-server/model.ts#L216-L219
  • apps/meteor/server/oauth2-server/model.ts#L259-L262
  • apps/meteor/server/oauth2-server/model.ts#L278-L281
  • apps/meteor/server/oauth2-server/oauth.ts#L49-L52
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/meteor/server/oauth2-server/model.ts` around lines 43 - 45, Prevent
credential and personal-data exposure in OAuth logging: in
apps/meteor/server/oauth2-server/model.ts at lines 43-45, 76-79, 101-104,
133-148, 174-189, 216-219, 259-262, and 278-281, remove or redact access tokens,
refresh tokens, authorization codes, clientSecret, and full user objects,
retaining only safe metadata; in apps/meteor/server/oauth2-server/oauth.ts at
lines 49-52, log only the request path rather than the full URL or query string.
Update the relevant getAccessToken and adjacent OAuth model methods without
changing their behavior.

}

const token = await OAuthAccessTokens.findOneByAccessToken(accessToken);
Expand Down Expand Up @@ -71,7 +75,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async getClient(clientId: string, clientSecret?: string): Promise<Client | Falsey> {
if (this.debug === true) {
console.log('[OAuth2Server]', 'in getClient (clientId:', clientId, ', clientSecret:', clientSecret, ')');
logger.debug('in getClient (clientId:', clientId, ', clientSecret:', clientSecret, ')');
}

let client;
Expand All @@ -96,7 +100,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async getAuthorizationCode(authorizationCode: string): Promise<AuthorizationCode | Falsey> {
if (this.debug === true) {
console.log('[OAuth2Server]', `in getAuthorizationCode (authCode: ${authorizationCode})`);
logger.debug(`in getAuthorizationCode (authCode: ${authorizationCode})`);
}

const code = await OAuthAuthCodes.findOneByAuthCode(authorizationCode);
Expand Down Expand Up @@ -132,9 +136,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {
user: User,
): Promise<AuthorizationCode | Falsey> {
if (this.debug === true) {
console.log(
'[OAuth2Server]',
'in saveAuthCode (code:',
logger.debug('in saveAuthCode (code:',
code.authorizationCode,
', clientId:',
client.id,
Expand Down Expand Up @@ -171,9 +173,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async saveToken(token: Token, client: Client, user: User): Promise<Token | Falsey> {
if (this.debug === true) {
console.log(
'[OAuth2Server]',
'in saveToken (token:',
logger.debug('in saveToken (token:',
token.accessToken,
', refreshToken:',
token.refreshToken,
Expand Down Expand Up @@ -215,7 +215,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async getRefreshToken(refreshToken: string): Promise<RefreshToken | Falsey> {
if (this.debug === true) {
console.log('[OAuth2Server]', `in getRefreshToken (refreshToken: ${refreshToken})`);
logger.debug(`in getRefreshToken (refreshToken: ${refreshToken})`);
}

// Keep compatibility with old collection
Expand Down Expand Up @@ -258,7 +258,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async revokeToken(token: RefreshToken | Token): Promise<boolean> {
if (this.debug === true) {
console.log('[OAuth2Server]', `in revokeToken (token: ${token.accessToken})`);
logger.debug(`in revokeToken (token: ${token.accessToken})`);
}

if (token.refreshToken) {
Expand All @@ -277,7 +277,7 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {

async revokeAuthorizationCode(code: AuthorizationCode): Promise<boolean> {
if (this.debug === true) {
console.log('[OAuth2Server]', `in revokeAuthorizationCode (code: ${code.authorizationCode})`);
logger.debug(`in revokeAuthorizationCode (code: ${code.authorizationCode})`);
}
await OAuthAuthCodes.deleteOne({ authCode: code.authorizationCode });
return true;
Expand Down
7 changes: 4 additions & 3 deletions apps/meteor/server/oauth2-server/oauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@

import OAuthServer, { OAuthError, UnauthorizedRequestError } from '@node-oauth/oauth2-server';
import { OAuthApps, Users } from '@rocket.chat/models';
import express from 'express';
import type { Express, NextFunction, Request, Response } from 'express';
import { Accounts } from 'meteor/accounts-base';

import type { ModelConfig } from './model';
import { Model } from './model';
import { Model, logger } from './model';

export class OAuth2Server {
public app: Express;
Expand Down Expand Up @@ -47,7 +48,7 @@ export class OAuth2Server {

const debugMiddleware = function (req: Request, _res: Response, next: NextFunction) {
if (config.debug === true) {
console.log('[OAuth2Server]', req.method, req.url);
logger.debug(req.method, req.url);

@cubic-dev-ai cubic-dev-ai Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Logging req.url can expose OAuth credentials (authorization codes, tokens) that appear in query strings. Consider logging only req.path or stripping query parameters to avoid leaking sensitive values to centralized log storage.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/server/oauth2-server/oauth.ts, line 51:

<comment>Logging `req.url` can expose OAuth credentials (authorization codes, tokens) that appear in query strings. Consider logging only `req.path` or stripping query parameters to avoid leaking sensitive values to centralized log storage.</comment>

<file context>
@@ -47,7 +48,7 @@ export class OAuth2Server {
 		const debugMiddleware = function (req: Request, _res: Response, next: NextFunction) {
 			if (config.debug === true) {
-				console.log('[OAuth2Server]', req.method, req.url);
+				logger.debug(req.method, req.url);
 			}
 			return next();
</file context>
Suggested change
logger.debug(req.method, req.url);
logger.debug(req.method, req.path);

}
return next();
};
Expand All @@ -71,7 +72,7 @@ export class OAuth2Server {
const transformRequestsNotUsingFormUrlencodedType = function (req: Request, _res: Response, next: NextFunction) {
if (!req.is('application/x-www-form-urlencoded') && req.method === 'POST') {
if (config.debug === true) {
console.log('[OAuth2Server]', 'Transforming a request to form-urlencoded with the query going to the body.');
logger.debug('Transforming a request to form-urlencoded with the query going to the body.');
}
req.headers['content-type'] = 'application/x-www-form-urlencoded';
req.body = Object.assign({}, req.body, req.query);
Expand Down