From 46c757a25e32d0f8309938eededf6b9c3a868b73 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 9 Apr 2024 13:40:42 -0300 Subject: [PATCH] Revert "fix!: api login should not suggest which credential is wrong" (#32156) This reverts commit 65324bc6ba3de9f7e60c49c72d6da07a456bf2fa. --- .changeset/fuzzy-cherries-buy.md | 7 ------- .../lib/server/lib/loginErrorMessageOverride.js | 14 ++++++++++++++ .../lib/server/lib/loginErrorMessageOverride.ts | 16 ---------------- .../client/meteorOverrides/login/google.ts | 10 ++++++++++ .../externals/meteor/accounts-base.d.ts | 10 +--------- .../end-to-end/api/31-failed-login-attempts.ts | 2 +- 6 files changed, 26 insertions(+), 33 deletions(-) delete mode 100644 .changeset/fuzzy-cherries-buy.md create mode 100644 apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js delete mode 100644 apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts diff --git a/.changeset/fuzzy-cherries-buy.md b/.changeset/fuzzy-cherries-buy.md deleted file mode 100644 index e185a148c917..000000000000 --- a/.changeset/fuzzy-cherries-buy.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"@rocket.chat/meteor": major ---- - -Api login should not suggest which credential is wrong (password/username) - -Failed login attemps will always return `Unauthorized` instead of the internal fail reason diff --git a/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js b/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js new file mode 100644 index 000000000000..4e054b81b2cf --- /dev/null +++ b/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js @@ -0,0 +1,14 @@ +// Do not disclose if user exists when password is invalid +import { Accounts } from 'meteor/accounts-base'; +import { Meteor } from 'meteor/meteor'; + +const { _runLoginHandlers } = Accounts; +Accounts._runLoginHandlers = function (methodInvocation, options) { + const result = _runLoginHandlers.call(Accounts, methodInvocation, options); + + if (result.error && result.error.reason === 'Incorrect password') { + result.error = new Meteor.Error(403, 'User not found'); + } + + return result; +}; diff --git a/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts b/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts deleted file mode 100644 index e2a6e0d10581..000000000000 --- a/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts +++ /dev/null @@ -1,16 +0,0 @@ -// Do not disclose if user exists when password is invalid -import { Accounts } from 'meteor/accounts-base'; -import { Meteor } from 'meteor/meteor'; - -const { _runLoginHandlers } = Accounts; - -Accounts._options.ambiguousErrorMessages = true; -Accounts._runLoginHandlers = async function (methodInvocation, options) { - const result = await _runLoginHandlers.call(Accounts, methodInvocation, options); - - if (result.error instanceof Meteor.Error) { - result.error = new Meteor.Error(401, 'User not found'); - } - - return result; -}; diff --git a/apps/meteor/client/meteorOverrides/login/google.ts b/apps/meteor/client/meteorOverrides/login/google.ts index 4e99ac3a281b..2742cade15d2 100644 --- a/apps/meteor/client/meteorOverrides/login/google.ts +++ b/apps/meteor/client/meteorOverrides/login/google.ts @@ -8,6 +8,16 @@ import { overrideLoginMethod, type LoginCallback } from '../../lib/2fa/overrideL import { wrapRequestCredentialFn } from '../../lib/wrapRequestCredentialFn'; import { createOAuthTotpLoginMethod } from './oauth'; +declare module 'meteor/accounts-base' { + // eslint-disable-next-line @typescript-eslint/no-namespace + namespace Accounts { + export const _options: { + restrictCreationByEmailDomain?: string | (() => string); + forbidClientAccountCreation?: boolean | undefined; + }; + } +} + declare module 'meteor/meteor' { // eslint-disable-next-line @typescript-eslint/no-namespace namespace Meteor { diff --git a/apps/meteor/definition/externals/meteor/accounts-base.d.ts b/apps/meteor/definition/externals/meteor/accounts-base.d.ts index f51c2f383987..3f0b148120e7 100644 --- a/apps/meteor/definition/externals/meteor/accounts-base.d.ts +++ b/apps/meteor/definition/externals/meteor/accounts-base.d.ts @@ -22,7 +22,7 @@ declare module 'meteor/accounts-base' { function _insertLoginToken(userId: string, token: { token: string; when: Date }): void; - function _runLoginHandlers(methodInvocation: T, loginRequest: Record): Promise; + function _runLoginHandlers(methodInvocation: T, loginRequest: Record): LoginMethodResult | undefined; function registerLoginHandler(name: string, handler: (options: any) => undefined | object): void; @@ -54,14 +54,6 @@ declare module 'meteor/accounts-base' { const _accountData: Record; - interface AccountsServerOptions { - ambiguousErrorMessages?: boolean; - restrictCreationByEmailDomain?: string | (() => string); - forbidClientAccountCreation?: boolean | undefined; - } - - export const _options: AccountsServerOptions; - // eslint-disable-next-line @typescript-eslint/no-namespace namespace oauth { function credentialRequestCompleteHandler( diff --git a/apps/meteor/tests/end-to-end/api/31-failed-login-attempts.ts b/apps/meteor/tests/end-to-end/api/31-failed-login-attempts.ts index 906b19d0a931..7e1019b60ecb 100644 --- a/apps/meteor/tests/end-to-end/api/31-failed-login-attempts.ts +++ b/apps/meteor/tests/end-to-end/api/31-failed-login-attempts.ts @@ -54,7 +54,7 @@ describe('[Failed Login Attempts]', function () { .expect(401) .expect((res) => { expect(res.body).to.have.property('status', 'error'); - expect(res.body).to.have.property('message', 'Unauthorized'); + expect(res.body).to.have.property('message', 'Incorrect password'); }); }