diff --git a/.changeset/fuzzy-cherries-buy.md b/.changeset/fuzzy-cherries-buy.md new file mode 100644 index 000000000000..e185a148c917 --- /dev/null +++ b/.changeset/fuzzy-cherries-buy.md @@ -0,0 +1,7 @@ +--- +"@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 deleted file mode 100644 index 4e054b81b2cf..000000000000 --- 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 000000000000..e2a6e0d10581 --- /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 2742cade15d2..4e99ac3a281b 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 3f0b148120e7..f51c2f383987 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 7e1019b60ecb..906b19d0a931 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', 'Incorrect password'); + expect(res.body).to.have.property('message', 'Unauthorized'); }); }