From 15920f53541139741e14b9fbb967f585409033dc Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 6 Sep 2022 08:21:59 +0200 Subject: [PATCH] [EC-512] Tree shakeable icons (#3427) * [EC-512] feat: create new icon class * [EC-512] feat: implement protected svgIcon function * [EC-512] feat: use new icon class in component * [EC-512] feat: integrate new icons in application * [EC-512] fix: linting * [EC-512] chore: move report icons to where they are used * [EC-512] chore: add export type explanation --- .eslintrc.json | 9 ++- .../app/reports/icons/report-breach.icon.ts | 14 +++++ .../icons/report-exposed-passwords.icon.ts | 16 +++++ .../icons/report-inactive-two-factor.icon.ts | 10 +++ .../icons/report-reused-passwords.icon.ts | 9 +++ .../icons/report-unsecured-websites.icon.ts | 12 ++++ .../icons/report-weak-passwords.icon.ts | 10 +++ .../src/app/reports/models/report-entry.ts | 4 +- apps/web/src/app/reports/reports.ts | 18 ++++-- libs/components/package.json | 1 + libs/components/src/icon/icon.component.ts | 10 +-- .../src/icon/icon.components.spec.ts | 39 ++++++++++++ libs/components/src/icon/icon.spec.ts | 38 +++++++++++ libs/components/src/icon/icon.ts | 25 ++++++++ libs/components/src/icon/icons.ts | 63 ------------------- libs/components/src/icon/icons/index.ts | 4 ++ libs/components/src/icon/index.ts | 2 + 17 files changed, 208 insertions(+), 76 deletions(-) create mode 100644 apps/web/src/app/reports/icons/report-breach.icon.ts create mode 100644 apps/web/src/app/reports/icons/report-exposed-passwords.icon.ts create mode 100644 apps/web/src/app/reports/icons/report-inactive-two-factor.icon.ts create mode 100644 apps/web/src/app/reports/icons/report-reused-passwords.icon.ts create mode 100644 apps/web/src/app/reports/icons/report-unsecured-websites.icon.ts create mode 100644 apps/web/src/app/reports/icons/report-weak-passwords.icon.ts create mode 100644 libs/components/src/icon/icon.components.spec.ts create mode 100644 libs/components/src/icon/icon.spec.ts create mode 100644 libs/components/src/icon/icon.ts delete mode 100644 libs/components/src/icon/icons.ts create mode 100644 libs/components/src/icon/icons/index.ts diff --git a/.eslintrc.json b/.eslintrc.json index 72459521605..404abef39a7 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -57,6 +57,13 @@ "pathGroupsExcludedImportTypes": ["builtin"] } ], - "rxjs-angular/prefer-takeuntil": "error" + "rxjs-angular/prefer-takeuntil": "error", + "no-restricted-syntax": [ + "error", + { + "message": "Calling `svgIcon` directly is not allowed", + "selector": "CallExpression[callee.name='svgIcon']" + } + ] } } diff --git a/apps/web/src/app/reports/icons/report-breach.icon.ts b/apps/web/src/app/reports/icons/report-breach.icon.ts new file mode 100644 index 00000000000..749779a87bc --- /dev/null +++ b/apps/web/src/app/reports/icons/report-breach.icon.ts @@ -0,0 +1,14 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportBreach = svgIcon` + + + + + + + + + + +`; diff --git a/apps/web/src/app/reports/icons/report-exposed-passwords.icon.ts b/apps/web/src/app/reports/icons/report-exposed-passwords.icon.ts new file mode 100644 index 00000000000..df532fe1582 --- /dev/null +++ b/apps/web/src/app/reports/icons/report-exposed-passwords.icon.ts @@ -0,0 +1,16 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportExposedPasswords = svgIcon` + + + + + + + + + + + + +`; diff --git a/apps/web/src/app/reports/icons/report-inactive-two-factor.icon.ts b/apps/web/src/app/reports/icons/report-inactive-two-factor.icon.ts new file mode 100644 index 00000000000..65ca4469648 --- /dev/null +++ b/apps/web/src/app/reports/icons/report-inactive-two-factor.icon.ts @@ -0,0 +1,10 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportInactiveTwoFactor = svgIcon` + + + + + + +`; diff --git a/apps/web/src/app/reports/icons/report-reused-passwords.icon.ts b/apps/web/src/app/reports/icons/report-reused-passwords.icon.ts new file mode 100644 index 00000000000..d28790ba051 --- /dev/null +++ b/apps/web/src/app/reports/icons/report-reused-passwords.icon.ts @@ -0,0 +1,9 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportReusedPasswords = svgIcon` + + + + + +`; diff --git a/apps/web/src/app/reports/icons/report-unsecured-websites.icon.ts b/apps/web/src/app/reports/icons/report-unsecured-websites.icon.ts new file mode 100644 index 00000000000..083ddb40db7 --- /dev/null +++ b/apps/web/src/app/reports/icons/report-unsecured-websites.icon.ts @@ -0,0 +1,12 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportUnsecuredWebsites = svgIcon` + + + + + + + + +`; diff --git a/apps/web/src/app/reports/icons/report-weak-passwords.icon.ts b/apps/web/src/app/reports/icons/report-weak-passwords.icon.ts new file mode 100644 index 00000000000..8a0ef324c73 --- /dev/null +++ b/apps/web/src/app/reports/icons/report-weak-passwords.icon.ts @@ -0,0 +1,10 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ReportWeakPasswords = svgIcon` + + + + + + +`; diff --git a/apps/web/src/app/reports/models/report-entry.ts b/apps/web/src/app/reports/models/report-entry.ts index 08e31a8eca2..d4da4e36763 100644 --- a/apps/web/src/app/reports/models/report-entry.ts +++ b/apps/web/src/app/reports/models/report-entry.ts @@ -1,9 +1,11 @@ +import { Icon } from "@bitwarden/components"; + import { ReportVariant } from "./report-variant"; export type ReportEntry = { title: string; description: string; route: string; - icon: string; + icon: Icon; variant: ReportVariant; }; diff --git a/apps/web/src/app/reports/reports.ts b/apps/web/src/app/reports/reports.ts index cd393dd113f..43d086d4186 100644 --- a/apps/web/src/app/reports/reports.ts +++ b/apps/web/src/app/reports/reports.ts @@ -1,3 +1,9 @@ +import { ReportBreach } from "./icons/report-breach.icon"; +import { ReportExposedPasswords } from "./icons/report-exposed-passwords.icon"; +import { ReportInactiveTwoFactor } from "./icons/report-inactive-two-factor.icon"; +import { ReportReusedPasswords } from "./icons/report-reused-passwords.icon"; +import { ReportUnsecuredWebsites } from "./icons/report-unsecured-websites.icon"; +import { ReportWeakPasswords } from "./icons/report-weak-passwords.icon"; import { ReportEntry } from "./models/report-entry"; export enum ReportType { @@ -16,36 +22,36 @@ export const reports: Record = { title: "exposedPasswordsReport", description: "exposedPasswordsReportDesc", route: "exposed-passwords-report", - icon: "reportExposedPasswords", + icon: ReportExposedPasswords, }, [ReportType.ReusedPasswords]: { title: "reusedPasswordsReport", description: "reusedPasswordsReportDesc", route: "reused-passwords-report", - icon: "reportReusedPasswords", + icon: ReportReusedPasswords, }, [ReportType.WeakPasswords]: { title: "weakPasswordsReport", description: "weakPasswordsReportDesc", route: "weak-passwords-report", - icon: "reportWeakPasswords", + icon: ReportWeakPasswords, }, [ReportType.UnsecuredWebsites]: { title: "unsecuredWebsitesReport", description: "unsecuredWebsitesReportDesc", route: "unsecured-websites-report", - icon: "reportUnsecuredWebsites", + icon: ReportUnsecuredWebsites, }, [ReportType.Inactive2fa]: { title: "inactive2faReport", description: "inactive2faReportDesc", route: "inactive-two-factor-report", - icon: "reportInactiveTwoFactor", + icon: ReportInactiveTwoFactor, }, [ReportType.DataBreach]: { title: "dataBreachReport", description: "breachDesc", route: "breach-report", - icon: "reportBreach", + icon: ReportBreach, }, }; diff --git a/libs/components/package.json b/libs/components/package.json index d4db87c3566..168c727a3e5 100644 --- a/libs/components/package.json +++ b/libs/components/package.json @@ -1,6 +1,7 @@ { "name": "@bitwarden/components", "version": "0.0.0", + "sideEffects": false, "scripts": { "ng": "ng", "start": "ng serve", diff --git a/libs/components/src/icon/icon.component.ts b/libs/components/src/icon/icon.component.ts index 85e23fb9ea9..664642a04e9 100644 --- a/libs/components/src/icon/icon.component.ts +++ b/libs/components/src/icon/icon.component.ts @@ -1,7 +1,7 @@ import { Component, HostBinding, Input } from "@angular/core"; import { DomSanitizer } from "@angular/platform-browser"; -import { Icon, IconSvg } from "./icons"; +import { Icon, isIcon } from "./icon"; @Component({ selector: "bit-icon", @@ -14,11 +14,11 @@ export class BitIconComponent { @HostBinding("innerHtml") protected get innerHtml() { - const svg = IconSvg[this.icon]; - if (svg == null) { - return "Unknown icon"; + if (!isIcon(this.icon)) { + return ""; } - return this.domSanitizer.bypassSecurityTrustHtml(IconSvg[this.icon]); + const svg = this.icon.svg; + return this.domSanitizer.bypassSecurityTrustHtml(svg); } } diff --git a/libs/components/src/icon/icon.components.spec.ts b/libs/components/src/icon/icon.components.spec.ts new file mode 100644 index 00000000000..351ed5f0218 --- /dev/null +++ b/libs/components/src/icon/icon.components.spec.ts @@ -0,0 +1,39 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; + +import { Icon, svgIcon } from "./icon"; +import { BitIconComponent } from "./icon.component"; + +describe("IconComponent", () => { + let component: BitIconComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [BitIconComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(BitIconComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should have empty innerHtml when input is not an Icon", () => { + const fakeIcon = { svg: "harmful user input" } as Icon; + + component.icon = fakeIcon; + fixture.detectChanges(); + + const el = fixture.nativeElement as HTMLElement; + expect(el.innerHTML).toBe(""); + }); + + it("should contain icon when input is a safe Icon", () => { + const icon = svgIcon`safe icon`; + + component.icon = icon; + fixture.detectChanges(); + + const el = fixture.nativeElement as HTMLElement; + expect(el.innerHTML).toBe(`safe icon`); + }); +}); diff --git a/libs/components/src/icon/icon.spec.ts b/libs/components/src/icon/icon.spec.ts new file mode 100644 index 00000000000..3e7c011be93 --- /dev/null +++ b/libs/components/src/icon/icon.spec.ts @@ -0,0 +1,38 @@ +import * as IconExports from "./icon"; +import { DynamicContentNotAllowedError, isIcon, svgIcon } from "./icon"; + +describe("Icon", () => { + it("exports should not expose Icon class", () => { + expect(Object.keys(IconExports)).not.toContain("Icon"); + }); + + describe("isIcon", () => { + it("should return true when input is icon", () => { + const result = isIcon(svgIcon`icon`); + + expect(result).toBe(true); + }); + + it("should return false when input is not an icon", () => { + const result = isIcon({ svg: "not an icon" }); + + expect(result).toBe(false); + }); + }); + + describe("template literal", () => { + it("should throw when attempting to create dynamic icons", () => { + const dynamic = "some user input"; + + const f = () => svgIcon`static and ${dynamic}`; + + expect(f).toThrow(DynamicContentNotAllowedError); + }); + + it("should return svg content when supplying icon with svg string", () => { + const icon = svgIcon`safe static content`; + + expect(icon.svg).toBe("safe static content"); + }); + }); +}); diff --git a/libs/components/src/icon/icon.ts b/libs/components/src/icon/icon.ts new file mode 100644 index 00000000000..b397431da28 --- /dev/null +++ b/libs/components/src/icon/icon.ts @@ -0,0 +1,25 @@ +class Icon { + constructor(readonly svg: string) {} +} + +// We only export the type to prohibit the creation of Icons without using +// the `svgIcon` template literal tag. +export type { Icon }; + +export function isIcon(icon: unknown): icon is Icon { + return icon instanceof Icon; +} + +export class DynamicContentNotAllowedError extends Error { + constructor() { + super("Dynamic content in icons is not allowed due to risk of user-injected XSS."); + } +} + +export function svgIcon(strings: TemplateStringsArray, ...values: unknown[]): Icon { + if (values.length > 0) { + throw new DynamicContentNotAllowedError(); + } + + return new Icon(strings[0]); +} diff --git a/libs/components/src/icon/icons.ts b/libs/components/src/icon/icons.ts deleted file mode 100644 index c316d312e30..00000000000 --- a/libs/components/src/icon/icons.ts +++ /dev/null @@ -1,63 +0,0 @@ -export const IconSvg = { - reportExposedPasswords: ` - - - - - - - - - - - - - `, - reportReusedPasswords: ` - - - - - - `, - reportWeakPasswords: ` - - - - - - - `, - reportUnsecuredWebsites: ` - - - - - - - - - `, - reportInactiveTwoFactor: ` - - - - - - - `, - reportBreach: ` - - - - - - - - - - - `, -}; - -export type Icon = keyof typeof IconSvg; diff --git a/libs/components/src/icon/icons/index.ts b/libs/components/src/icon/icons/index.ts new file mode 100644 index 00000000000..bbb98817da3 --- /dev/null +++ b/libs/components/src/icon/icons/index.ts @@ -0,0 +1,4 @@ +// Put generic icons in this folder and export them here. +// Note: Icons need to be in separate files for tree-shaking to work properly + +export {}; // <- remove when adding icons in here diff --git a/libs/components/src/icon/index.ts b/libs/components/src/icon/index.ts index 1ee66e59837..8f29234dbc5 100644 --- a/libs/components/src/icon/index.ts +++ b/libs/components/src/icon/index.ts @@ -1 +1,3 @@ export * from "./icon.module"; +export * from "./icon"; +export * as Icons from "./icons";