Skip to content

Commit

Permalink
fix!: Login services button colors (#33333)
Browse files Browse the repository at this point in the history
  • Loading branch information
yash-rajpal authored Oct 18, 2024
1 parent 15b6f4b commit d44f614
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 3 deletions.
8 changes: 8 additions & 0 deletions .changeset/fifty-mails-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@rocket.chat/web-ui-registration': patch
"@rocket.chat/meteor": major
---

Login services button was not respecting the button color and text color settings. Implemented a fix to respect these settings and change the button colors accordingly.

Added a warning on all settings which allow admins to change OAuth button colors, so that they can be alerted about WCAG (Web Content Accessibility Guidelines) compliance.
2 changes: 2 additions & 0 deletions apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,12 @@ export const addSettings = async function (name: string): Promise<void> {
await this.add(`SAML_Custom_${name}_button_label_color`, '#FFFFFF', {
type: 'string',
i18nLabel: 'Accounts_OAuth_Custom_Button_Label_Color',
alert: 'OAuth_button_colors_alert',
});
await this.add(`SAML_Custom_${name}_button_color`, '#1d74f5', {
type: 'string',
i18nLabel: 'Accounts_OAuth_Custom_Button_Color',
alert: 'OAuth_button_colors_alert',
});
});

Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/server/lib/oauth/addOAuthService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,15 @@ export async function addOAuthService(name: string, values: { [k: string]: strin
section: `Custom OAuth: ${name}`,
i18nLabel: 'Accounts_OAuth_Custom_Button_Label_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
await settingsRegistry.add(`Accounts_OAuth_Custom-${name}-button_color`, values.buttonColor || '#1d74f5', {
type: 'string',
group: 'OAuth',
section: `Custom OAuth: ${name}`,
i18nLabel: 'Accounts_OAuth_Custom_Button_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
await settingsRegistry.add(`Accounts_OAuth_Custom-${name}-key_field`, values.keyField || 'username', {
type: 'select',
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/server/settings/cas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ export const createCasSettings = () =>
await this.add('CAS_popup_width', 810, { type: 'int', group: 'CAS', public: true });
await this.add('CAS_popup_height', 610, { type: 'int', group: 'CAS', public: true });
await this.add('CAS_button_label_text', 'CAS', { type: 'string', group: 'CAS' });
await this.add('CAS_button_label_color', '#FFFFFF', { type: 'color', group: 'CAS' });
await this.add('CAS_button_color', '#1d74f5', { type: 'color', group: 'CAS' });
await this.add('CAS_button_label_color', '#FFFFFF', { type: 'color', group: 'CAS', alert: 'OAuth_button_colors_alert' });
await this.add('CAS_button_color', '#1d74f5', { type: 'color', group: 'CAS', alert: 'OAuth_button_colors_alert' });
await this.add('CAS_autoclose', true, { type: 'boolean', group: 'CAS' });
});
});
4 changes: 4 additions & 0 deletions apps/meteor/server/settings/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,14 @@ export const createOauthSettings = () =>
public: true,
i18nLabel: 'Accounts_OAuth_Custom_Button_Label_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
await this.add('Accounts_OAuth_Nextcloud_button_color', '#0082c9', {
type: 'string',
public: true,
i18nLabel: 'Accounts_OAuth_Custom_Button_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
});

Expand Down Expand Up @@ -273,11 +275,13 @@ export const createOauthSettings = () =>
type: 'string',
i18nLabel: 'Accounts_OAuth_Custom_Button_Label_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
await this.add('Accounts_OAuth_Dolphin_button_color', '#1d74f5', {
type: 'string',
i18nLabel: 'Accounts_OAuth_Custom_Button_Color',
persistent: true,
alert: 'OAuth_button_colors_alert',
});
});
await this.section('Facebook', async function () {
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/server/startup/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ import './v313';
import './v314';
import './v315';
import './v316';
import './v317';

export * from './xrun';
119 changes: 119 additions & 0 deletions apps/meteor/server/startup/migrations/v317.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import type { ILoginServiceConfiguration, OAuthConfiguration } from '@rocket.chat/core-typings';
import { Settings, LoginServiceConfiguration } from '@rocket.chat/models';

import { isTruthy } from '../../../lib/isTruthy';
import { SystemLogger } from '../../lib/logger/system';
import { addMigration } from '../../lib/migrations';

const newDefaultButtonColor = '#e4e7ea';
const newDefaultButtonLabelColor = '#1f2329';

const settingsToUpdate = [
// button background colors
{ key: 'SAML_Custom_Default_button_color', defaultValue: '#1d74f5', newValue: newDefaultButtonColor },
{ key: 'CAS_button_color', defaultValue: '#1d74f5', newValue: newDefaultButtonColor },
{ key: 'Accounts_OAuth_Nextcloud_button_color', defaultValue: '#0082c9', newValue: newDefaultButtonColor },
{ key: 'Accounts_OAuth_Dolphin_button_color', defaultValue: '#1d74f5', newValue: newDefaultButtonColor },
// button label colors
{ key: 'SAML_Custom_Default_button_label_color', defaultValue: '#1d74f5', newValue: newDefaultButtonLabelColor },
{ key: 'CAS_button_label_color', defaultValue: '#1d74f5', newValue: newDefaultButtonLabelColor },
{ key: 'Accounts_OAuth_Nextcloud_button_label_color', defaultValue: '#1d74f5', newValue: newDefaultButtonLabelColor },
{ key: 'Accounts_OAuth_Dolphin_button_label_color', defaultValue: '#1d74f5', newValue: newDefaultButtonLabelColor },
];

const getSettingValue = async (key: string) => Settings.getValueById(key);

async function updateOAuthServices(): Promise<void> {
const services = await Settings.find({ _id: { $regex: /^(Accounts_OAuth_|Accounts_OAuth_Custom-)[a-z0-9_]+$/i } }).toArray();
const filteredServices = services.filter(({ value }) => typeof value === 'boolean');
for await (const { _id: key, value } of filteredServices) {
if (value !== true) {
continue;
}

let serviceName = key.replace('Accounts_OAuth_', '');
if (serviceName === 'Meteor') {
serviceName = 'meteor-developer';
}
if (/Accounts_OAuth_Custom-/.test(key)) {
serviceName = key.replace('Accounts_OAuth_Custom-', '');
}

const serviceKey = serviceName.toLowerCase();

const data: Partial<ILoginServiceConfiguration & Omit<OAuthConfiguration, '_id'>> = {};

if (/Accounts_OAuth_Custom-/.test(key)) {
data.buttonLabelColor = (await getSettingValue(`${key}-button_label_color`)) as string;
data.buttonColor = (await getSettingValue(`${key}-button_color`)) as string;
}

if (serviceName === 'Nextcloud') {
data.buttonLabelColor = (await getSettingValue('Accounts_OAuth_Nextcloud_button_label_color')) as string;
data.buttonColor = (await getSettingValue('Accounts_OAuth_Nextcloud_button_color')) as string;
}

await LoginServiceConfiguration.createOrUpdateService(serviceKey, data);
}
}

addMigration({
version: 317,
name: 'Change default color of OAuth login services buttons',
async up() {
const promises = settingsToUpdate
.map(async ({ key, defaultValue, newValue }) => {
const oldSettingValue = await getSettingValue(key);

if (!oldSettingValue || oldSettingValue !== defaultValue) {
return;
}

SystemLogger.warn(`The default value of the setting ${key} has changed to ${newValue}. Please review your settings.`);

return Settings.updateOne({ _id: key }, { $set: { value: newValue } });
})
.filter(isTruthy);

await Promise.all(promises);

const customOAuthButtonColors = await Settings.find({
_id: { $regex: /^Accounts_OAuth_Custom-[a-zA-Z0-9_-]+-button_color$/ },
}).toArray();
const customOAuthButtonLabelColors = await Settings.find({
_id: { $regex: /^Accounts_OAuth_Custom-[a-zA-Z0-9_-]+-button_label_color$/ },
}).toArray();

const buttonColorPromises = customOAuthButtonColors
.map(({ _id, value, packageValue }) => {
if (packageValue !== value) {
return;
}

SystemLogger.warn(
`The default value of the custom setting ${_id} has changed to ${newDefaultButtonColor}. Please review your settings.`,
);

return Settings.updateOne({ _id }, { $set: { value: newDefaultButtonColor } });
})
.filter(isTruthy);

const buttonLabelColorPromises = customOAuthButtonLabelColors
.map(({ _id, value, packageValue }) => {
if (packageValue !== value) {
return;
}

SystemLogger.warn(
`The default value of the custom setting ${_id} has changed to ${newDefaultButtonLabelColor}. Please review your settings.`,
);

return Settings.updateOne({ _id }, { $set: { value: newDefaultButtonLabelColor } });
})
.filter(isTruthy);

await Promise.all([...buttonColorPromises, ...buttonLabelColorPromises]);
// update login service configurations
await updateOAuthServices();
},
});
2 changes: 1 addition & 1 deletion apps/meteor/tests/e2e/page-objects/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class Registration {
}

get btnLoginWithSaml(): Locator {
return this.page.locator('role=button[name="SAML"]');
return this.page.locator('role=button[name="SAML test login button"]');
}

get btnLoginWithGoogle(): Locator {
Expand Down
7 changes: 7 additions & 0 deletions apps/meteor/tests/e2e/saml.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as constants from './config/constants';
import { createUserFixture } from './fixtures/collections/users';
import { Users } from './fixtures/userStates';
import { Registration } from './page-objects';
import { convertHexToRGB } from './utils/convertHexToRGB';
import { createCustomRole, deleteCustomRole } from './utils/custom-role';
import { getUserInfo } from './utils/getUserInfo';
import { parseMeteorResponse } from './utils/parseMeteorResponse';
Expand Down Expand Up @@ -59,6 +60,8 @@ const resetTestData = async ({ api, cleanupOnly = false }: { api?: any; cleanupO
{ _id: 'SAML_Custom_Default_issuer', value: 'http://localhost:3000/_saml/metadata/test-sp' },
{ _id: 'SAML_Custom_Default_entry_point', value: 'http://localhost:8080/simplesaml/saml2/idp/SSOService.php' },
{ _id: 'SAML_Custom_Default_idp_slo_redirect_url', value: 'http://localhost:8080/simplesaml/saml2/idp/SingleLogoutService.php' },
{ _id: 'SAML_Custom_Default_button_label_text', value: 'SAML test login button' },
{ _id: 'SAML_Custom_Default_button_color', value: '#185925' },
];

await Promise.all(settings.map(({ _id, value }) => setSettingValueById(api, _id, value)));
Expand Down Expand Up @@ -152,6 +155,10 @@ test.describe('SAML', () => {
await expect(poRegistration.btnLoginWithSaml).toBeVisible({ timeout: 10000 });
});

await test.step('expect to have SAML login button to have the required background color', async () => {
await expect(poRegistration.btnLoginWithSaml).toHaveCSS('background-color', convertHexToRGB('#185925'));
});

await test.step('expect to be redirected to the IdP for login', async () => {
await poRegistration.btnLoginWithSaml.click();

Expand Down
9 changes: 9 additions & 0 deletions apps/meteor/tests/e2e/utils/convertHexToRGB.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const convertHexToRGB = (hex: string) => {
hex = hex.replace(/^#/, '');

const red = parseInt(hex.substring(0, 2), 16);
const green = parseInt(hex.substring(2, 4), 16);
const blue = parseInt(hex.substring(4, 6), 16);

return `rgb(${red}, ${green}, ${blue})`;
};
1 change: 1 addition & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -4030,6 +4030,7 @@
"OAuth": "OAuth",
"OAuth_Description": "Configure authentication methods beyond just username and password.",
"OAuth_Application": "OAuth Application",
"OAuth_button_colors_alert": "Changing the color may result in non-compliance with WCAG (Web Content Accessibility Guidelines) requirements. Please ensure that the new colors meet the recommended contrast and readability standards to maintain accessibility for all users.",
"Objects": "Objects",
"Off": "Off",
"Off_the_record_conversation": "Off-the-Record Conversation",
Expand Down
4 changes: 4 additions & 0 deletions packages/web-ui-registration/src/LoginServicesButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const LoginServicesButton = <T extends LoginService>({
className,
disabled,
setError,
buttonColor,
buttonLabelColor,
...props
}: T & {
className?: string;
Expand Down Expand Up @@ -43,6 +45,8 @@ const LoginServicesButton = <T extends LoginService>({
alignItems='center'
display='flex'
justifyContent='center'
color={buttonLabelColor}
backgroundColor={buttonColor}
>
{buttonLabelText || t('Sign_in_with__provider__', { provider: title })}
</Button>
Expand Down

0 comments on commit d44f614

Please sign in to comment.