Skip to content

Commit

Permalink
fix: login buttons remain visible until refresh after disabling authe…
Browse files Browse the repository at this point in the history
…ntication service (#31371)
  • Loading branch information
pierre-lehnen-rc authored Jan 9, 2024
1 parent 319f05e commit 9a6e9b4
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 7 deletions.
7 changes: 7 additions & 0 deletions .changeset/little-planes-wonder.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions apps/meteor/packages/rocketchat-mongo-config/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'] = {};
}

Expand Down
6 changes: 6 additions & 0 deletions apps/meteor/server/modules/watchers/watchers.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,13 @@ export function initWatchers(watcher: DatabaseWatcher, broadcast: BroadcastCallb
});

watcher.on<ILoginServiceConfiguration>(LoginServiceConfiguration.getCollectionName(), async ({ clientAction, id }) => {
if (clientAction === 'removed') {
void broadcast('watch.loginServiceConfiguration', { clientAction, id });
return;
}

const data = await LoginServiceConfiguration.findOne<Omit<ILoginServiceConfiguration, 'secret'>>(id, { projection: { secret: 0 } });

if (!data) {
return;
}
Expand Down
8 changes: 5 additions & 3 deletions apps/meteor/server/services/meteor/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
});
}

Expand Down
4 changes: 4 additions & 0 deletions apps/meteor/tests/e2e/fixtures/inject-initial-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
26 changes: 26 additions & 0 deletions apps/meteor/tests/e2e/oauth.spec.ts
Original file line number Diff line number Diff line change
@@ -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 });
});
});
});
4 changes: 4 additions & 0 deletions apps/meteor/tests/e2e/page-objects/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]');
}
Expand Down
49 changes: 49 additions & 0 deletions apps/meteor/tests/end-to-end/api/08-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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', () => {
Expand Down
4 changes: 3 additions & 1 deletion ee/apps/ddp-streamer/src/DDPStreamer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
15 changes: 14 additions & 1 deletion packages/core-services/src/events/Events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClientAction, 'removed'>;
data: Partial<ILoginServiceConfiguration>;
}
);

export type EventSignatures = {
'room.video-conference': (params: { rid: string; callId: string }) => void;
'shutdown': (params: Record<string, string[]>) => void;
Expand Down Expand Up @@ -235,7 +248,7 @@ export type EventSignatures = {
}
),
): void;
'watch.loginServiceConfiguration'(data: { clientAction: ClientAction; data: Partial<ILoginServiceConfiguration>; id: string }): void;
'watch.loginServiceConfiguration'(data: LoginServiceConfigurationEvent): void;
'watch.instanceStatus'(data: {
clientAction: ClientAction;
data?: undefined | Partial<IInstanceStatus>;
Expand Down

0 comments on commit 9a6e9b4

Please sign in to comment.