From 9a6e9b4e280b6044cd49344921e426f31622faeb Mon Sep 17 00:00:00 2001 From: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com> Date: Tue, 9 Jan 2024 15:01:38 -0300 Subject: [PATCH] fix: login buttons remain visible until refresh after disabling authentication service (#31371) --- .changeset/little-planes-wonder.md | 7 +++ .../rocketchat-mongo-config/server/index.js | 4 +- .../modules/watchers/watchers.module.ts | 6 +++ apps/meteor/server/services/meteor/service.ts | 8 +-- .../tests/e2e/fixtures/inject-initial-data.ts | 4 ++ apps/meteor/tests/e2e/oauth.spec.ts | 26 ++++++++++ apps/meteor/tests/e2e/page-objects/auth.ts | 4 ++ .../tests/end-to-end/api/08-settings.js | 49 +++++++++++++++++++ ee/apps/ddp-streamer/src/DDPStreamer.ts | 4 +- packages/core-services/src/events/Events.ts | 15 +++++- 10 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 .changeset/little-planes-wonder.md create mode 100644 apps/meteor/tests/e2e/oauth.spec.ts diff --git a/.changeset/little-planes-wonder.md b/.changeset/little-planes-wonder.md new file mode 100644 index 00000000000..13c90d0efcd --- /dev/null +++ b/.changeset/little-planes-wonder.md @@ -0,0 +1,7 @@ +--- +'@rocket.chat/core-services': patch +'@rocket.chat/ddp-streamer': patch +'@rocket.chat/meteor': patch +--- + +Fixed an issue that caused login buttons to not be reactively removed from the login page when the related authentication service was disabled by an admin. diff --git a/apps/meteor/packages/rocketchat-mongo-config/server/index.js b/apps/meteor/packages/rocketchat-mongo-config/server/index.js index 65464a31095..684620d0905 100644 --- a/apps/meteor/packages/rocketchat-mongo-config/server/index.js +++ b/apps/meteor/packages/rocketchat-mongo-config/server/index.js @@ -4,8 +4,8 @@ import { PassThrough } from 'stream'; import { Email } from 'meteor/email'; import { Mongo } from 'meteor/mongo'; -const shouldDisableOplog = ['yes', 'true'].includes(String(process.env.USE_NATIVE_OPLOG).toLowerCase()); -if (!shouldDisableOplog) { +const shouldUseNativeOplog = ['yes', 'true'].includes(String(process.env.USE_NATIVE_OPLOG).toLowerCase()); +if (!shouldUseNativeOplog) { Package['disable-oplog'] = {}; } diff --git a/apps/meteor/server/modules/watchers/watchers.module.ts b/apps/meteor/server/modules/watchers/watchers.module.ts index 88e465edc01..efa0866ab4a 100644 --- a/apps/meteor/server/modules/watchers/watchers.module.ts +++ b/apps/meteor/server/modules/watchers/watchers.module.ts @@ -339,7 +339,13 @@ export function initWatchers(watcher: DatabaseWatcher, broadcast: BroadcastCallb }); watcher.on(LoginServiceConfiguration.getCollectionName(), async ({ clientAction, id }) => { + if (clientAction === 'removed') { + void broadcast('watch.loginServiceConfiguration', { clientAction, id }); + return; + } + const data = await LoginServiceConfiguration.findOne>(id, { projection: { secret: 0 } }); + if (!data) { return; } diff --git a/apps/meteor/server/services/meteor/service.ts b/apps/meteor/server/services/meteor/service.ts index 8b9462740c6..95d2061e2f6 100644 --- a/apps/meteor/server/services/meteor/service.ts +++ b/apps/meteor/server/services/meteor/service.ts @@ -152,9 +152,11 @@ export class MeteorService extends ServiceClassInternal implements IMeteor { return; } - serviceConfigCallbacks.forEach((callbacks) => { - callbacks[clientAction === 'inserted' ? 'added' : 'changed']?.(id, data); - }); + if (data) { + serviceConfigCallbacks.forEach((callbacks) => { + callbacks[clientAction === 'inserted' ? 'added' : 'changed']?.(id, data); + }); + } }); } diff --git a/apps/meteor/tests/e2e/fixtures/inject-initial-data.ts b/apps/meteor/tests/e2e/fixtures/inject-initial-data.ts index 11cea78b3f3..38835db4aaa 100644 --- a/apps/meteor/tests/e2e/fixtures/inject-initial-data.ts +++ b/apps/meteor/tests/e2e/fixtures/inject-initial-data.ts @@ -57,6 +57,10 @@ export default async function injectInitialData() { _id: 'API_Enable_Rate_Limiter_Dev', value: false, }, + { + _id: 'Accounts_OAuth_Google', + value: false, + }, ].map((setting) => connection .db() diff --git a/apps/meteor/tests/e2e/oauth.spec.ts b/apps/meteor/tests/e2e/oauth.spec.ts new file mode 100644 index 00000000000..e8ad6a6c7e5 --- /dev/null +++ b/apps/meteor/tests/e2e/oauth.spec.ts @@ -0,0 +1,26 @@ +import { Registration } from './page-objects'; +import { setSettingValueById } from './utils/setSettingValueById'; +import { test, expect } from './utils/test'; + +test.describe('OAuth', () => { + let poRegistration: Registration; + + test.beforeEach(async ({ page }) => { + poRegistration = new Registration(page); + + await page.goto('/home'); + }); + + test('Login Page', async ({ api }) => { + await test.step('expect OAuth button to be visible', async () => { + await expect((await setSettingValueById(api, 'Accounts_OAuth_Google', true)).status()).toBe(200); + await expect(poRegistration.btnLoginWithGoogle).toBeVisible({ timeout: 10000 }); + }); + + await test.step('expect OAuth button to not be visible', async () => { + await expect((await setSettingValueById(api, 'Accounts_OAuth_Google', false)).status()).toBe(200); + + await expect(poRegistration.btnLoginWithGoogle).not.toBeVisible({ timeout: 10000 }); + }); + }); +}); \ No newline at end of file diff --git a/apps/meteor/tests/e2e/page-objects/auth.ts b/apps/meteor/tests/e2e/page-objects/auth.ts index 9b47d2e44ad..d0a7e13d650 100644 --- a/apps/meteor/tests/e2e/page-objects/auth.ts +++ b/apps/meteor/tests/e2e/page-objects/auth.ts @@ -20,6 +20,10 @@ export class Registration { return this.page.locator('role=button[name="Login"]'); } + get btnLoginWithGoogle(): Locator { + return this.page.locator('role=button[name="Sign in with Google"]'); + } + get goToRegister(): Locator { return this.page.locator('role=link[name="Create an account"]'); } diff --git a/apps/meteor/tests/end-to-end/api/08-settings.js b/apps/meteor/tests/end-to-end/api/08-settings.js index de8a21ffac4..d517b60eea9 100644 --- a/apps/meteor/tests/end-to-end/api/08-settings.js +++ b/apps/meteor/tests/end-to-end/api/08-settings.js @@ -2,6 +2,7 @@ import { expect } from 'chai'; import { before, describe, it } from 'mocha'; import { getCredentials, api, request, credentials } from '../../data/api-data.js'; +import { updateSetting } from '../../data/permissions.helper'; describe('[Settings]', function () { this.retries(0); @@ -84,6 +85,54 @@ describe('[Settings]', function () { }) .end(done); }); + + describe('With OAuth enabled', () => { + before((done) => { + updateSetting('Accounts_OAuth_Google', true).then(done); + }); + + it('should include the OAuth service in the response', (done) => { + // wait 3 seconds before getting the service list so the server has had time to update it + setTimeout(() => { + request + .get(api('service.configurations')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('configurations'); + + expect(res.body.configurations.find(({ service }) => service === 'google')).to.exist; + }) + .end(done); + }, 3000); + }); + }); + + describe('With OAuth disabled', () => { + before((done) => { + updateSetting('Accounts_OAuth_Google', false).then(done); + }); + + it('should not include the OAuth service in the response', (done) => { + // wait 3 seconds before getting the service list so the server has had time to update it + setTimeout(() => { + request + .get(api('service.configurations')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('configurations'); + + expect(res.body.configurations.find(({ service }) => service === 'google')).to.not.exist; + }) + .end(done); + }, 3000); + }); + }); }); describe('/settings.oauth', () => { diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 8cc55a09134..79905fc8206 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -44,7 +44,9 @@ export class DDPStreamer extends ServiceClass { return; } - events.emit('meteor.loginServiceConfiguration', clientAction === 'inserted' ? 'added' : 'changed', data); + if (data) { + events.emit('meteor.loginServiceConfiguration', clientAction === 'inserted' ? 'added' : 'changed', data); + } }); this.onEvent('meteor.clientVersionUpdated', (versions): void => { diff --git a/packages/core-services/src/events/Events.ts b/packages/core-services/src/events/Events.ts index 67327c3ea21..6315bf43fa1 100644 --- a/packages/core-services/src/events/Events.ts +++ b/packages/core-services/src/events/Events.ts @@ -40,6 +40,19 @@ import type { AutoUpdateRecord } from '../types/IMeteor'; type ClientAction = 'inserted' | 'updated' | 'removed' | 'changed'; +type LoginServiceConfigurationEvent = { + id: string; +} & ( + | { + clientAction: 'removed'; + data?: never; + } + | { + clientAction: Omit; + data: Partial; + } +); + export type EventSignatures = { 'room.video-conference': (params: { rid: string; callId: string }) => void; 'shutdown': (params: Record) => void; @@ -235,7 +248,7 @@ export type EventSignatures = { } ), ): void; - 'watch.loginServiceConfiguration'(data: { clientAction: ClientAction; data: Partial; id: string }): void; + 'watch.loginServiceConfiguration'(data: LoginServiceConfigurationEvent): void; 'watch.instanceStatus'(data: { clientAction: ClientAction; data?: undefined | Partial;