From 0b2276548de78d696ab75109b654e09576b5db8a Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Wed, 27 Mar 2024 13:56:39 -0300 Subject: [PATCH 1/3] fix: api login should not suggest which credential is wrong --- .../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 +- 5 files changed, 26 insertions(+), 26 deletions(-) delete mode 100644 apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js create mode 100644 apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts diff --git a/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js b/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js deleted file mode 100644 index 4e054b81b2cf6..0000000000000 --- a/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.js +++ /dev/null @@ -1,14 +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._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 new file mode 100644 index 0000000000000..e2a6e0d105812 --- /dev/null +++ b/apps/meteor/app/lib/server/lib/loginErrorMessageOverride.ts @@ -0,0 +1,16 @@ +// 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 2742cade15d2c..4e99ac3a281b5 100644 --- a/apps/meteor/client/meteorOverrides/login/google.ts +++ b/apps/meteor/client/meteorOverrides/login/google.ts @@ -8,16 +8,6 @@ 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 3f0b148120e71..f51c2f3839873 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): LoginMethodResult | undefined; + function _runLoginHandlers(methodInvocation: T, loginRequest: Record): Promise; function registerLoginHandler(name: string, handler: (options: any) => undefined | object): void; @@ -54,6 +54,14 @@ 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 92ea1ac56ed33..be124c440b97d 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 @@ -48,7 +48,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', 'Incorrect password'); + expect(res.body).to.have.property('message', 'Unauthorized'); }); } From 91506ba4392842a774f0a72aa20378c55a827206 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Wed, 27 Mar 2024 15:33:06 -0400 Subject: [PATCH 2/3] Create fuzzy-cherries-buy.md --- .changeset/fuzzy-cherries-buy.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fuzzy-cherries-buy.md diff --git a/.changeset/fuzzy-cherries-buy.md b/.changeset/fuzzy-cherries-buy.md new file mode 100644 index 0000000000000..2a1764f3aaf11 --- /dev/null +++ b/.changeset/fuzzy-cherries-buy.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Api login should not suggest which credential is wrong (password/username) From a236e2ae7e488d3ecf14550121e82582d9fb5c7a Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Wed, 27 Mar 2024 17:32:36 -0400 Subject: [PATCH 3/3] Update fuzzy-cherries-buy.md --- .changeset/fuzzy-cherries-buy.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/fuzzy-cherries-buy.md b/.changeset/fuzzy-cherries-buy.md index 2a1764f3aaf11..e185a148c9172 100644 --- a/.changeset/fuzzy-cherries-buy.md +++ b/.changeset/fuzzy-cherries-buy.md @@ -1,5 +1,7 @@ --- -"@rocket.chat/meteor": patch +"@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