Skip to content

Commit

Permalink
fix: Login services button colors (#32570)
Browse files Browse the repository at this point in the history
  • Loading branch information
yash-rajpal authored Jul 10, 2024
1 parent 72b8b5e commit 1566412
Show file tree
Hide file tree
Showing 10 changed files with 40 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": minor
---

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
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 @@ -57,6 +58,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 @@ -150,6 +153,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 @@ -3955,6 +3955,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 1566412

Please sign in to comment.