From f05997740f9f7eaa9074d7119097a7d139a79a20 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 7 Jun 2023 12:03:40 +0200 Subject: [PATCH] ATH-01-008: fix web forms not submitting correctly when pressing enter When submitting some forms with the Enter key instead of clicking "Confirm"/etc, the form would not get submitted correctly This would in the worst case is when setting a user's password, where the new password can end up in the URL, but the password was not actually saved to the user. Signed-off-by: Jens Langhammer # Conflicts: # web/src/admin/applications/ApplicationCheckAccessForm.ts # web/src/admin/crypto/CertificateGenerateForm.ts # web/src/admin/flows/FlowImportForm.ts # web/src/admin/groups/RelatedGroupList.ts # web/src/admin/policies/PolicyTestForm.ts # web/src/admin/property-mappings/PropertyMappingTestForm.ts # web/src/admin/providers/saml/SAMLProviderImportForm.ts # web/src/admin/users/RelatedUserList.ts # web/src/admin/users/ServiceAccountForm.ts # web/src/admin/users/UserPasswordForm.ts # web/src/admin/users/UserResetEmailForm.ts --- authentik/sources/ldap/signals.py | 2 +- .../ApplicationCheckAccessForm.ts | 8 +-- .../admin/crypto/CertificateGenerateForm.ts | 12 ++-- web/src/admin/flows/FlowImportForm.ts | 8 +-- web/src/admin/groups/RelatedGroupList.ts | 66 +++++++++---------- web/src/admin/policies/PolicyTestForm.ts | 8 +-- .../PropertyMappingTestForm.ts | 63 ++++++++++++++++-- .../providers/saml/SAMLProviderImportForm.ts | 8 +-- web/src/admin/users/RelatedUserList.ts | 8 +-- web/src/admin/users/ServiceAccountForm.ts | 10 ++- web/src/admin/users/UserPasswordForm.ts | 14 ++-- web/src/admin/users/UserResetEmailForm.ts | 56 ++++++++-------- web/src/elements/forms/Form.ts | 14 ++++ 13 files changed, 168 insertions(+), 109 deletions(-) diff --git a/authentik/sources/ldap/signals.py b/authentik/sources/ldap/signals.py index a89e39077..5cfc406f0 100644 --- a/authentik/sources/ldap/signals.py +++ b/authentik/sources/ldap/signals.py @@ -63,8 +63,8 @@ def ldap_sync_password(sender, user: User, password: str, **_): if not sources.exists(): return source = sources.first() - changer = LDAPPasswordChanger(source) try: + changer = LDAPPasswordChanger(source) changer.change_password(user, password) except LDAPOperationResult as exc: Event.new( diff --git a/web/src/admin/applications/ApplicationCheckAccessForm.ts b/web/src/admin/applications/ApplicationCheckAccessForm.ts index ea5f6937c..b1ef4fb36 100644 --- a/web/src/admin/applications/ApplicationCheckAccessForm.ts +++ b/web/src/admin/applications/ApplicationCheckAccessForm.ts @@ -115,9 +115,8 @@ export class ApplicationCheckAccessForm extends Form<{ forUser: number }> { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -144,7 +143,6 @@ export class ApplicationCheckAccessForm extends Form<{ forUser: number }> { > - ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/crypto/CertificateGenerateForm.ts b/web/src/admin/crypto/CertificateGenerateForm.ts index 7ccf46f29..aedc4d0a8 100644 --- a/web/src/admin/crypto/CertificateGenerateForm.ts +++ b/web/src/admin/crypto/CertificateGenerateForm.ts @@ -21,9 +21,12 @@ export class CertificateKeyPairForm extends Form { }); }; - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` @@ -38,7 +41,6 @@ export class CertificateKeyPairForm extends Form { ?required=${true} > - - `; +
`; } } diff --git a/web/src/admin/flows/FlowImportForm.ts b/web/src/admin/flows/FlowImportForm.ts index 8f576abbf..07f004675 100644 --- a/web/src/admin/flows/FlowImportForm.ts +++ b/web/src/admin/flows/FlowImportForm.ts @@ -87,15 +87,13 @@ export class FlowImportForm extends Form { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html`

${t`.yaml files, which can be found on goauthentik.io and can be exported by authentik.`}

- ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/groups/RelatedGroupList.ts b/web/src/admin/groups/RelatedGroupList.ts index fb2429979..ca752ef09 100644 --- a/web/src/admin/groups/RelatedGroupList.ts +++ b/web/src/admin/groups/RelatedGroupList.ts @@ -46,41 +46,39 @@ export class RelatedGroupAdd extends Form<{ groups: string[] }> { return data; } - renderForm(): TemplateResult { - return html`
- -
- { - this.groupsToAdd = items; - this.requestUpdate(); - return Promise.resolve(); - }} - > - - -
- - ${this.groupsToAdd.map((group) => { - return html` { - const idx = this.groupsToAdd.indexOf(group); - this.groupsToAdd.splice(idx, 1); - this.requestUpdate(); - }} - > - ${group.name} - `; - })} - -
+ renderInlineForm(): TemplateResult { + return html` +
+ { + this.groupsToAdd = items; + this.requestUpdate(); + return Promise.resolve(); + }} + > + + +
+ + ${this.groupsToAdd.map((group) => { + return html` { + const idx = this.groupsToAdd.indexOf(group); + this.groupsToAdd.splice(idx, 1); + this.requestUpdate(); + }} + > + ${group.name} + `; + })} +
- - `; +
+
`; } } diff --git a/web/src/admin/policies/PolicyTestForm.ts b/web/src/admin/policies/PolicyTestForm.ts index 9549a6389..29b81ec0b 100644 --- a/web/src/admin/policies/PolicyTestForm.ts +++ b/web/src/admin/policies/PolicyTestForm.ts @@ -116,9 +116,8 @@ export class PolicyTestForm extends Form { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -155,7 +154,6 @@ export class PolicyTestForm extends Form { ${t`Set custom attributes using YAML or JSON.`}

- ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/property-mappings/PropertyMappingTestForm.ts b/web/src/admin/property-mappings/PropertyMappingTestForm.ts index d0108e993..c3a686f87 100644 --- a/web/src/admin/property-mappings/PropertyMappingTestForm.ts +++ b/web/src/admin/property-mappings/PropertyMappingTestForm.ts @@ -64,9 +64,63 @@ export class PolicyTestForm extends Form {
`; } - renderForm(): TemplateResult { - return html`
- + renderExampleButtons(): TemplateResult { + const header = html`

${t`Example context data`}

`; + switch (this.mapping?.metaModelName) { + case "authentik_sources_ldap.ldappropertymapping": + return html`${header}${this.renderExampleLDAP()}`; + default: + return html``; + } + } + + renderExampleLDAP(): TemplateResult { + return html` + + + `; + } + + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -98,7 +152,6 @@ export class PolicyTestForm extends Form { >> - ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/providers/saml/SAMLProviderImportForm.ts b/web/src/admin/providers/saml/SAMLProviderImportForm.ts index 4b7c47791..844941c88 100644 --- a/web/src/admin/providers/saml/SAMLProviderImportForm.ts +++ b/web/src/admin/providers/saml/SAMLProviderImportForm.ts @@ -37,9 +37,8 @@ export class SAMLProviderImportForm extends Form { }); }; - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` { - - `; + `; } } diff --git a/web/src/admin/users/RelatedUserList.ts b/web/src/admin/users/RelatedUserList.ts index d606e8a2b..11583958c 100644 --- a/web/src/admin/users/RelatedUserList.ts +++ b/web/src/admin/users/RelatedUserList.ts @@ -59,9 +59,8 @@ export class RelatedUserAdd extends Form<{ users: number[] }> { return data; } - renderForm(): TemplateResult { - return html`
- ${this.group?.isSuperuser ? html`` : html``} + renderInlineForm(): TemplateResult { + return html`${this.group?.isSuperuser ? html`` : html``}
{
-
- `; +
`; } } diff --git a/web/src/admin/users/ServiceAccountForm.ts b/web/src/admin/users/ServiceAccountForm.ts index 66af4be9b..0f1d9cc90 100644 --- a/web/src/admin/users/ServiceAccountForm.ts +++ b/web/src/admin/users/ServiceAccountForm.ts @@ -35,9 +35,8 @@ export class ServiceAccountForm extends Form { this.result = undefined; } - renderRequestForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html`

${t`User's primary identifier. 150 characters or fewer.`} @@ -78,8 +77,7 @@ export class ServiceAccountForm extends Form { value="${dateTimeLocal(new Date(Date.now() + 1000 * 60 ** 2 * 24 * 360))}" class="pf-c-form-control" /> - - `; + `; } renderResponseForm(): TemplateResult { @@ -113,6 +111,6 @@ export class ServiceAccountForm extends Form { if (this.result) { return this.renderResponseForm(); } - return this.renderRequestForm(); + return super.renderForm(); } } diff --git a/web/src/admin/users/UserPasswordForm.ts b/web/src/admin/users/UserPasswordForm.ts index b6de9eca0..4af6a7ab5 100644 --- a/web/src/admin/users/UserPasswordForm.ts +++ b/web/src/admin/users/UserPasswordForm.ts @@ -26,11 +26,13 @@ export class UserPasswordForm extends Form { }); }; - renderForm(): TemplateResult { - return html`

- - - -
`; + renderInlineForm(): TemplateResult { + return html` + + `; } } diff --git a/web/src/admin/users/UserResetEmailForm.ts b/web/src/admin/users/UserResetEmailForm.ts index e26d42738..cd9638abc 100644 --- a/web/src/admin/users/UserResetEmailForm.ts +++ b/web/src/admin/users/UserResetEmailForm.ts @@ -32,32 +32,34 @@ export class UserResetEmailForm extends Form - - => { - const args: StagesAllListRequest = { - ordering: "name", - }; - if (query !== undefined) { - args.search = query; - } - const stages = await new StagesApi(DEFAULT_CONFIG).stagesEmailList(args); - return stages.results; - }} - .groupBy=${(items: Stage[]) => { - return groupBy(items, (stage) => stage.verboseNamePlural); - }} - .renderElement=${(stage: Stage): string => { - return stage.name; - }} - .value=${(stage: Stage | undefined): string | undefined => { - return stage?.pk; - }} - > - - - `; + renderInlineForm(): TemplateResult { + return html` + => { + const args: StagesAllListRequest = { + ordering: "name", + }; + if (query !== undefined) { + args.search = query; + } + const stages = await new StagesApi(DEFAULT_CONFIG).stagesEmailList(args); + return stages.results; + }} + .groupBy=${(items: Stage[]) => { + return groupBy(items, (stage) => stage.verboseNamePlural); + }} + .renderElement=${(stage: Stage): string => { + return stage.name; + }} + .value=${(stage: Stage | undefined): string | undefined => { + return stage?.pk; + }} + > + + `; } } diff --git a/web/src/elements/forms/Form.ts b/web/src/elements/forms/Form.ts index 5b80fe9ac..4d0c6e948 100644 --- a/web/src/elements/forms/Form.ts +++ b/web/src/elements/forms/Form.ts @@ -279,9 +279,23 @@ export abstract class Form extends AKElement { } renderForm(): TemplateResult { + const inline = this.renderInlineForm(); + if (inline) { + return html`
+ ${inline} +
`; + } return html``; } + /** + * Inline form render callback when inheriting this class, should be overwritten + * instead of `this.renderForm` + */ + renderInlineForm(): TemplateResult | undefined { + return undefined; + } + renderNonFieldErrors(): TemplateResult { if (!this.nonFieldErrors) { return html``;