From fa661956194f673aa656f48a8477d04795f6265c Mon Sep 17 00:00:00 2001 From: Teffen Ellis <592134+GirlBossRush@users.noreply.github.com> Date: Wed, 28 May 2025 13:08:09 +0200 Subject: [PATCH] web: Controller refinements, error handling (#14700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * web: Partial fix for issue where config is not consistently available. * web: Fix issues surrounding controller readiness. * web: Catch abort errors when originating when wrapped by OpenAPI or Sentry. * web: Fix color on dark mode. --------- Co-authored-by: Simonyi Gergő --- web/src/admin/common/ak-license-notice.ts | 3 ++ web/src/common/errors/network.ts | 13 +++++++ web/src/elements/AuthenticatedInterface.ts | 2 +- web/src/elements/Interface.ts | 3 +- web/src/elements/ak-mdx/ak-mdx.tsx | 2 +- .../controllers/BrandContextController.ts | 8 ++-- .../controllers/ConfigContextController.ts | 18 ++++----- ...troller.ts => LicenseContextController.ts} | 8 ++-- .../controllers/VersionContextController.ts | 8 ++-- web/src/elements/forms/ModalForm.ts | 3 ++ web/src/elements/mixins/branding.ts | 34 ++++++++++++----- web/src/elements/mixins/capabilities.ts | 38 ++++++++++--------- web/src/elements/mixins/config.ts | 17 ++++----- web/src/elements/mixins/license.ts | 34 ++++++++++------- web/src/elements/mixins/version.ts | 15 ++------ web/src/flow/FlowExecutor.ts | 2 +- 16 files changed, 122 insertions(+), 86 deletions(-) rename web/src/elements/controllers/{EnterpriseContextController.ts => LicenseContextController.ts} (91%) diff --git a/web/src/admin/common/ak-license-notice.ts b/web/src/admin/common/ak-license-notice.ts index 2fbc15710a..81829be442 100644 --- a/web/src/admin/common/ak-license-notice.ts +++ b/web/src/admin/common/ak-license-notice.ts @@ -1,3 +1,4 @@ +import { $PFBase } from "#common/theme"; import { WithLicenseSummary } from "#elements/mixins/license"; import "@goauthentik/elements/Alert"; import { AKElement } from "@goauthentik/elements/Base"; @@ -8,6 +9,8 @@ import { customElement, property } from "lit/decorators.js"; @customElement("ak-license-notice") export class AkLicenceNotice extends WithLicenseSummary(AKElement) { + static styles = [$PFBase]; + @property() notice = msg("Enterprise only"); diff --git a/web/src/common/errors/network.ts b/web/src/common/errors/network.ts index 7f78deeac0..ad123abaec 100644 --- a/web/src/common/errors/network.ts +++ b/web/src/common/errors/network.ts @@ -57,6 +57,19 @@ export function isAbortError(error: unknown): error is AbortErrorLike { return error instanceof DOMException && error.name === "AbortError"; } +/** + * Type predicate to check if an error originates from an aborted request. + * + * @see {@linkcode isAbortError} for the underlying implementation. + */ +export function isCausedByAbortError(error: unknown): error is AbortErrorLike { + return ( + error instanceof Error && + // --- + (isAbortError(error) || isAbortError(error.cause)) + ); +} + //#endregion //#region API diff --git a/web/src/elements/AuthenticatedInterface.ts b/web/src/elements/AuthenticatedInterface.ts index 3093b3d91c..2a38fde5e3 100644 --- a/web/src/elements/AuthenticatedInterface.ts +++ b/web/src/elements/AuthenticatedInterface.ts @@ -1,5 +1,5 @@ import { Interface } from "#elements/Interface"; -import { LicenseContextController } from "#elements/controllers/EnterpriseContextController"; +import { LicenseContextController } from "#elements/controllers/LicenseContextController"; import { VersionContextController } from "#elements/controllers/VersionContextController"; export class AuthenticatedInterface extends Interface { diff --git a/web/src/elements/Interface.ts b/web/src/elements/Interface.ts index f9f8da6d86..eb9ed49a72 100644 --- a/web/src/elements/Interface.ts +++ b/web/src/elements/Interface.ts @@ -4,14 +4,13 @@ import { AKElement } from "#elements/Base"; import { BrandingContextController } from "#elements/controllers/BrandContextController"; import { ConfigContextController } from "#elements/controllers/ConfigContextController"; import { ModalOrchestrationController } from "#elements/controllers/ModalOrchestrationController"; -import { WithAuthentikConfig } from "#elements/mixins/config"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; /** * The base interface element for the application. */ -export abstract class Interface extends WithAuthentikConfig(AKElement) { +export abstract class Interface extends AKElement { static styles = [PFBase]; constructor() { diff --git a/web/src/elements/ak-mdx/ak-mdx.tsx b/web/src/elements/ak-mdx/ak-mdx.tsx index ad58f1c314..0d7ea57bac 100644 --- a/web/src/elements/ak-mdx/ak-mdx.tsx +++ b/web/src/elements/ak-mdx/ak-mdx.tsx @@ -54,7 +54,7 @@ const highlightThemeOptions: HighlightOptions = { export type Replacer = (input: string) => string; @customElement("ak-mdx") -export class AKMDX extends WithAuthentikConfig(AKElement) { +export class AKMDX extends AKElement { @property({ reflect: true, }) diff --git a/web/src/elements/controllers/BrandContextController.ts b/web/src/elements/controllers/BrandContextController.ts index 4a36758548..dd7ebe4036 100644 --- a/web/src/elements/controllers/BrandContextController.ts +++ b/web/src/elements/controllers/BrandContextController.ts @@ -1,10 +1,10 @@ import { DEFAULT_CONFIG } from "#common/api/config"; import { EVENT_REFRESH } from "#common/constants"; -import { isAbortError } from "#common/errors/network"; +import { isCausedByAbortError } from "#common/errors/network"; import { BrandingContext, BrandingMixin } from "#elements/mixins/branding"; import type { ReactiveElementHost } from "#elements/types"; -import { Context, ContextProvider } from "@lit/context"; +import { ContextProvider } from "@lit/context"; import type { ReactiveController } from "lit"; import { CoreApi, CurrentBrand } from "@goauthentik/api"; @@ -14,7 +14,7 @@ export class BrandingContextController implements ReactiveController { #abortController: null | AbortController = null; #host: ReactiveElementHost; - #context: ContextProvider>; + #context: ContextProvider; constructor(host: ReactiveElementHost, initialValue: CurrentBrand) { this.#host = host; @@ -42,7 +42,7 @@ export class BrandingContextController implements ReactiveController { }) .catch((error: unknown) => { - if (isAbortError(error)) { + if (isCausedByAbortError(error)) { this.#log("Aborted fetching brand"); return; } diff --git a/web/src/elements/controllers/ConfigContextController.ts b/web/src/elements/controllers/ConfigContextController.ts index 0f45397097..11386e509e 100644 --- a/web/src/elements/controllers/ConfigContextController.ts +++ b/web/src/elements/controllers/ConfigContextController.ts @@ -1,10 +1,10 @@ import { DEFAULT_CONFIG } from "#common/api/config"; import { EVENT_REFRESH } from "#common/constants"; -import { isAbortError } from "#common/errors/network"; -import { AKConfigMixin, AuthentikConfigContext } from "#elements/mixins/config"; +import { isCausedByAbortError } from "#common/errors/network"; +import { AKConfigMixin, AuthentikConfigContext, kAKConfig } from "#elements/mixins/config"; import type { ReactiveElementHost } from "#elements/types"; -import { Context, ContextProvider } from "@lit/context"; +import { ContextProvider } from "@lit/context"; import type { ReactiveController } from "lit"; import { Config, RootApi } from "@goauthentik/api"; @@ -17,7 +17,7 @@ export class ConfigContextController implements ReactiveController { #abortController: null | AbortController = null; #host: ReactiveElementHost; - #context: ContextProvider>; + #context: ContextProvider; constructor(host: ReactiveElementHost, initialValue: Config) { this.#host = host; @@ -27,7 +27,7 @@ export class ConfigContextController implements ReactiveController { initialValue, }); - this.#host.authentikConfig = initialValue; + this.#host[kAKConfig] = initialValue; } #fetch = () => { @@ -43,10 +43,10 @@ export class ConfigContextController implements ReactiveController { }) .then((authentikConfig) => { this.#context.setValue(authentikConfig); - this.#host.authentikConfig = authentikConfig; + this.#host[kAKConfig] = authentikConfig; }) .catch((error: unknown) => { - if (isAbortError(error)) { + if (isCausedByAbortError(error)) { this.#log("Aborted fetching configuration"); return; } @@ -72,8 +72,8 @@ export class ConfigContextController implements ReactiveController { // If the Interface changes its config information, we should notify all // users of the context of that change, without creating an infinite // loop of resets. - if (this.#host.authentikConfig && this.#host.authentikConfig !== this.#context.value) { - this.#context.setValue(this.#host.authentikConfig); + if (this.#host[kAKConfig] && this.#host[kAKConfig] !== this.#context.value) { + this.#context.setValue(this.#host[kAKConfig]); } } } diff --git a/web/src/elements/controllers/EnterpriseContextController.ts b/web/src/elements/controllers/LicenseContextController.ts similarity index 91% rename from web/src/elements/controllers/EnterpriseContextController.ts rename to web/src/elements/controllers/LicenseContextController.ts index bc4ef9e08e..82c634fbaa 100644 --- a/web/src/elements/controllers/EnterpriseContextController.ts +++ b/web/src/elements/controllers/LicenseContextController.ts @@ -1,10 +1,10 @@ import { DEFAULT_CONFIG } from "#common/api/config"; import { EVENT_REFRESH_ENTERPRISE } from "#common/constants"; -import { isAbortError } from "#common/errors/network"; +import { isCausedByAbortError } from "#common/errors/network"; import { LicenseContext, LicenseMixin } from "#elements/mixins/license"; import type { ReactiveElementHost } from "#elements/types"; -import { Context, ContextProvider } from "@lit/context"; +import { ContextProvider } from "@lit/context"; import type { ReactiveController } from "lit"; import { EnterpriseApi, LicenseSummary } from "@goauthentik/api"; @@ -14,7 +14,7 @@ export class LicenseContextController implements ReactiveController { #abortController: null | AbortController = null; #host: ReactiveElementHost; - #context: ContextProvider>; + #context: ContextProvider; constructor(host: ReactiveElementHost, initialValue?: LicenseSummary) { this.#host = host; @@ -44,7 +44,7 @@ export class LicenseContextController implements ReactiveController { }) .catch((error: unknown) => { - if (isAbortError(error)) { + if (isCausedByAbortError(error)) { this.#log("Aborted fetching license summary"); return; } diff --git a/web/src/elements/controllers/VersionContextController.ts b/web/src/elements/controllers/VersionContextController.ts index 6b1dc6a472..54209a9403 100644 --- a/web/src/elements/controllers/VersionContextController.ts +++ b/web/src/elements/controllers/VersionContextController.ts @@ -1,10 +1,10 @@ import { DEFAULT_CONFIG } from "#common/api/config"; import { EVENT_REFRESH } from "#common/constants"; -import { isAbortError } from "#common/errors/network"; +import { isCausedByAbortError } from "#common/errors/network"; import { VersionContext, VersionMixin } from "#elements/mixins/version"; import type { ReactiveElementHost } from "#elements/types"; -import { Context, ContextProvider } from "@lit/context"; +import { ContextProvider } from "@lit/context"; import type { ReactiveController } from "lit"; import { AdminApi, Version } from "@goauthentik/api"; @@ -14,7 +14,7 @@ export class VersionContextController implements ReactiveController { #abortController: null | AbortController = null; #host: ReactiveElementHost; - #context: ContextProvider>; + #context: ContextProvider; constructor(host: ReactiveElementHost, initialValue?: Version) { this.#host = host; @@ -41,7 +41,7 @@ export class VersionContextController implements ReactiveController { }) .catch((error: unknown) => { - if (isAbortError(error)) { + if (isCausedByAbortError(error)) { this.#log("Aborted fetching license summary"); return; } diff --git a/web/src/elements/forms/ModalForm.ts b/web/src/elements/forms/ModalForm.ts index 46001e3f76..ba0addbc11 100644 --- a/web/src/elements/forms/ModalForm.ts +++ b/web/src/elements/forms/ModalForm.ts @@ -37,6 +37,9 @@ export class ModalForm extends ModalButton { if (this.closeAfterSuccessfulSubmit) { this.open = false; form?.resetForm(); + + // TODO: We may be fetching too frequently. + // Repeat dispatching will prematurely abort refresh listeners and cause several fetches and re-renders. this.dispatchEvent( new CustomEvent(EVENT_REFRESH, { bubbles: true, diff --git a/web/src/elements/mixins/branding.ts b/web/src/elements/mixins/branding.ts index afca72bcf8..9b61083391 100644 --- a/web/src/elements/mixins/branding.ts +++ b/web/src/elements/mixins/branding.ts @@ -1,7 +1,7 @@ import { DefaultBrand } from "#common/ui/config"; import { createMixin } from "#elements/types"; -import { consume, createContext } from "@lit/context"; +import { Context, consume, createContext } from "@lit/context"; import type { CurrentBrand, FooterLink } from "@goauthentik/api"; @@ -16,6 +16,8 @@ export const BrandingContext = createContext( Symbol.for("authentik-branding-context"), ); +export type BrandingContext = Context; + /** * A mixin that provides the current brand to the element. * @@ -27,9 +29,30 @@ export interface BrandingMixin { */ readonly brand: Readonly; + /** + * The application title. + * + * @see {@linkcode DefaultBrand.brandingTitle} + */ readonly brandingTitle: string; + + /** + * The application logo. + * + * @see {@linkcode DefaultBrand.brandingLogo} + */ readonly brandingLogo: string; + + /** + * The application favicon. + * + * @see {@linkcode DefaultBrand.brandingFavicon} + */ readonly brandingFavicon: string; + + /** + * Footer links provided by the brand configuration. + */ readonly brandingFooterLinks: FooterLink[]; } @@ -37,18 +60,11 @@ export interface BrandingMixin { * A mixin that provides the current brand to the element. * * @category Mixin - * - * @see {@link https://lit.dev/docs/composition/mixins/#mixins-in-typescript | Lit Mixins} */ export const WithBrandConfig = createMixin( ({ - /** - * The superclass constructor to extend. - */ + // --- SuperClass, - /** - * Whether or not to subscribe to the context. - */ subscribe = true, }) => { abstract class BrandingProvider extends SuperClass implements BrandingMixin { diff --git a/web/src/elements/mixins/capabilities.ts b/web/src/elements/mixins/capabilities.ts index fd7847139b..13e6310b5b 100644 --- a/web/src/elements/mixins/capabilities.ts +++ b/web/src/elements/mixins/capabilities.ts @@ -1,5 +1,6 @@ -import { AKConfigMixin } from "#elements/mixins/config"; -import { createMixin } from "@goauthentik/elements/types"; +import { WithAuthentikConfig } from "#elements/mixins/config"; +import { kAKConfig } from "#elements/mixins/config"; +import { LitElementConstructor, createMixin } from "#elements/types"; import { CapabilitiesEnum } from "@goauthentik/api"; @@ -43,25 +44,26 @@ export interface CapabilitiesMixin { * @category Mixin * */ -export const WithCapabilitiesConfig = createMixin( - ({ SuperClass }) => { - abstract class CapabilitiesProvider extends SuperClass implements CapabilitiesMixin { - public can(capability: CapabilitiesEnum) { - const config = this.authentikConfig; +export const WithCapabilitiesConfig = createMixin(({ SuperClass }) => { + abstract class CapabilitiesProvider + extends WithAuthentikConfig(SuperClass) + implements CapabilitiesMixin + { + public can(capability: CapabilitiesEnum) { + const config = this[kAKConfig]; - if (!config) { - throw new Error( - `ConfigContext: Attempted to check capability "${capability}" before initialization. Does the element have the AuthentikConfigMixin applied?`, - ); - } - - return config.capabilities.includes(capability); + if (!config) { + throw new Error( + `CapabilitiesMixin: Attempted to check capability "${capability}" before initialization. Does the element have the AuthentikConfigMixin applied?`, + ); } - } - return CapabilitiesProvider; - }, -); + return config.capabilities.includes(capability); + } + } + + return CapabilitiesProvider; +}); // Re-export `CapabilitiesEnum`, so you won't have to import it on a separate line if you // don't need anything else from the API. diff --git a/web/src/elements/mixins/config.ts b/web/src/elements/mixins/config.ts index d96e45bdce..775ad09475 100644 --- a/web/src/elements/mixins/config.ts +++ b/web/src/elements/mixins/config.ts @@ -1,9 +1,11 @@ import { createMixin } from "@goauthentik/elements/types"; -import { consume, createContext } from "@lit/context"; +import { Context, consume, createContext } from "@lit/context"; import type { Config } from "@goauthentik/api"; +export const kAKConfig = Symbol("kAKConfig"); + /** * The Lit context for the application configuration. * @@ -13,6 +15,8 @@ import type { Config } from "@goauthentik/api"; */ export const AuthentikConfigContext = createContext(Symbol.for("authentik-config-context")); +export type AuthentikConfigContext = Context; + /** * A consumer that provides the application configuration to the element. * @@ -23,7 +27,7 @@ export interface AKConfigMixin { /** * The current configuration of the application. */ - readonly authentikConfig: Readonly; + readonly [kAKConfig]: Readonly; } /** @@ -33,13 +37,8 @@ export interface AKConfigMixin { */ export const WithAuthentikConfig = createMixin( ({ - /** - * The superclass constructor to extend. - */ + // --- SuperClass, - /** - * Whether or not to subscribe to the context. - */ subscribe = true, }) => { abstract class AKConfigProvider extends SuperClass implements AKConfigMixin { @@ -47,7 +46,7 @@ export const WithAuthentikConfig = createMixin( context: AuthentikConfigContext, subscribe, }) - public readonly authentikConfig!: Readonly; + public readonly [kAKConfig]!: Readonly; } return AKConfigProvider; diff --git a/web/src/elements/mixins/license.ts b/web/src/elements/mixins/license.ts index 22fcd5b04d..8577f7fdf4 100644 --- a/web/src/elements/mixins/license.ts +++ b/web/src/elements/mixins/license.ts @@ -1,6 +1,6 @@ import { createMixin } from "#elements/types"; -import { consume, createContext } from "@lit/context"; +import { Context, consume, createContext } from "@lit/context"; import { type LicenseSummary, LicenseSummaryStatusEnum } from "@goauthentik/api"; @@ -8,6 +8,8 @@ export const LicenseContext = createContext( Symbol.for("authentik-license-context"), ); +export type LicenseContext = Context; + /** * A consumer that provides license information to the element. */ @@ -26,18 +28,24 @@ export interface LicenseMixin { /** * A mixin that provides the license information to the element. */ -export const WithLicenseSummary = createMixin(({ SuperClass, subscribe = true }) => { - abstract class LicenseProvider extends SuperClass implements LicenseMixin { - @consume({ - context: LicenseContext, - subscribe, - }) - public readonly licenseSummary!: LicenseSummary; +export const WithLicenseSummary = createMixin( + ({ + // --- + SuperClass, + subscribe = true, + }) => { + abstract class LicenseProvider extends SuperClass implements LicenseMixin { + @consume({ + context: LicenseContext, + subscribe, + }) + public readonly licenseSummary!: LicenseSummary; - get hasEnterpriseLicense() { - return this.licenseSummary?.status !== LicenseSummaryStatusEnum.Unlicensed; + get hasEnterpriseLicense() { + return this.licenseSummary?.status !== LicenseSummaryStatusEnum.Unlicensed; + } } - } - return LicenseProvider; -}); + return LicenseProvider; + }, +); diff --git a/web/src/elements/mixins/version.ts b/web/src/elements/mixins/version.ts index 9d88c451d9..446fb1a1c1 100644 --- a/web/src/elements/mixins/version.ts +++ b/web/src/elements/mixins/version.ts @@ -1,7 +1,6 @@ import { createMixin } from "#elements/types"; -import { consume, createContext } from "@lit/context"; -import { state } from "lit/decorators.js"; +import { Context, consume, createContext } from "@lit/context"; import type { Version } from "@goauthentik/api"; @@ -15,6 +14,8 @@ import type { Version } from "@goauthentik/api"; export const VersionContext = createContext(Symbol.for("authentik-version-context")); +export type VersionContext = Context; + /** * A mixin that provides the current version to the element. * @@ -33,18 +34,11 @@ export interface VersionMixin { * A mixin that provides the current authentik version to the element. * * @category Mixin - * - * @see {@link https://lit.dev/docs/composition/mixins/#mixins-in-typescript | Lit Mixins} */ export const WithVersion = createMixin( ({ - /** - * The superclass constructor to extend. - */ + // --- SuperClass, - /** - * Whether or not to subscribe to the context. - */ subscribe = true, }) => { abstract class VersionProvider extends SuperClass implements VersionMixin { @@ -52,7 +46,6 @@ export const WithVersion = createMixin( context: VersionContext, subscribe, }) - @state() public version!: Version; } diff --git a/web/src/flow/FlowExecutor.ts b/web/src/flow/FlowExecutor.ts index 3da324197c..efba4f8bb5 100644 --- a/web/src/flow/FlowExecutor.ts +++ b/web/src/flow/FlowExecutor.ts @@ -519,7 +519,7 @@ export class FlowExecutor class="pf-c-login__main-header pf-c-brand ak-brand" > ${msg(