Skip to content

Commit 05d4d20

Browse files
authored
web: Fix hidden textarea required attribute. (goauthentik#16168)
* web: Fix hidden textarea `required` attribute. * web: Fix missing flag property. * web: Clarify field error reporting.
1 parent 11efc75 commit 05d4d20

File tree

6 files changed

+65
-42
lines changed

6 files changed

+65
-42
lines changed

web/src/admin/admin-settings/AdminSettingsForm.ts

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,7 @@ const DEFAULT_REPUTATION_UPPER_LIMIT = 5;
2929

3030
@customElement("ak-admin-settings-form")
3131
export class AdminSettingsForm extends Form<SettingsRequest> {
32-
//
33-
// Custom property accessors in Lit 2 require a manual call to requestUpdate(). See:
34-
// https://lit.dev/docs/v2/components/properties/#accessors-custom
35-
//
36-
set settings(value: Settings | undefined) {
37-
this._settings = value;
38-
this.requestUpdate();
39-
}
40-
41-
@property({ type: Object })
42-
get settings() {
43-
return this._settings;
44-
}
45-
46-
private _settings?: Settings;
47-
48-
static styles: CSSResult[] = [
32+
public static styles: CSSResult[] = [
4933
...super.styles,
5034
PFList,
5135
css`
@@ -55,25 +39,35 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
5539
`,
5640
];
5741

42+
@property({ attribute: false })
43+
public settings!: Settings;
44+
5845
getSuccessMessage(): string {
5946
return msg("Successfully updated settings.");
6047
}
6148

62-
async send(data: SettingsRequest): Promise<Settings> {
49+
async send(settingsRequest: SettingsRequest): Promise<Settings> {
50+
settingsRequest.flags ??= this.settings.flags;
51+
6352
const result = await new AdminApi(DEFAULT_CONFIG).adminSettingsUpdate({
64-
settingsRequest: data,
53+
settingsRequest,
6554
});
55+
6656
this.dispatchEvent(new CustomEvent("ak-admin-setting-changed"));
57+
6758
return result;
6859
}
6960

7061
renderForm(): TemplateResult {
62+
const { settings } = this;
63+
7164
return html`
7265
<ak-text-input
7366
name="avatars"
7467
label=${msg("Avatars")}
75-
value="${ifDefined(this._settings?.avatars)}"
68+
value="${ifDefined(settings.avatars)}"
7669
input-hint="code"
70+
required
7771
.bighelp=${html`
7872
<p class="pf-c-form__helper-text">
7973
${msg(
@@ -133,27 +127,26 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
133127
)}
134128
</p>
135129
`}
136-
required
137130
>
138131
</ak-text-input>
139132
<ak-switch-input
140133
name="defaultUserChangeName"
141134
label=${msg("Allow users to change name")}
142-
?checked="${this._settings?.defaultUserChangeName}"
135+
?checked=${settings.defaultUserChangeName}
143136
help=${msg("Enable the ability for users to change their name.")}
144137
>
145138
</ak-switch-input>
146139
<ak-switch-input
147140
name="defaultUserChangeEmail"
148141
label=${msg("Allow users to change email")}
149-
?checked="${this._settings?.defaultUserChangeEmail}"
142+
?checked=${settings.defaultUserChangeEmail}
150143
help=${msg("Enable the ability for users to change their email.")}
151144
>
152145
</ak-switch-input>
153146
<ak-switch-input
154147
name="defaultUserChangeUsername"
155148
label=${msg("Allow users to change username")}
156-
?checked="${this._settings?.defaultUserChangeUsername}"
149+
?checked=${settings.defaultUserChangeUsername}
157150
help=${msg("Enable the ability for users to change their username.")}
158151
>
159152
</ak-switch-input>
@@ -162,7 +155,7 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
162155
label=${msg("Event retention")}
163156
input-hint="code"
164157
required
165-
value="${ifDefined(this._settings?.eventRetention)}"
158+
value="${ifDefined(settings.eventRetention)}"
166159
.bighelp=${html`<p class="pf-c-form__helper-text">
167160
${msg("Duration after which events will be deleted from the database.")}
168161
</p>
@@ -184,19 +177,19 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
184177
label=${msg("Reputation: lower limit")}
185178
required
186179
name="reputationLowerLimit"
187-
value="${this._settings?.reputationLowerLimit ?? DEFAULT_REPUTATION_LOWER_LIMIT}"
180+
value="${settings.reputationLowerLimit ?? DEFAULT_REPUTATION_LOWER_LIMIT}"
188181
help=${msg("Reputation cannot decrease lower than this value. Zero or negative.")}
189182
></ak-number-input>
190183
<ak-number-input
191184
label=${msg("Reputation: upper limit")}
192185
required
193186
name="reputationUpperLimit"
194-
value="${this._settings?.reputationUpperLimit ?? DEFAULT_REPUTATION_UPPER_LIMIT}"
187+
value="${settings.reputationUpperLimit ?? DEFAULT_REPUTATION_UPPER_LIMIT}"
195188
help=${msg("Reputation cannot increase higher than this value. Zero or positive.")}
196189
></ak-number-input>
197190
<ak-form-element-horizontal label=${msg("Footer links")} name="footerLinks">
198191
<ak-array-input
199-
.items=${this._settings?.footerLinks ?? []}
192+
.items=${settings.footerLinks ?? []}
200193
.newItem=${() => ({ name: "", href: "" })}
201194
.row=${(f?: FooterLink) =>
202195
akFooterLinkInput({
@@ -215,7 +208,7 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
215208
<ak-switch-input
216209
name="gdprCompliance"
217210
label=${msg("GDPR compliance")}
218-
?checked="${this._settings?.gdprCompliance}"
211+
?checked=${settings.gdprCompliance}
219212
help=${msg(
220213
"When enabled, all the events caused by a user will be deleted upon the user's deletion.",
221214
)}
@@ -224,14 +217,14 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
224217
<ak-switch-input
225218
name="impersonation"
226219
label=${msg("Impersonation")}
227-
?checked="${this._settings?.impersonation}"
220+
?checked=${settings.impersonation}
228221
help=${msg("Globally enable/disable impersonation.")}
229222
>
230223
</ak-switch-input>
231224
<ak-switch-input
232225
name="impersonationRequireReason"
233226
label=${msg("Require reason for impersonation")}
234-
?checked="${this._settings?.impersonationRequireReason}"
227+
?checked=${settings.impersonationRequireReason}
235228
help=${msg("Require administrators to provide a reason for impersonating a user.")}
236229
>
237230
</ak-switch-input>
@@ -240,7 +233,7 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
240233
label=${msg("Default token duration")}
241234
input-hint="code"
242235
required
243-
value="${ifDefined(this._settings?.defaultTokenDuration)}"
236+
value="${ifDefined(settings.defaultTokenDuration)}"
244237
.bighelp=${html`<p class="pf-c-form__helper-text">
245238
${msg("Default duration for generated tokens")}
246239
</p>
@@ -251,7 +244,7 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
251244
label=${msg("Default token length")}
252245
required
253246
name="defaultTokenLength"
254-
value="${this._settings?.defaultTokenLength ?? 60}"
247+
value="${settings.defaultTokenLength ?? 60}"
255248
help=${msg("Default length of generated tokens")}
256249
></ak-number-input>
257250
`;

web/src/admin/crypto/CertificateKeyPairForm.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@ export class CertificateKeyPairForm extends ModelForm<CertificateKeyPair, string
5353
name="certificateData"
5454
input-hint="code"
5555
placeholder="-----BEGIN CERTIFICATE-----"
56-
required
57-
?revealed=${this.instance === undefined}
56+
?required=${!this.instance}
57+
?revealed=${!this.instance}
5858
help=${msg("PEM-encoded Certificate data.")}
5959
></ak-secret-textarea-input>
6060
<ak-secret-textarea-input
6161
label=${msg("Private Key")}
6262
name="keyData"
6363
input-hint="code"
64-
?revealed=${this.instance === undefined}
64+
?revealed=${!this.instance}
6565
help=${msg(
6666
"Optional Private Key. If this is set, you can use this keypair for encryption.",
6767
)}

web/src/admin/enterprise/EnterpriseLicenseForm.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export class EnterpriseLicenseForm extends ModelForm<License, string> {
6666
</ak-form-element-horizontal>
6767
<ak-secret-textarea-input
6868
name="key"
69-
?revealed=${this.instance === undefined}
69+
?revealed=${!this.instance}
7070
label=${msg("License key")}
7171
input-hint="code"
7272
>

web/src/admin/sources/kerberos/KerberosSourceForm.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export class KerberosSourceForm extends WithCapabilitiesConfig(BaseSourceForm<Ke
259259
<ak-secret-textarea-input
260260
name="syncKeytab"
261261
label=${msg("Sync keytab")}
262-
?revealed=${this.instance === undefined}
262+
?revealed=${!this.instance}
263263
help=${msg(
264264
"Keytab used to authenticate to the KDC for syncing. Optional if Sync password or Sync credentials cache is provided. Must be base64 encoded or in the form TYPE:residual.",
265265
)}
@@ -287,7 +287,7 @@ export class KerberosSourceForm extends WithCapabilitiesConfig(BaseSourceForm<Ke
287287
<ak-secret-textarea-input
288288
name="spnegoKeytab"
289289
label=${msg("SPNEGO keytab")}
290-
?revealed=${this.instance === undefined}
290+
?revealed=${!this.instance}
291291
help=${msg(
292292
"Keytab used for SPNEGO. Optional if SPNEGO credentials cache is provided. Must be base64 encoded or in the form TYPE:residual.",
293293
)}

web/src/admin/sources/oauth/OAuthSourceForm.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,8 +442,8 @@ export class OAuthSourceForm extends WithCapabilitiesConfig(BaseSourceForm<OAuth
442442
name="consumerSecret"
443443
input-hint="code"
444444
help=${msg("Also known as Client Secret.")}
445-
required
446-
?revealed=${this.instance === undefined}
445+
?required=${!this.instance}
446+
?revealed=${!this.instance}
447447
></ak-secret-textarea-input>
448448
<ak-form-element-horizontal label=${msg("Scopes")} name="additionalScopes">
449449
<input

web/src/elements/forms/Form.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { instanceOfValidationError } from "@goauthentik/api";
1818

1919
import { snakeCase } from "change-case";
2020

21-
import { msg } from "@lit/localize";
21+
import { msg, str } from "@lit/localize";
2222
import { css, CSSResult, html, nothing, TemplateResult } from "lit";
2323
import { property, state } from "lit/decorators.js";
2424
import { ifDefined } from "lit/directives/if-defined.js";
@@ -246,6 +246,8 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
246246
return createFileMap<T>(this.shadowRoot?.querySelectorAll("ak-form-element-horizontal"));
247247
}
248248

249+
//#region Validation
250+
249251
public checkValidity(): boolean {
250252
return !!this.form?.checkValidity?.();
251253
}
@@ -261,6 +263,10 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
261263
return reportValidityDeep(form);
262264
}
263265

266+
//#endregion
267+
268+
//#region Submission
269+
264270
/**
265271
* Convert the elements of the form to JSON.[4]
266272
*/
@@ -273,6 +279,7 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
273279

274280
return serializeForm<T>(elements);
275281
}
282+
276283
/**
277284
* Serialize and send the form to the destination. The `send()` method must be overridden for
278285
* this to work. If processing the data results in an error, we catch the error, distribute
@@ -310,6 +317,8 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
310317
let errorMessage = pluckErrorDetail(error);
311318
let focused = false;
312319

320+
//#region Validation errors
321+
313322
if (instanceOfValidationError(parsedError)) {
314323
// assign all input-related errors to their elements
315324
const elements =
@@ -346,6 +355,23 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
346355

347356
if (parsedError.nonFieldErrors) {
348357
this.nonFieldErrors = parsedError.nonFieldErrors;
358+
} else if (!focused) {
359+
// It's possible that the API has returned a field error that we're
360+
// not aware of. We can still show the error message, to at least
361+
// give the user some feedback.
362+
for (const [fieldName, fieldErrors] of Object.entries(parsedError)) {
363+
if (Array.isArray(fieldErrors)) {
364+
this.nonFieldErrors = [
365+
msg(str`${fieldName}: ${fieldErrors.join(", ")}`),
366+
];
367+
break;
368+
}
369+
}
370+
371+
console.error(
372+
"authentik/forms: API rejected the form submission due to an invalid field that doesn't appear to be in the form. This is likely a bug in authentik.",
373+
parsedError,
374+
);
349375
}
350376

351377
errorMessage = msg("Invalid update request.");
@@ -357,6 +383,8 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
357383
}
358384
}
359385

386+
//#endregion
387+
360388
showMessage({
361389
message: errorMessage,
362390
level: MessageLevel.error,
@@ -369,6 +397,8 @@ export abstract class Form<T = Record<string, unknown>> extends AKElement {
369397

370398
//#endregion
371399

400+
//#endregion
401+
372402
//#region Render
373403

374404
public renderFormWrapper(): TemplateResult {

0 commit comments

Comments
 (0)