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 <jens@goauthentik.io> # 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
2023-06-07 12:03:40 +02:00 · 2023-06-07 12:03:40 +02:00 · f05997740f
parent 1aff300171
commit f05997740f
13 changed files with 168 additions and 109 deletions
--- 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(
--- 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`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`User`} ?required=${true} name="forUser">
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`User`} ?required=${true} name="forUser">
                <ak-search-select
                    .fetchObjects=${async (query?: string): Promise<User[]> => {
                        const args: CoreUsersListRequest = {
@ -144,7 +143,6 @@ export class ApplicationCheckAccessForm extends Form<{ forUser: number }> {
                >
                </ak-search-select>
            </ak-form-element-horizontal>
-            ${this.result ? this.renderResult() : html``}
-        </form>`;
+            ${this.result ? this.renderResult() : html``}`;
    }
 }
--- a/web/src/admin/crypto/CertificateGenerateForm.ts
+++ b/web/src/admin/crypto/CertificateGenerateForm.ts
@ -21,9 +21,12 @@ export class CertificateKeyPairForm extends Form<CertificateGenerationRequest> {
        });
    };

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Common Name`} name="commonName" ?required=${true}>
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal
+                label=${t`Common Name`}
+                name="commonName"
+                ?required=${true}
+            >
                <input type="text" class="pf-c-form-control" required />
            </ak-form-element-horizontal>
            <ak-form-element-horizontal label=${t`Subject-alt name`} name="subjectAltName">
@ -38,7 +41,6 @@ export class CertificateKeyPairForm extends Form<CertificateGenerationRequest> {
                ?required=${true}
            >
                <input class="pf-c-form-control" type="number" value="365" />
-            </ak-form-element-horizontal>
-        </form>`;
+            </ak-form-element-horizontal>`;
    }
 }
--- a/web/src/admin/flows/FlowImportForm.ts
+++ b/web/src/admin/flows/FlowImportForm.ts
@ -87,15 +87,13 @@ export class FlowImportForm extends Form<Flow> {
        `;
    }

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Flow`} name="flow">
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`Flow`} name="flow">
                <input type="file" value="" class="pf-c-form-control" />
                <p class="pf-c-form__helper-text">
                    ${t`.yaml files, which can be found on goauthentik.io and can be exported by authentik.`}
                </p>
            </ak-form-element-horizontal>
-            ${this.result ? this.renderResult() : html``}
-        </form>`;
+            ${this.result ? this.renderResult() : html``}`;
    }
 }
--- 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`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Groups to add`} name="groups">
-                <div class="pf-c-input-group">
-                    <ak-user-group-select-table
-                        .confirm=${(items: Group[]) => {
-                            this.groupsToAdd = items;
-                            this.requestUpdate();
-                            return Promise.resolve();
-                        }}
-                    >
-                        <button slot="trigger" class="pf-c-button pf-m-control" type="button">
-                            <i class="fas fa-plus" aria-hidden="true"></i>
-                        </button>
-                    </ak-user-group-select-table>
-                    <div class="pf-c-form-control">
-                        <ak-chip-group>
-                            ${this.groupsToAdd.map((group) => {
-                                return html`<ak-chip
-                                    .removable=${true}
-                                    value=${ifDefined(group.pk)}
-                                    @remove=${() => {
-                                        const idx = this.groupsToAdd.indexOf(group);
-                                        this.groupsToAdd.splice(idx, 1);
-                                        this.requestUpdate();
-                                    }}
-                                >
-                                    ${group.name}
-                                </ak-chip>`;
-                            })}
-                        </ak-chip-group>
-                    </div>
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`Groups to add`} name="groups">
+            <div class="pf-c-input-group">
+                <ak-user-group-select-table
+                    .confirm=${(items: Group[]) => {
+                        this.groupsToAdd = items;
+                        this.requestUpdate();
+                        return Promise.resolve();
+                    }}
+                >
+                    <button slot="trigger" class="pf-c-button pf-m-control" type="button">
+                        <i class="fas fa-plus" aria-hidden="true"></i>
+                    </button>
+                </ak-user-group-select-table>
+                <div class="pf-c-form-control">
+                    <ak-chip-group>
+                        ${this.groupsToAdd.map((group) => {
+                            return html`<ak-chip
+                                .removable=${true}
+                                value=${ifDefined(group.pk)}
+                                @remove=${() => {
+                                    const idx = this.groupsToAdd.indexOf(group);
+                                    this.groupsToAdd.splice(idx, 1);
+                                    this.requestUpdate();
+                                }}
+                            >
+                                ${group.name}
+                            </ak-chip>`;
+                        })}
+                    </ak-chip-group>
                </div>
-            </ak-form-element-horizontal>
-        </form> `;
+            </div>
+        </ak-form-element-horizontal>`;
    }
 }

--- a/web/src/admin/policies/PolicyTestForm.ts
+++ b/web/src/admin/policies/PolicyTestForm.ts
@ -116,9 +116,8 @@ export class PolicyTestForm extends Form<PolicyTestRequest> {
        `;
    }

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`User`} ?required=${true} name="user">
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`User`} ?required=${true} name="user">
                <ak-search-select
                    .fetchObjects=${async (query?: string): Promise<User[]> => {
                        const args: CoreUsersListRequest = {
@ -155,7 +154,6 @@ export class PolicyTestForm extends Form<PolicyTestRequest> {
                    ${t`Set custom attributes using YAML or JSON.`}
                </p>
            </ak-form-element-horizontal>
-            ${this.result ? this.renderResult() : html``}
-        </form>`;
+            ${this.result ? this.renderResult() : html``}`;
    }
 }
--- 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<PolicyTestRequest> {
        </ak-form-element-horizontal>`;
    }

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`User`} ?required=${true} name="user">
+    renderExampleButtons(): TemplateResult {
+        const header = html`<p>${t`Example context data`}</p>`;
+        switch (this.mapping?.metaModelName) {
+            case "authentik_sources_ldap.ldappropertymapping":
+                return html`${header}${this.renderExampleLDAP()}`;
+            default:
+                return html``;
+        }
+    }
+
+    renderExampleLDAP(): TemplateResult {
+        return html`
+            <button
+                class="pf-c-button pf-m-secondary"
+                role="button"
+                @click=${() => {
+                    this.request = {
+                        user: this.request?.user || 0,
+                        context: {
+                            ldap: {
+                                name: "test-user",
+                                objectSid: "S-1-5-21-2611707862-2219215769-354220275-1137",
+                                objectClass: "person",
+                                displayName: "authentik test user",
+                                sAMAccountName: "sAMAccountName",
+                                distinguishedName: "cn=user,ou=users,dc=goauthentik,dc=io",
+                            },
+                        },
+                    };
+                }}
+            >
+                ${t`Active Directory User`}
+            </button>
+            <button
+                class="pf-c-button pf-m-secondary"
+                role="button"
+                @click=${() => {
+                    this.request = {
+                        user: this.request?.user || 0,
+                        context: {
+                            ldap: {
+                                name: "test-group",
+                                objectSid: "S-1-5-21-2611707862-2219215769-354220275-1137",
+                                objectClass: "group",
+                                distinguishedName: "cn=group,ou=groups,dc=goauthentik,dc=io",
+                            },
+                        },
+                    };
+                }}
+            >
+                ${t`Active Directory Group`}
+            </button>
+        `;
+    }
+
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`User`} ?required=${true} name="user">
                <ak-search-select
                    .fetchObjects=${async (query?: string): Promise<User[]> => {
                        const args: CoreUsersListRequest = {
@ -98,7 +152,6 @@ export class PolicyTestForm extends Form<PolicyTestRequest> {
                    >>
                </ak-codemirror>
            </ak-form-element-horizontal>
-            ${this.result ? this.renderResult() : html``}
-        </form>`;
+            ${this.result ? this.renderResult() : html``}`;
    }
 }
--- 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<SAMLProvider> {
        });
    };

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Name`} ?required=${true} name="name">
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`Name`} ?required=${true} name="name">
                <input type="text" class="pf-c-form-control" required />
            </ak-form-element-horizontal>
            <ak-form-element-horizontal
@ -77,7 +76,6 @@ export class SAMLProviderImportForm extends Form<SAMLProvider> {

            <ak-form-element-horizontal label=${t`Metadata`} name="metadata">
                <input type="file" value="" class="pf-c-form-control" />
-            </ak-form-element-horizontal>
-        </form>`;
+            </ak-form-element-horizontal>`;
    }
 }
--- 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`<form class="pf-c-form pf-m-horizontal">
-            ${this.group?.isSuperuser ? html`` : html``}
+    renderInlineForm(): TemplateResult {
+        return html`${this.group?.isSuperuser ? html`` : html``}
            <ak-form-element-horizontal label=${t`Users to add`} name="users">
                <div class="pf-c-input-group">
                    <ak-group-member-select-table
@ -93,8 +92,7 @@ export class RelatedUserAdd extends Form<{ users: number[] }> {
                        </ak-chip-group>
                    </div>
                </div>
-            </ak-form-element-horizontal>
-        </form> `;
+            </ak-form-element-horizontal>`;
    }
 }

--- a/web/src/admin/users/ServiceAccountForm.ts
+++ b/web/src/admin/users/ServiceAccountForm.ts
@ -35,9 +35,8 @@ export class ServiceAccountForm extends Form<UserServiceAccountRequest> {
        this.result = undefined;
    }

-    renderRequestForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Username`} ?required=${true} name="name">
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal label=${t`Username`} ?required=${true} name="name">
                <input type="text" value="" class="pf-c-form-control" required />
                <p class="pf-c-form__helper-text">
                    ${t`User's primary identifier. 150 characters or fewer.`}
@ -78,8 +77,7 @@ export class ServiceAccountForm extends Form<UserServiceAccountRequest> {
                    value="${dateTimeLocal(new Date(Date.now() + 1000 * 60 ** 2 * 24 * 360))}"
                    class="pf-c-form-control"
                />
-            </ak-form-element-horizontal>
-        </form>`;
+            </ak-form-element-horizontal>`;
    }

    renderResponseForm(): TemplateResult {
@ -113,6 +111,6 @@ export class ServiceAccountForm extends Form<UserServiceAccountRequest> {
        if (this.result) {
            return this.renderResponseForm();
        }
-        return this.renderRequestForm();
+        return super.renderForm();
    }
 }
--- a/web/src/admin/users/UserPasswordForm.ts
+++ b/web/src/admin/users/UserPasswordForm.ts
@ -26,11 +26,13 @@ export class UserPasswordForm extends Form<UserPasswordSetRequest> {
        });
    };

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Password`} ?required=${true} name="password">
-                <input type="password" value="" class="pf-c-form-control" required />
-            </ak-form-element-horizontal>
-        </form>`;
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal
+            label=${t`Password`}
+            ?required=${true}
+            name="password"
+        >
+            <input type="password" value="" class="pf-c-form-control" required />
+        </ak-form-element-horizontal>`;
    }
 }
--- a/web/src/admin/users/UserResetEmailForm.ts
+++ b/web/src/admin/users/UserResetEmailForm.ts
@ -32,32 +32,34 @@ export class UserResetEmailForm extends Form<CoreUsersRecoveryEmailRetrieveReque
        return new CoreApi(DEFAULT_CONFIG).coreUsersRecoveryEmailRetrieve(data);
    };

-    renderForm(): TemplateResult {
-        return html`<form class="pf-c-form pf-m-horizontal">
-            <ak-form-element-horizontal label=${t`Email stage`} ?required=${true} name="emailStage">
-                <ak-search-select
-                    .fetchObjects=${async (query?: string): Promise<Stage[]> => {
-                        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;
-                    }}
-                >
-                </ak-search-select>
-            </ak-form-element-horizontal>
-        </form>`;
+    renderInlineForm(): TemplateResult {
+        return html`<ak-form-element-horizontal
+            label=${t`Email stage`}
+            ?required=${true}
+            name="emailStage"
+        >
+            <ak-search-select
+                .fetchObjects=${async (query?: string): Promise<Stage[]> => {
+                    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;
+                }}
+            >
+            </ak-search-select>
+        </ak-form-element-horizontal>`;
    }
 }
--- a/web/src/elements/forms/Form.ts
+++ b/web/src/elements/forms/Form.ts
@ -279,9 +279,23 @@ export abstract class Form<T> extends AKElement {
    }

    renderForm(): TemplateResult {
+        const inline = this.renderInlineForm();
+        if (inline) {
+            return html`<form class="pf-c-form pf-m-horizontal" @submit=${this.submit}>
+                ${inline}
+            </form>`;
+        }
        return html`<slot></slot>`;
    }

+    /**
+     * 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``;