Skip to content

Commit

Permalink
web/admin: bugfix: dual select initialization revision (#12051)
Browse files Browse the repository at this point in the history
* 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.

* Start of dual select revision process.

* Progress.

* Made the RuleFormHelper's dualselect conform.

* Providers and Selectors harmonized for sources.

* web/bugfix/dual-select-full-options

# What

- Replaces the dual-select "selected" list mechanism with a more comprehensive (if computationally
  expensive) version that is correct.

# How

In the previous iteration, each dual select controller gets a *provider* and a *selector*; the
latter keeps the keys of all the objects a specific instance may have, and marks those objects as
"selected" when they appear in the dual-selects "selected" panel.

In order to distinguish between "selected on the existing instance" and "selected by the user," the
*selector* only runs at construction time, creating a unified "selected" list; this is standard and
allows for a uniform experience of adding and deleting items. Unfortunately, this means that the
"selected" items, because their displays are crafted bespoke, are only chosen from those available
at construction. If there are selected items later in the paginated collection, they will not be
marked as selected.

This defeats the purpose of having a paginated multi-select!

The correct way to do this is to retrieve every item pased to the *selector* and use the same
algorithm to craft the views in both windows.

For every instance of Dual Select with dynamic selection, the *provider* and *selector* have been
put in a separate file (usually suffixed as a `*FormHelper.ts` file); the algorithm by which an item is
crafted for use by DualSelect has been broken out into a small function (usually named
`*toSelect()`). The *provider* works as before. The *selector* takes every instance key passed to it
and runs a `Promise.allSettled(...*Retrieve({ uuid: instanceId }))` on them, mapping them onto the
`selected` collection using the same `*toSelect()`, so they resemble the possibilities in every way.

# Lessons

This exercise emphasizes just how much sheer *repetition* the Django REST API creates on the client
side.  Every Helper file is a copy-pasta of a sibling, with only a few minor changes:

- How the objects are turned into displays for DualSelect
- The type and calls being used;
- The field on which retrival is defined
- The defaulting rule.

There are 19 `*FormHelper` files, and each one is 50 lines long.  That's 950 lines of code.
Of those 950 lines of code, 874 of those lines are *complete duplicates* of those in the other
FormHelper files.  Only 76 lines are unique.

This language really needs macros.  That, or I need to seriously level up my Typescript and figure
out how to make this whole thing a lot smarter.

* order fields by field_key and order

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: Jens Langhammer <jens@goauthentik.io>
  • Loading branch information
kensternberg-authentik and BeryJu authored Dec 2, 2024
1 parent 248fcdd commit e077a5c
Show file tree
Hide file tree
Showing 47 changed files with 1,025 additions and 603 deletions.
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import "@goauthentik/admin/applications/wizard/ak-wizard-title";
import "@goauthentik/admin/common/ak-crypto-certificate-search";
import "@goauthentik/admin/common/ak-flow-search/ak-branded-flow-search";
import {
makeOAuth2PropertyMappingsSelector,
oauth2PropertyMappingsProvider,
} from "@goauthentik/admin/providers/oauth2/OAuth2PropertyMappings.js";
import {
clientTypeOptions,
issuerModeOptions,
redirectUriHelp,
subjectModeOptions,
} from "@goauthentik/admin/providers/oauth2/OAuth2ProviderForm";
import {
propertyMappingsProvider,
propertyMappingsSelector,
} from "@goauthentik/admin/providers/oauth2/OAuth2ProviderFormHelpers.js";
import {
IRedirectURIInput,
akOAuthRedirectURIInput,
} from "@goauthentik/admin/providers/oauth2/OAuth2ProviderRedirectURI";
import {
makeSourceSelector,
oauth2SourcesProvider,
oauth2SourcesSelector,
} from "@goauthentik/admin/providers/oauth2/OAuth2Sources.js";
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { ascii_letters, digits, first, randomString } from "@goauthentik/common/utils";
Expand Down Expand Up @@ -252,10 +252,8 @@ export class ApplicationWizardAuthenticationByOauth extends BaseProviderPanel {
.errorMessages=${errors?.propertyMappings ?? []}
>
<ak-dual-select-dynamic-selected
.provider=${oauth2PropertyMappingsProvider}
.selector=${makeOAuth2PropertyMappingsSelector(
provider?.propertyMappings,
)}
.provider=${propertyMappingsProvider}
.selector=${propertyMappingsSelector(provider?.propertyMappings)}
available-label=${msg("Available Scopes")}
selected-label=${msg("Selected Scopes")}
></ak-dual-select-dynamic-selected>
Expand Down Expand Up @@ -309,7 +307,7 @@ export class ApplicationWizardAuthenticationByOauth extends BaseProviderPanel {
>
<ak-dual-select-dynamic-selected
.provider=${oauth2SourcesProvider}
.selector=${makeSourceSelector(provider?.jwksSources)}
.selector=${oauth2SourcesSelector(provider?.jwksSources)}
available-label=${msg("Available Sources")}
selected-label=${msg("Selected Sources")}
></ak-dual-select-dynamic-selected>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import "@goauthentik/admin/applications/wizard/ak-wizard-title";
import {
makeSourceSelector,
oauth2SourcesProvider,
oauth2SourcesSelector,
} from "@goauthentik/admin/providers/oauth2/OAuth2Sources.js";
import {
makeProxyPropertyMappingsSelector,
proxyPropertyMappingsProvider,
} from "@goauthentik/admin/providers/proxy/ProxyProviderPropertyMappings.js";
propertyMappingsProvider,
propertyMappingsSelector,
} from "@goauthentik/admin/providers/proxy/ProxyProviderFormHelpers.js";
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { first } from "@goauthentik/common/utils";
import "@goauthentik/components/ak-switch-input";
Expand Down Expand Up @@ -147,8 +147,8 @@ export class AkTypeProxyApplicationWizardPage extends BaseProviderPanel {
name="propertyMappings"
>
<ak-dual-select-dynamic-selected
.provider=${proxyPropertyMappingsProvider}
.selector=${makeProxyPropertyMappingsSelector(
.provider=${propertyMappingsProvider}
.selector=${propertyMappingsSelector(
this.instance?.propertyMappings,
)}
available-label="${msg("Available Scopes")}"
Expand Down Expand Up @@ -248,7 +248,7 @@ export class AkTypeProxyApplicationWizardPage extends BaseProviderPanel {
>
<ak-dual-select-dynamic-selected
.provider=${oauth2SourcesProvider}
.selector=${makeSourceSelector(this.instance?.jwksSources)}
.selector=${oauth2SourcesSelector(this.instance?.jwksSources)}
available-label=${msg("Available Sources")}
selected-label=${msg("Selected Sources")}
></ak-dual-select-dynamic-selected>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import "@goauthentik/admin/applications/wizard/ak-wizard-title";
import "@goauthentik/admin/common/ak-flow-search/ak-flow-search";
import {
makeRACPropertyMappingsSelector,
racPropertyMappingsProvider,
} from "@goauthentik/admin/providers/rac/RACPropertyMappings.js";
propertyMappingsProvider,
propertyMappingsSelector,
} from "@goauthentik/admin/providers/rac/RACProviderFormHelpers.js";
import "@goauthentik/components/ak-text-input";
import "@goauthentik/elements/CodeMirror";
import "@goauthentik/elements/ak-dual-select/ak-dual-select-dynamic-selected-provider.js";
Expand Down Expand Up @@ -70,10 +70,8 @@ export class ApplicationWizardAuthenticationByRAC extends BaseProviderPanel {
name="propertyMappings"
>
<ak-dual-select-dynamic-selected
.provider=${racPropertyMappingsProvider}
.selector=${makeRACPropertyMappingsSelector(
provider?.propertyMappings,
)}
.provider=${propertyMappingsProvider}
.selector=${propertyMappingsSelector(provider?.propertyMappings)}
available-label="${msg("Available Property Mappings")}"
selected-label="${msg("Selected Property Mappings")}"
></ak-dual-select-dynamic-selected>
Expand Down
25 changes: 2 additions & 23 deletions web/src/admin/events/RuleForm.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { severityToLabel } from "@goauthentik/common/labels";
import "@goauthentik/elements/ak-dual-select/ak-dual-select-dynamic-selected-provider.js";
import { DualSelectPair } from "@goauthentik/elements/ak-dual-select/types";
import "@goauthentik/elements/forms/HorizontalFormElement";
import { ModelForm } from "@goauthentik/elements/forms/ModelForm";
import "@goauthentik/elements/forms/Radio";
Expand All @@ -18,32 +17,12 @@ import {
EventsApi,
Group,
NotificationRule,
NotificationTransport,
PaginatedNotificationTransportList,
SeverityEnum,
} from "@goauthentik/api";

async function eventTransportsProvider(page = 1, search = "") {
const eventTransports = await new EventsApi(DEFAULT_CONFIG).eventsTransportsList({
ordering: "name",
pageSize: 20,
search: search.trim(),
page,
});
import { eventTransportsProvider, eventTransportsSelector } from "./RuleFormHelpers.js";

return {
pagination: eventTransports.pagination,
options: eventTransports.results.map((transport) => [transport.pk, transport.name]),
};
}

export function makeTransportSelector(instanceTransports: string[] | undefined) {
const localTransports = instanceTransports ? new Set(instanceTransports) : undefined;

return localTransports
? ([pk, _]: DualSelectPair) => localTransports.has(pk)
: ([_0, _1, _2, stage]: DualSelectPair<NotificationTransport>) => stage !== undefined;
}
@customElement("ak-event-rule-form")
export class RuleForm extends ModelForm<NotificationRule, string> {
eventTransports?: PaginatedNotificationTransportList;
Expand Down Expand Up @@ -126,7 +105,7 @@ export class RuleForm extends ModelForm<NotificationRule, string> {
>
<ak-dual-select-dynamic-selected
.provider=${eventTransportsProvider}
.selector=${makeTransportSelector(this.instance?.transports)}
.selector=${eventTransportsSelector(this.instance?.transports)}
available-label="${msg("Available Transports")}"
selected-label="${msg("Selected Transports")}"
></ak-dual-select-dynamic-selected>
Expand Down
42 changes: 42 additions & 0 deletions web/src/admin/events/RuleFormHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { DualSelectPair } from "@goauthentik/elements/ak-dual-select/types";

import { EventsApi, NotificationTransport } from "@goauthentik/api";

const transportToSelect = (transport: NotificationTransport) => [transport.pk, transport.name];

export async function eventTransportsProvider(page = 1, search = "") {
const eventTransports = await new EventsApi(DEFAULT_CONFIG).eventsTransportsList({
ordering: "name",
pageSize: 20,
search: search.trim(),
page,
});

return {
pagination: eventTransports.pagination,
options: eventTransports.results.map(transportToSelect),
};
}

export function eventTransportsSelector(instanceTransports: string[] | undefined) {
if (!instanceTransports) {
return async (transports: DualSelectPair<NotificationTransport>[]) =>
transports.filter(
([_0, _1, _2, stage]: DualSelectPair<NotificationTransport>) => stage !== undefined,
);
}

return async () => {
const transportsApi = new EventsApi(DEFAULT_CONFIG);
const transports = await Promise.allSettled(
instanceTransports.map((instanceId) =>
transportsApi.eventsTransportsRetrieve({ uuid: instanceId }),
),
);
return transports
.filter((s) => s.status === "fulfilled")
.map((s) => s.value)
.map(transportToSelect);
};
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { BaseProviderForm } from "@goauthentik/admin/providers/BaseProviderForm";
import {
googleWorkspacePropertyMappingsProvider,
makeGoogleWorkspacePropertyMappingsSelector,
} from "@goauthentik/admin/providers/google_workspace/GoogleWorkspaceProviderPropertyMappings";
propertyMappingsProvider,
propertyMappingsSelector,
} from "@goauthentik/admin/providers/google_workspace/GoogleWorkspaceProviderFormHelpers.js";
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { first } from "@goauthentik/common/utils";
import "@goauthentik/elements/CodeMirror";
Expand Down Expand Up @@ -224,8 +224,8 @@ export class GoogleWorkspaceProviderFormPage extends BaseProviderForm<GoogleWork
name="propertyMappings"
>
<ak-dual-select-dynamic-selected
.provider=${googleWorkspacePropertyMappingsProvider}
.selector=${makeGoogleWorkspacePropertyMappingsSelector(
.provider=${propertyMappingsProvider}
.selector=${propertyMappingsSelector(
this.instance?.propertyMappings,
"goauthentik.io/providers/google_workspace/user",
)}
Expand All @@ -241,8 +241,8 @@ export class GoogleWorkspaceProviderFormPage extends BaseProviderForm<GoogleWork
name="propertyMappingsGroup"
>
<ak-dual-select-dynamic-selected
.provider=${googleWorkspacePropertyMappingsProvider}
.selector=${makeGoogleWorkspacePropertyMappingsSelector(
.provider=${propertyMappingsProvider}
.selector=${propertyMappingsSelector(
this.instance?.propertyMappingsGroup,
"goauthentik.io/providers/google_workspace/group",
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { DualSelectPair } from "@goauthentik/elements/ak-dual-select/types.js";

import { GoogleWorkspaceProviderMapping, PropertymappingsApi } from "@goauthentik/api";

const mappingToSelect = (m: GoogleWorkspaceProviderMapping) => [m.pk, m.name, m.name, m];

export async function propertyMappingsProvider(page = 1, search = "") {
const propertyMappings = await new PropertymappingsApi(
DEFAULT_CONFIG,
).propertymappingsProviderGoogleWorkspaceList({
ordering: "managed",
pageSize: 20,
search: search.trim(),
page,
});
return {
pagination: propertyMappings.pagination,
options: propertyMappings.results.map(mappingToSelect),
};
}

export function propertyMappingsSelector(
instanceMappings: string[] | undefined,
defaultSelection: string,
) {
if (!instanceMappings) {
return async (mappings: DualSelectPair<GoogleWorkspaceProviderMapping>[]) =>
mappings.filter(
([_0, _1, _2, mapping]: DualSelectPair<GoogleWorkspaceProviderMapping>) =>
mapping?.managed === defaultSelection,
);
}

return async () => {
const pm = new PropertymappingsApi(DEFAULT_CONFIG);
const mappings = await Promise.allSettled(
instanceMappings.map((instanceId) =>
pm.propertymappingsProviderGoogleWorkspaceRetrieve({ pmUuid: instanceId }),
),
);

return mappings
.filter((s) => s.status === "fulfilled")
.map((s) => s.value)
.map(mappingToSelect);
};
}

This file was deleted.

46 changes: 46 additions & 0 deletions web/src/admin/providers/ldap/LDAPProviderFormHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { DualSelectPair } from "@goauthentik/elements/ak-dual-select/types.js";

import { LDAPSourcePropertyMapping, PropertymappingsApi } from "@goauthentik/api";

const mappingToSelect = (m: LDAPSourcePropertyMapping) => [m.pk, m.name, m.name, m];

export async function propertyMappingsProvider(page = 1, search = "") {
const propertyMappings = await new PropertymappingsApi(
DEFAULT_CONFIG,
).propertymappingsSourceLdapList({
ordering: "managed",
pageSize: 20,
search: search.trim(),
page,
});
return {
pagination: propertyMappings.pagination,
options: propertyMappings.results.map(mappingToSelect),
};
}

export function propertyMappingsSelector(instanceMappings?: string[]) {
if (!instanceMappings) {
return async (transports: DualSelectPair<LDAPSourcePropertyMapping>[]) =>
transports.filter(
([_0, _1, _2, mapping]: DualSelectPair<LDAPSourcePropertyMapping>) =>
mapping?.managed?.startsWith("goauthentik.io/sources/ldap/default") ||
mapping?.managed?.startsWith("goauthentik.io/sources/ldap/ms"),
);
}

return async () => {
const pm = new PropertymappingsApi(DEFAULT_CONFIG);
const mappings = await Promise.allSettled(
instanceMappings.map((instanceId) =>
pm.propertymappingsSourceLdapRetrieve({ pmUuid: instanceId }),
),
);

return mappings
.filter((s) => s.status === "fulfilled")
.map((s) => s.value)
.map(mappingToSelect);
};
}
Loading

0 comments on commit e077a5c

Please sign in to comment.