From a71778651fe70639e9fd6ef3a4c047a7bcd1cc5a Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 8 Jun 2023 08:35:23 -0700 Subject: [PATCH] web: improve password experience This commit disassembles PromptStage and places function that don't need a reference to the PromptStage object into a collection of maps between the Stage type and the prompt associated with it. (In a better world, this would be a great place to try some post-Midgard mplementation of itemtype/itemid/itemprop). This surfaced the nature of the relationship between Password and Password (Repeat), allowing us to modify both to show password strength and password matching for the "change password" dialog. --- web/README.md | 13 + web/package-lock.json | 15 +- web/package.json | 5 +- .../password-match-indicator/index.ts | 5 + .../password-match-indicator.stories.ts | 14 + .../password-match-indicator.ts | 91 +++++++ .../password-strength-indicator/findInput.ts | 18 ++ .../password-strength-indicator/index.ts | 5 + .../password-strength-indicator.stories.ts | 13 + .../password-strength-indicator.ts | 91 +++++++ web/src/flow/stages/prompt/FieldRenderers.ts | 245 ++++++++++++++++++ web/src/flow/stages/prompt/PromptStage.ts | 191 +------------- 12 files changed, 526 insertions(+), 180 deletions(-) create mode 100644 web/src/elements/password-match-indicator/index.ts create mode 100644 web/src/elements/password-match-indicator/password-match-indicator.stories.ts create mode 100644 web/src/elements/password-match-indicator/password-match-indicator.ts create mode 100644 web/src/elements/password-strength-indicator/findInput.ts create mode 100644 web/src/elements/password-strength-indicator/index.ts create mode 100644 web/src/elements/password-strength-indicator/password-strength-indicator.stories.ts create mode 100644 web/src/elements/password-strength-indicator/password-strength-indicator.ts create mode 100644 web/src/flow/stages/prompt/FieldRenderers.ts diff --git a/web/README.md b/web/README.md index 24cc5c622..9e95b2272 100644 --- a/web/README.md +++ b/web/README.md @@ -3,6 +3,15 @@ This is the default UI for the authentik server. The documentation is going to be a little sparse for awhile, but at least let's get started. +# Standards + +- Be flexible in what you accept as input, be precise in what you produce as output. +- Mis-use is always a crash. A component that takes the ID of an HTMLInputElement as an argument + should throw an exception if the element is anything but an HTMLInputElement ("anything" includes + non-existent, null, undefined, etc.). +- Single Responsibility is ideal, but not always practical. To the best of your obility, every + object in the system should do one thing and do it well. + # Comments **NOTE:** The comments in this section are for specific changes to this repository that cannot be @@ -21,3 +30,7 @@ settings in JSON files, which do not support comments. - `compilerOptions.plugins.ts-lit-plugin.rules.no-incompatible-type-binding: "warn"`: lit-analyzer does not support generics well when parsing a subtype of `HTMLElement`. As a result, this threw too many errors to be supportable. +- `package.json` + - `prettier` should always be the last thing run in any pre-commit pass. The `precommit` script + does this, but if you don't use `precommit`, make sure `prettier` is the _last_ thing you do + before a `git commit`. diff --git a/web/package-lock.json b/web/package-lock.json index a033c5a5c..4c004870d 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -35,7 +35,8 @@ "mermaid": "^10.2.2", "rapidoc": "^9.3.4", "webcomponent-qr-code": "^1.1.1", - "yaml": "^2.3.1" + "yaml": "^2.3.1", + "zxcvbn": "^4.4.2" }, "devDependencies": { "@babel/core": "^7.22.1", @@ -64,6 +65,7 @@ "@types/chart.js": "^2.9.37", "@types/codemirror": "5.60.8", "@types/grecaptcha": "^3.0.4", + "@types/zxcvbn": "^4.4.1", "@typescript-eslint/eslint-plugin": "^5.59.9", "@typescript-eslint/parser": "^5.59.9", "babel-plugin-macros": "^3.1.0", @@ -6507,6 +6509,12 @@ "integrity": "sha512-iO9ZQHkZxHn4mSakYV0vFHAVDyEOIJQrV2uZ06HxEPcx+mt8swXoZHIbaaJ2crJYFfErySgktuTZ3BeLz+XmFA==", "dev": true }, + "node_modules/@types/zxcvbn": { + "version": "4.4.1", + "resolved": "https://registry.npmjs.org/@types/zxcvbn/-/zxcvbn-4.4.1.tgz", + "integrity": "sha512-3NoqvZC2W5gAC5DZbTpCeJ251vGQmgcWIHQJGq2J240HY6ErQ9aWKkwfoKJlHLx+A83WPNTZ9+3cd2ILxbvr1w==", + "dev": true + }, "node_modules/@typescript-eslint/eslint-plugin": { "version": "5.59.9", "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-5.59.9.tgz", @@ -18034,6 +18042,11 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/zxcvbn": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/zxcvbn/-/zxcvbn-4.4.2.tgz", + "integrity": "sha512-Bq0B+ixT/DMyG8kgX2xWcI5jUvCwqrMxSFam7m0lAf78nf04hv6lNCsyLYdyYTrCVMqNDY/206K7eExYCeSyUQ==" } } } diff --git a/web/package.json b/web/package.json index dd127c296..48797c24f 100644 --- a/web/package.json +++ b/web/package.json @@ -16,6 +16,7 @@ "watch": "run-s build-locales rollup:watch", "lint": "eslint . --max-warnings 0 --fix", "lit-analyse": "lit-analyzer src", + "precommit": "run-s build-locales lint tsc prettier", "prettier-check": "prettier --check .", "prettier": "prettier --write .", "tsc:execute": "tsc --noEmit -p .", @@ -51,7 +52,8 @@ "mermaid": "^10.2.2", "rapidoc": "^9.3.4", "webcomponent-qr-code": "^1.1.1", - "yaml": "^2.3.1" + "yaml": "^2.3.1", + "zxcvbn": "^4.4.2" }, "devDependencies": { "@babel/core": "^7.22.1", @@ -80,6 +82,7 @@ "@types/chart.js": "^2.9.37", "@types/codemirror": "5.60.8", "@types/grecaptcha": "^3.0.4", + "@types/zxcvbn": "^4.4.1", "@typescript-eslint/eslint-plugin": "^5.59.9", "@typescript-eslint/parser": "^5.59.9", "babel-plugin-macros": "^3.1.0", diff --git a/web/src/elements/password-match-indicator/index.ts b/web/src/elements/password-match-indicator/index.ts new file mode 100644 index 000000000..4116e1985 --- /dev/null +++ b/web/src/elements/password-match-indicator/index.ts @@ -0,0 +1,5 @@ +import PasswordMatchIndicator from "./password-match-indicator.js"; + +export { PasswordMatchIndicator }; + +export default PasswordMatchIndicator; diff --git a/web/src/elements/password-match-indicator/password-match-indicator.stories.ts b/web/src/elements/password-match-indicator/password-match-indicator.stories.ts new file mode 100644 index 000000000..eb6e85ea8 --- /dev/null +++ b/web/src/elements/password-match-indicator/password-match-indicator.stories.ts @@ -0,0 +1,14 @@ +import { html } from "lit"; + +import "."; + +export default { + title: "Elements/Password Match Indicator", +}; + +export const Primary = () => + html`
+

Type some text:

+

Type some other text:

+ +
`; diff --git a/web/src/elements/password-match-indicator/password-match-indicator.ts b/web/src/elements/password-match-indicator/password-match-indicator.ts new file mode 100644 index 000000000..745926faa --- /dev/null +++ b/web/src/elements/password-match-indicator/password-match-indicator.ts @@ -0,0 +1,91 @@ +import { AKElement } from "@goauthentik/elements/Base"; + +import { css, html } from "lit"; +import { customElement, property, state } from "lit/decorators.js"; + +import PFBase from "@patternfly/patternfly/patternfly-base.css"; + +import findInput from "../password-strength-indicator/findInput.js"; + +/** + * A simple display showing if the passwords match. This element is extremely fragile and + * role-specific, depending as it does on the token string '_repeat' inside the selector. + */ + +const ELEMENT = "ak-password-match-indicator"; + +@customElement(ELEMENT) +export class PasswordMatchIndicator extends AKElement { + static styles = [ + PFBase, + css` + :host { + display: grid; + place-items: center center; + } + `, + ]; + + /** + * The input element to observe. Attaching this to anything other than an HTMLInputElement will + * throw an exception. + */ + @property({ attribute: true }) + src = ""; + + sourceInput?: HTMLInputElement; + otherInput?: HTMLInputElement; + + @state() + match = false; + + constructor() { + super(); + this.checkPasswordMatch = this.checkPasswordMatch.bind(this); + } + + connectedCallback() { + super.connectedCallback(); + this.input.addEventListener("keyup", this.checkPasswordMatch); + this.other.addEventListener("keyup", this.checkPasswordMatch); + } + + disconnectedCallback() { + this.other.removeEventListener("keyup", this.checkPasswordMatch); + this.input.removeEventListener("keyup", this.checkPasswordMatch); + super.disconnectedCallback(); + } + + checkPasswordMatch() { + this.match = + this.input.value.length > 0 && + this.other.value.length > 0 && + this.input.value === this.other.value; + } + + get input() { + if (this.sourceInput) { + return this.sourceInput; + } + return (this.sourceInput = findInput(this.getRootNode() as Element, ELEMENT, this.src)); + } + + get other() { + if (this.otherInput) { + return this.otherInput; + } + return (this.otherInput = findInput( + this.getRootNode() as Element, + ELEMENT, + this.src.replace(/_repeat/, ""), + )); + } + + render() { + return this.match + ? html`` + : html``; + } +} + +export default PasswordMatchIndicator; diff --git a/web/src/elements/password-strength-indicator/findInput.ts b/web/src/elements/password-strength-indicator/findInput.ts new file mode 100644 index 000000000..7a42d8699 --- /dev/null +++ b/web/src/elements/password-strength-indicator/findInput.ts @@ -0,0 +1,18 @@ +export function findInput(root: Element, tag: string, src: string) { + const inputs = Array.from(root.querySelectorAll(src)); + if (inputs.length === 0) { + throw new Error(`${tag}: no element found for 'src' ${src}`); + } + if (inputs.length > 1) { + throw new Error(`${tag}: more than one element found for 'src' ${src}`); + } + const input = inputs[0]; + if (!(input instanceof HTMLInputElement)) { + throw new Error( + `${tag}: the 'src' element must be an tag, found ${input.localName}`, + ); + } + return input; +} + +export default findInput; diff --git a/web/src/elements/password-strength-indicator/index.ts b/web/src/elements/password-strength-indicator/index.ts new file mode 100644 index 000000000..1b1bcba7b --- /dev/null +++ b/web/src/elements/password-strength-indicator/index.ts @@ -0,0 +1,5 @@ +import PasswordStrengthIndicator from "./password-strength-indicator.js"; + +export { PasswordStrengthIndicator }; + +export default PasswordStrengthIndicator; diff --git a/web/src/elements/password-strength-indicator/password-strength-indicator.stories.ts b/web/src/elements/password-strength-indicator/password-strength-indicator.stories.ts new file mode 100644 index 000000000..787e66da3 --- /dev/null +++ b/web/src/elements/password-strength-indicator/password-strength-indicator.stories.ts @@ -0,0 +1,13 @@ +import { html } from "lit"; + +import "."; + +export default { + title: "Elements/Password Strength Indicator", +}; + +export const Primary = () => + html`
+

Type some text:

+ +
`; diff --git a/web/src/elements/password-strength-indicator/password-strength-indicator.ts b/web/src/elements/password-strength-indicator/password-strength-indicator.ts new file mode 100644 index 000000000..141fe2035 --- /dev/null +++ b/web/src/elements/password-strength-indicator/password-strength-indicator.ts @@ -0,0 +1,91 @@ +import { AKElement } from "@goauthentik/elements/Base"; +import zxcvbn from "zxcvbn"; + +import { css, html } from "lit"; +import { styleMap } from "lit-html/directives/style-map.js"; +import { customElement, property, state } from "lit/decorators.js"; + +import findInput from "./findInput"; + +const styles = css` + .password-meter-wrap { + margin-top: 5px; + height: 0.5em; + background-color: #ddd; + border-radius: 0.25em; + + overflow: hidden; + } + + .password-meter-bar { + width: 0; + height: 100%; + transition: width 400ms ease-in; + } +`; + +const LEVELS = [ + ["20%", "#dd0000"], + ["40%", "#ff5500"], + ["60%", "#ffff00"], + ["80%", "#a1a841"], + ["100%", "#339933"], +].map(([width, backgroundColor]) => ({ width, backgroundColor })); + +/** + * A simple display of the password strength. + */ + +const ELEMENT = "ak-password-strength-indicator"; + +@customElement(ELEMENT) +export class PasswordStrengthIndicator extends AKElement { + static styles = styles; + + /** + * The input element to observe. Attaching this to anything other than an HTMLInputElement will + * throw an exception. + */ + @property({ attribute: true }) + src = ""; + + sourceInput?: HTMLInputElement; + + @state() + strength = LEVELS[0]; + + constructor() { + super(); + this.checkPasswordStrength = this.checkPasswordStrength.bind(this); + } + + connectedCallback() { + super.connectedCallback(); + this.input.addEventListener("keyup", this.checkPasswordStrength); + } + + disconnectedCallback() { + this.input.removeEventListener("keyup", this.checkPasswordStrength); + super.disconnectedCallback(); + } + + checkPasswordStrength() { + const { score } = zxcvbn(this.input.value); + this.strength = LEVELS[score]; + } + + get input(): HTMLInputElement { + if (this.sourceInput) { + return this.sourceInput; + } + return (this.sourceInput = findInput(this.getRootNode() as Element, ELEMENT, this.src)); + } + + render() { + return html`
+
+
`; + } +} + +export default PasswordStrengthIndicator; diff --git a/web/src/flow/stages/prompt/FieldRenderers.ts b/web/src/flow/stages/prompt/FieldRenderers.ts new file mode 100644 index 000000000..2442d8f27 --- /dev/null +++ b/web/src/flow/stages/prompt/FieldRenderers.ts @@ -0,0 +1,245 @@ +import { rootInterface } from "@goauthentik/elements/Base"; +import { LOCALES } from "@goauthentik/common/ui/locale"; +import "@goauthentik/elements/password-match-indicator"; +import "@goauthentik/elements/password-strength-indicator"; +import { unsafeHTML } from "lit/directives/unsafe-html.js"; +import { TemplateResult, html } from "lit"; +import { msg } from "@lit/localize"; +import { StagePrompt, CapabilitiesEnum, PromptTypeEnum } from "@goauthentik/api"; + +export function password(prompt: StagePrompt) { + return html``; +} + +export function repeatPassword(prompt: StagePrompt) { + return html`
+ +
`; +} + +export function renderPassword(prompt: StagePrompt) { + return /_repeat$/.test(prompt.fieldKey) ? repeatPassword(prompt) : password(prompt); +} + +export function renderText(prompt: StagePrompt) { + return html``; +} + +export function renderTextArea(prompt: StagePrompt) { + return html``; +} + +export function renderTextReadOnly(prompt: StagePrompt) { + return html``; +} + +export function renderTextAreaReadOnly(prompt: StagePrompt) { + return html``; +} + +export function renderUsername(prompt: StagePrompt) { + return html``; +} + +export function renderEmail(prompt: StagePrompt) { + return html``; +} + +export function renderNumber(prompt: StagePrompt) { + return html``; +} + +export function renderDate(prompt: StagePrompt) { + return html``; +} + +export function renderDateTime(prompt: StagePrompt) { + return html``; +} + +export function renderFile(prompt: StagePrompt) { + return html``; +} + +export function renderSeparator(prompt: StagePrompt) { + return html`${prompt.placeholder}`; +} + +export function renderHidden(prompt: StagePrompt) { + return html``; +} + +export function renderStatic(prompt: StagePrompt) { + return html`

${unsafeHTML(prompt.initialValue)}

`; +} + +export function renderDropdown(prompt: StagePrompt) { + return html``; +} + +export function renderRadioButtonGroup(prompt: StagePrompt) { + return html`${(prompt.choices || []).map((choice) => { + const id = `${prompt.fieldKey}-${choice}`; + return html`
+ + +
`; + })}`; +} + +export function renderAkLocale(prompt: StagePrompt) { + // TODO: External reference. + const inDebug = rootInterface()?.config?.capabilities.includes(CapabilitiesEnum.CanDebug); + const locales = inDebug ? LOCALES : LOCALES.filter((locale) => locale.code !== "debug"); + const options = locales.map( + (locale) => html` ` + ); + + return html``; +} + +type Renderer = (prompt: StagePrompt) => TemplateResult; + +export const promptRenderers = new Map([ + [PromptTypeEnum.Text, renderText], + [PromptTypeEnum.TextArea, renderTextArea], + [PromptTypeEnum.TextReadOnly, renderTextReadOnly], + [PromptTypeEnum.TextAreaReadOnly, renderTextAreaReadOnly], + [PromptTypeEnum.Username, renderUsername], + [PromptTypeEnum.Email, renderEmail], + [PromptTypeEnum.Password, renderPassword], + [PromptTypeEnum.Number, renderNumber], + [PromptTypeEnum.Date, renderDate], + [PromptTypeEnum.DateTime, renderDateTime], + [PromptTypeEnum.File, renderFile], + [PromptTypeEnum.Separator, renderSeparator], + [PromptTypeEnum.Hidden, renderHidden], + [PromptTypeEnum.Static, renderStatic], + [PromptTypeEnum.Dropdown, renderDropdown], + [PromptTypeEnum.RadioButtonGroup, renderRadioButtonGroup], + [PromptTypeEnum.AkLocale, renderAkLocale], +]); + +export default promptRenderers; diff --git a/web/src/flow/stages/prompt/PromptStage.ts b/web/src/flow/stages/prompt/PromptStage.ts index 1d074b2e0..4d422e4ce 100644 --- a/web/src/flow/stages/prompt/PromptStage.ts +++ b/web/src/flow/stages/prompt/PromptStage.ts @@ -1,5 +1,3 @@ -import { LOCALES } from "@goauthentik/common/ui/locale"; -import { rootInterface } from "@goauthentik/elements/Base"; import "@goauthentik/elements/Divider"; import "@goauthentik/elements/EmptyState"; import "@goauthentik/elements/forms/FormElement"; @@ -20,13 +18,14 @@ import PFTitle from "@patternfly/patternfly/components/Title/title.css"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; import { - CapabilitiesEnum, PromptChallenge, PromptChallengeResponseRequest, PromptTypeEnum, StagePrompt, } from "@goauthentik/api"; +import promptRenderers from "./FieldRenderers"; + @customElement("ak-stage-prompt") export class PromptStage extends BaseStage { static get styles(): CSSResult[] { @@ -50,174 +49,11 @@ export class PromptStage extends BaseStage`; - case PromptTypeEnum.TextArea: - return html``; - case PromptTypeEnum.TextReadOnly: - return html``; - case PromptTypeEnum.TextAreaReadOnly: - return html``; - case PromptTypeEnum.Username: - return html``; - case PromptTypeEnum.Email: - return html``; - case PromptTypeEnum.Password: - return html``; - case PromptTypeEnum.Number: - return html``; - case PromptTypeEnum.Date: - return html``; - case PromptTypeEnum.DateTime: - return html``; - case PromptTypeEnum.File: - return html``; - case PromptTypeEnum.Separator: - return html`${prompt.placeholder}`; - case PromptTypeEnum.Hidden: - return html``; - case PromptTypeEnum.Static: - return html`

${unsafeHTML(prompt.initialValue)}

`; - case PromptTypeEnum.Dropdown: - return html``; - case PromptTypeEnum.RadioButtonGroup: - return html`${(prompt.choices || []).map((choice) => { - const id = `${prompt.fieldKey}-${choice}`; - return html`
- - -
`; - })}`; - case PromptTypeEnum.AkLocale: { - const inDebug = rootInterface()?.config?.capabilities.includes( - CapabilitiesEnum.CanDebug, - ); - const locales = inDebug - ? LOCALES - : LOCALES.filter((locale) => locale.code !== "debug"); - const options = locales.map( - (locale) => html` `, - ); - - return html``; - } - default: - return html`

invalid type '${prompt.type}'

`; + const renderer = promptRenderers.get(prompt.type); + if (!renderer) { + return html`

invalid type '${prompt.type}'

`; } + return renderer(prompt); } renderPromptHelpText(prompt: StagePrompt): TemplateResult { @@ -229,14 +65,13 @@ ${prompt.initialValue} s === prompt.type); + return !special; } renderField(prompt: StagePrompt): TemplateResult {