Skip to content

Commit

Permalink
web/admin: add application bindings to the application wizard (#11462)
Browse files Browse the repository at this point in the history
* web: fix Flash of Unstructured Content while SearchSelect is loading from the backend

Provide an alternative, readonly, disabled, unindexed input object with the text "Loading...", to be
replaced with the _real_ input element after the content is loaded.

This provides the correct appearance and spacing so the content doesn't jiggle about between the
start of loading and the SearchSelect element being finalized.  It was visually distracting and
unappealing.

* web: comment on state management in API layer, move file to point to correct component under test.

* web: test for flash of unstructured content

- Add a unit test to ensure the "Loading..." element is displayed correctly before data arrives
- Demo how to mock a `fetchObjects()` call in testing. Very cool.
- Make distinguishing rule sets for code, tests, and scripts in nightmare mode
- In SearchSelect, Move the `styles()` declaration to the top of the class for consistency.

- To test for the FLOUC issue in SearchSelect.

This is both an exercise in mocking @BeryJu's `fetchObjects()` protocol, and shows how we can unit
test generic components that render API objects.

* web: interim commit of the basic sortable & selectable table.

* web: added basic unit testing to API-free tables

Mostly these tests assert that the table renders and that the content we give it
is where we expect it to be after sorting. For select tables, it also asserts that
the overall value of the table is what we expect it to be when we click on a
single row, or on the "select all" button.

* web: finalize testing for tables

Includes documentation updates and better tests for select-table.

* Provide unit test accessibility to Firefox and Safari; wrap calls to manipulate test DOMs directly in a browser.exec call so they run in the proper context and be await()ed properly

* web: repeat is needed to make sure sub-elements move around correctly. Map does not do full tracking.

* web: Update HorizontalLightComponent to accurately convey its value "upwards."

* interim commit, gods, the CSS is finally working.

* web: update

Got the binding editor in.  The tests complete.  Removed sonarjs.

* web: fixed tests to complete.

* web: fixed round-trip between binding list and binding editor. Fixed 'delete'.  TODO: Fix error reporting on home page, the edit button is ugly, and the height is off somehow, but I'm not yet sure how. I just know it bugs my eyes.

* core: add support to set policy bindings in transactional endpoint

Signed-off-by: Jens Langhammer <[email protected]>

* improve permission checks

especially since we'll be using the wizard as default in the future, it shouldn't be superuser only

Signed-off-by: Jens Langhammer <[email protected]>

* web: update api-less tables

- Replace `th` with `td` in `thead` components. Because Patternfly.
- Add @BeryJu's styling to the tables, which make it much better looking

* web: wizard for applications, now with bindings!

- Add policy bindings to the application wizard

- Restructures the Wizard base code.
  - ak-wizard-steps holds the steps and listens for NavigationRequest events to move
    from one step to the next.
  - WizardStep is a base class (no component registration provided) that provides the *whole frame*,
    not just the form.  It receives the navigation content for the sidebar from ak-wizard-steps,
    and provides the styling for the header, footer, sidebar, and main form.  It has abstractions
    for `buttons`, `renderMain()`, `handleButton()`, `handleEnable()`, in a section well-marked as
    "Public API".  Steps inherit from this class.

Conceptually:

- A wizard is a series of pages ("steps") with a distinct beginning and end, linked in a series,
  to complete a task.
- Later steps in the series are inaccessible until an earlier steps has granted access to it.
- Access is predicated on the earlier step being complete and valid. The developer is responsible
  for determining what "complete and valid" means.
- The series is visible, giving the customer a sense of how much effort is needed to complete the
  task.
- A parent object maintains (and can modify as needed) the list of steps. It *can* maintain the
  information being collected from the user. Alternatively, that information can be kept in each
  step.

Details:

- Keeping with the Lit paradigm, "requests to change the system flow up, information changed by
  valid requests flows down."
- The information flows up using events: WizardNavigation, WizardUpdate, WizardClose.
- The information flows down using properties.

- ak-application-wizard-main holds the list of steps, providing a unique slot name for each.
  - It maintains the ApplicationWizardState object.
- ApplicationWizardStep inherits from WizardStep and provides:
  - A means of extraction information from forms
  - A convenience method for updating the ApplicationWizardState object, enabling future steps, and
    navigating to a future step, in the correct order.
  - A method for cleaning error from the error reporting mechanism as the user navigates from an
    error-handling state.
  - The title, description, and cancelability of the wizard.
- Steps:
  - step: Handles the application. A good starting point for understanding the point of
    the Wizard.  Check the `handleButton()` method to understand how we enable or disable access to
    future steps.
  - provider-choice: Just a list. Shows validation without the form.
  - provider: Uses a *very* esoteric Lit feature, `unsafeStaticTag`, which enables
    the display to show anything that conforms to the expectations of ApplicationWizardProviderForm.
    - ApplicationWizardProviderForm repeats some of the base of ApplicationWizardStep, but allows us
      to provide multiple variants on a single form without having to create separate steps for each
      form.
    - The forms (`provider-for-ldap`, `provider-for-radius`) are therefore *just* the form and any
      fetchers needed to populate it.
  - bindings: Shows the table of bindings.  Has a custom display for "This table is empty."
  - edit-binding: Showcase for the `SearchSelectEZ` configuration format. Has an override on the
    `handleButton` feature to figure out which binding is about to be overridden. Is also a
    `.hidden` page; it doesn't show up on the navigation sidebar, as is only navigable-to by buttons
    not associated with the button bar at the bottom.
  - submit: Has a lot of machinery of state: Reviewing with errors, reviewing without errors,
    running submission, and success. Uses `ts-pattern` a lot to make sure the state/request pairs
    make sense.

The key insight is that, even though a wizard is a series in order, that order can't be simply
maintained in a list. The parent needs various strategies for swapping pages in and out of the
sequence, while still maintaining a coherent idea of "flow" and providing the visual cues the user
needs to feel confident that the work can be completed and completed quickly. The entire mechanism
for using an array and index to navigate, with index numbering, blocked the implementation of the
bindings pages.

One thing led to another.  *Sigh*  Really wish this hadn't been as much of a mess as it turned out.
The end result is pretty good, though.  Definitely re-usable.

One important feature to note is that the wizard is *not* tied to the ModalButton object; it's
simply embedded in a modal as-needed.  This allows us to use wizards in other places, such as just
being in a DIV, or just a page on its own.

* web: rollback dependabot "upgrade" that broke testing

Dependabot rolled us into WebdriverIO 9.  While that's probably the
right thing to do, right now it breaks out end-to-end tests badly.
Dependabot's mucking with infrastructure should not be taken lightly,
especially in cases when the infrastructure is for DX, not UX, and
doesn't create a bigger attack surface on the running product.

* web: small fixes for wdio and lint

- Roll back another dependabot breaking change, this time to WebdriverIO
- Remove the redundant scripts wrapping ESLint for Precommit mode. Access to those modes is
  available through the flags to the `./web/scripts/eslint.mjs` script.
- Remove SonarJS checks until SonarJS is ESLint 9 compatible.
- Minor nitpicking.

* web: not sure where all these getElement() additions come from; did I add them?  Anyway, they were breaking the tests, they're a Wdio9-ism.

* package-lock.json update

* web: small fixes for wdio and lint

**PLEASE** Stop trying to upgrade WebdriverIO following Dependabot's instructions. The changes
between wdio8 and wdio9 are extensive enough to require a lot more manual intervention. The unit
tests fail in wdio 9, with the testbed driver Wdio uses to compile content to push to the browser
([vite](https://vitejs.dev) complaining:

```
2024-09-27T15:30:03.672Z WARN @wdio/browser-runner:vite: warning: Unrecognized default export in file /Users/ken/projects/dev/web/node_modules/@patternfly/patternfly/components/Dropdown/dropdown.css
  Plugin: postcss-lit
  File: /Users/ken/projects/dev/web/node_modules/@patternfly/patternfly/components/Dropdown/dropdown.css
[0-6] 2024-09-27T15:30:04.083Z INFO webdriver: BIDI COMMAND script.callFunction {"functionDeclaration":"<Function[976 bytes]>","awaitPromise":true,"arguments":[],"target":{"context":"8E608E6D13E355DFFC28112C236B73AF"}}
[0-6]  Error:  Test failed due to following error(s):
  - ak-search-select.test.ts: The requested module '/src/common/styles/authentik.css' does not provide an export named 'default': SyntaxError: The requested module '/src/common/styles/authentik.css' does not provide an export named 'default'

```

So until we can figure out why the Vite installation isn't liking our CSS import scheme, we'll
have to soldier on with what we have.  At least with Wdio 8, we get:

```
Spec Files:      7 passed, 7 total (100% completed) in 00:00:19
```

* Forgot to run prettier.

* web: small fixes for elements and forms

- provides a new utility, `_isSlug_`, used to verify a user input
- extends the ak-horizontal-component wrapper to have a stronger identity and available value
- updates the types that use the wrapper to be typed more strongly
  - (Why) The above are used in the wizard to get and store values
- fixes a bug in SearchSelectEZ that broke the display if the user didn't supply a `groupBy` field.
- Adds `@wdio/types` to the package file so eslint is satisfied wdio builds correctly
- updates the end-to-end test to understand the revised button identities on the login page
  - Running the end-to-end tests verifies that changes to the components listed above did not break
    the semantics of those components.

* Prettier had opinions

* Fix the oauth2 provider test.

* web: fix oauth2 provider.  Fix resolutions in package-lock.json

* Provide an error field for the form errors on the OAuth2 form.  Unfortunately, this does not solve the general problem that we have a UX issue with which stage bindings to show where now that we've introduced the Invalidation Stage.

* web: Add InvalidationFlow to Radius Provider dialogues

## What

- Bugfix: adds the InvalidationFlow to the Radius Provider dialogues
  - Repairs: `{"invalidation_flow":["This field is required."]}` message, which was *not* propagated
    to the Notification.
- Nitpick: Pretties `?foo=${true}` expressions: `s/\?([^=]+)=\$\{true\}/\1/`

## Note

Yes, I know I'm going to have to do more magic when we harmonize the forms, and no, I didn't add the
Property Mappings to the wizard, and yes, I know I'm going to have pain with the *new* version of
the wizard. But this is a serious bug; you can't make Radius servers with *either* of the current
dialogues at the moment.

* web/admin: provide default invalidation flows for LDAP provider.

* admin/web: the default invalidation flows for LDAP and Radius are different from the others.

* Updating the SAML Wizard page to correspond to the provider page.  *This is an intermediate fix to get the tests passing. It will probably be mooted with the next revision.*

* Making progress...

* web/admin: provider formectomy complete

* fix minor issues

Signed-off-by: Jens Langhammer <[email protected]>

* custom ordering for provider types

Signed-off-by: Jens Langhammer <[email protected]>

* fix css

Signed-off-by: Jens Langhammer <[email protected]>

* fix missing PFBase causing wrong font

Signed-off-by: Jens Langhammer <[email protected]>

* fix missing card for type select

Signed-off-by: Jens Langhammer <[email protected]>

* fix padding on last page

Signed-off-by: Jens Langhammer <[email protected]>

* add card to bindings

Signed-off-by: Jens Langhammer <[email protected]>

* web/element/wizard: fix the CSS cascade so the modifications to the title display don't affect the wiard header.

* web/elements/wizard: fix logic on unavailable / available / current indicators in nav bar.

* Debugging code is not needed.

* web: small visual fixes

As requested by reviewers:

- Fixed the height to 75% of the viewport
- Put 1rem of whitespace between the hint label and the Wizard startup button.

* web: disable lint check for cAsEfUnNy AtTrIbUtE nAmEs.

* Apply suggestions from code review

Co-authored-by: Marc 'risson' Schmitt <[email protected]>
Signed-off-by: Jens L. <[email protected]>

* rework title

Signed-off-by: Jens Langhammer <[email protected]>

* format

Signed-off-by: Jens Langhammer <[email protected]>

---------

Signed-off-by: Jens Langhammer <[email protected]>
Signed-off-by: Jens L. <[email protected]>
Co-authored-by: Jens Langhammer <[email protected]>
Co-authored-by: Marc 'risson' Schmitt <[email protected]>
  • Loading branch information
3 people authored Dec 18, 2024
1 parent 821f06f commit c528a6c
Show file tree
Hide file tree
Showing 99 changed files with 6,368 additions and 6,018 deletions.
1 change: 1 addition & 0 deletions web/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export default [
},
files: ["src/**"],
rules: {
"lit/attribute-names": "off",
// "lit/attribute-names": "error",
"lit/no-private-properties": "error",
// "lit/prefer-nothing": "warn",
Expand Down
1,487 changes: 1,137 additions & 350 deletions web/package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@
"@types/showdown": "^2.0.6",
"@typescript-eslint/eslint-plugin": "^8.8.0",
"@typescript-eslint/parser": "^8.8.0",
"@wdio/browser-runner": "^9.1.2",
"@wdio/cli": "^9.1.2",
"@wdio/browser-runner": "9.4",
"@wdio/cli": "9.4",
"@wdio/spec-reporter": "^9.1.2",
"chokidar": "^4.0.1",
"chromedriver": "^131.0.1",
Expand Down
9 changes: 9 additions & 0 deletions web/scripts/eslint.nightmare.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ const MAX_DEPTH = 4;
const MAX_NESTED_CALLBACKS = 4;
const MAX_PARAMS = 5;

// Waiting for SonarJS to be compatible
// const MAX_COGNITIVE_COMPLEXITY = 9;

const rules = {
"accessor-pairs": "error",
"array-callback-return": "error",
Expand Down Expand Up @@ -126,6 +129,11 @@ const rules = {

"no-unused-vars": "off",
"no-console": ["error", { allow: ["debug", "warn", "error"] }],
// SonarJS is not yet compatible with ESLint 9. Commenting these out
// until it is.
// "sonarjs/cognitive-complexity": ["off", MAX_COGNITIVE_COMPLEXITY],
// "sonarjs/no-duplicate-string": "off",
// "sonarjs/no-nested-template-literals": "off",
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/no-unused-vars": [
"error",
Expand Down Expand Up @@ -162,6 +170,7 @@ export default [
wcconf.configs["flat/recommended"],
litconf.configs["flat/recommended"],
...tseslint.configs.recommended,
// sonar.configs.recommended,
{
languageOptions: {
parser: tsparser,
Expand Down
6 changes: 6 additions & 0 deletions web/scripts/eslint.precommit.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default [
wcconf.configs["flat/recommended"],
litconf.configs["flat/recommended"],
...tseslint.configs.recommended,
// sonar.configs.recommended,
{
languageOptions: {
parser: tsparser,
Expand All @@ -41,6 +42,11 @@ export default [
rules: {
"no-unused-vars": "off",
"no-console": ["error", { allow: ["debug", "warn", "error"] }],
// SonarJS is not yet compatible with ESLint 9. Commenting these out
// until it is.
// "sonarjs/cognitive-complexity": ["off", 9],
// "sonarjs/no-duplicate-string": "off",
// "sonarjs/no-nested-template-literals": "off",
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/no-unused-vars": [
"error",
Expand Down
17 changes: 2 additions & 15 deletions web/src/admin/applications/ApplicationForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,12 @@ import { TemplateResult, html } from "lit";
import { customElement, property, state } from "lit/decorators.js";
import { ifDefined } from "lit/directives/if-defined.js";

import { Application, CoreApi, PolicyEngineMode, Provider } from "@goauthentik/api";
import { Application, CoreApi, Provider } from "@goauthentik/api";

import { policyOptions } from "./PolicyOptions.js";
import "./components/ak-backchannel-input";
import "./components/ak-provider-search-input";

export const policyOptions = [
{
label: "any",
value: PolicyEngineMode.Any,
default: true,
description: html`${msg("Any policy must match to grant access")}`,
},
{
label: "all",
value: PolicyEngineMode.All,
description: html`${msg("All policies must match to grant access")}`,
},
];

@customElement("ak-application-form")
export class ApplicationForm extends WithCapabilitiesConfig(ModelForm<Application, string>) {
constructor() {
Expand Down
21 changes: 15 additions & 6 deletions web/src/admin/applications/ApplicationWizardHint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { styleMap } from "lit/directives/style-map.js";
import PFButton from "@patternfly/patternfly/components/Button/button.css";
import PFLabel from "@patternfly/patternfly/components/Label/label.css";
import PFPage from "@patternfly/patternfly/components/Page/page.css";
import PFBase from "@patternfly/patternfly/patternfly-base.css";

const closeButtonIcon = html`<svg
fill="currentColor"
Expand All @@ -37,6 +38,7 @@ const closeButtonIcon = html`<svg
export class AkApplicationWizardHint extends AKElement implements ShowHintControllerHost {
static get styles() {
return [
PFBase,
PFButton,
PFPage,
PFLabel,
Expand All @@ -45,6 +47,9 @@ export class AkApplicationWizardHint extends AKElement implements ShowHintContro
padding-top: 0;
padding-bottom: 0;
}
.ak-hint-text {
padding-bottom: var(--pf-global--spacer--md);
}
`,
];
}
Expand Down Expand Up @@ -101,16 +106,20 @@ export class AkApplicationWizardHint extends AKElement implements ShowHintContro
return html` <section class="pf-c-page__main-section pf-m-no-padding-mobile">
<ak-hint>
<ak-hint-body>
<p>
<p class="ak-hint-text">
You can now configure both an application and its authentication provider at
the same time with our new Application Wizard.
<!-- <a href="(link to docs)">Learn more about the wizard here.</a> -->
</p>
<ak-application-wizard
.open=${getURLParam("createWizard", false)}
.showButton=${false}
></ak-application-wizard>
<ak-application-wizard .open=${getURLParam("createWizard", false)}>
<button
slot="trigger"
class="pf-c-button pf-m-primary"
data-ouia-component-id="start-application-wizard"
>
${msg("Create with wizard")}
</button>
</ak-application-wizard>
</ak-hint-body>
${this.showHintController.render()}
</ak-hint>
Expand Down
18 changes: 18 additions & 0 deletions web/src/admin/applications/PolicyOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { msg } from "@lit/localize";
import { html } from "lit";

import { PolicyEngineMode } from "@goauthentik/api";

export const policyOptions = [
{
label: "any",
value: PolicyEngineMode.Any,
default: true,
description: html`${msg("Any policy must match to grant access")}`,
},
{
label: "all",
value: PolicyEngineMode.All,
description: html`${msg("All policies must match to grant access")}`,
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import PFFormControl from "@patternfly/patternfly/components/FormControl/form-co
import PFInputGroup from "@patternfly/patternfly/components/InputGroup/input-group.css";
import PFRadio from "@patternfly/patternfly/components/Radio/radio.css";
import PFSwitch from "@patternfly/patternfly/components/Switch/switch.css";
import PFWizard from "@patternfly/patternfly/components/Wizard/wizard.css";
import PFBase from "@patternfly/patternfly/patternfly-base.css";

export const styles = [
Expand All @@ -20,6 +21,7 @@ export const styles = [
PFInputGroup,
PFFormControl,
PFSwitch,
PFWizard,
css`
select[multiple] {
height: 15em;
Expand Down
86 changes: 86 additions & 0 deletions web/src/admin/applications/wizard/ApplicationWizardStep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { styles } from "@goauthentik/admin/applications/wizard/ApplicationWizardFormStepStyles.css.js";
import { WizardStep } from "@goauthentik/components/ak-wizard/WizardStep.js";
import {
NavigationUpdate,
WizardNavigationEvent,
WizardUpdateEvent,
} from "@goauthentik/components/ak-wizard/events";
import { KeyUnknown, serializeForm } from "@goauthentik/elements/forms/Form";
import { HorizontalFormElement } from "@goauthentik/elements/forms/HorizontalFormElement";

import { msg } from "@lit/localize";
import { property, query } from "lit/decorators.js";

import { ValidationError } from "@goauthentik/api";

import {
type ApplicationWizardState,
type ApplicationWizardStateUpdate,
ExtendedValidationError,
} from "./types";

export class ApplicationWizardStep extends WizardStep {
static get styles() {
return [...WizardStep.styles, ...styles];
}

@property({ type: Object, attribute: false })
wizard!: ApplicationWizardState;

// As recommended in [WizardStep](../../../components/ak-wizard/WizardStep.ts), we override
// these fields and provide them to all the child classes.
wizardTitle = msg("New application");
wizardDescription = msg("Create a new application");
canCancel = true;

// This should be overridden in the children for more precise targeting.
@query("form")
form!: HTMLFormElement;

get formValues(): KeyUnknown | undefined {
const elements = [
...Array.from(
this.form.querySelectorAll<HorizontalFormElement>("ak-form-element-horizontal"),
),
...Array.from(this.form.querySelectorAll<HTMLElement>("[data-ak-control=true]")),
];
return serializeForm(elements as unknown as NodeListOf<HorizontalFormElement>);
}

protected removeErrors(
keyToDelete: keyof ExtendedValidationError,
): ValidationError | undefined {
if (!this.wizard.errors) {
return undefined;
}
const empty = {};
const errors = Object.entries(this.wizard.errors).reduce(
(acc, [key, value]) =>
key === keyToDelete ||
value === undefined ||
(Array.isArray(this.wizard?.errors?.[key]) && this.wizard.errors[key].length === 0)
? acc
: { ...acc, [key]: value },
empty,
);
return errors;
}

// This pattern became visible during development, and the order is important: wizard updating
// and validation must complete before navigation is attempted.
public handleUpdate(
update?: ApplicationWizardStateUpdate,
destination?: string,
enable?: NavigationUpdate,
) {
// Inform ApplicationWizard of content state
if (update) {
this.dispatchEvent(new WizardUpdateEvent(update));
}

// Inform WizardStepManager of steps state
if (destination || enable) {
this.dispatchEvent(new WizardNavigationEvent(destination, enable));
}
}
}
72 changes: 0 additions & 72 deletions web/src/admin/applications/wizard/BasePanel.ts

This file was deleted.

6 changes: 3 additions & 3 deletions web/src/admin/applications/wizard/ContextIdentity.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createContext } from "@lit/context";

import { ApplicationWizardState } from "./types";
import { LocalTypeCreate } from "./steps/ProviderChoices.js";

export const applicationWizardContext = createContext<ApplicationWizardState>(
Symbol("ak-application-wizard-state-context"),
export const applicationWizardProvidersContext = createContext<LocalTypeCreate[]>(
Symbol("ak-application-wizard-providers-context"),
);
Loading

0 comments on commit c528a6c

Please sign in to comment.