web: remove `./element`⇢`./user` references (#6866)

* Web: Detangling some circular dependencies in Admin and User

Admin, User, and Flow should not dependend upon each other, at least
not in a circular way.  If Admin and User depend on Flow, that's
fine, but Flow should not correspondingly depend upon elements of
either; if they have something in common, let's put them in
`@goauthentik/common` or find some other smart place to store them.

This commit refactors the intentToLabel and actionToLabel functions
into `@goauthentik/common/labels` and converts them to static tables
for maintenance purposes.

* web: "Consistency is the hobgoblin of small minds" - Ralph Waldo Emerson

* web: I found these confusing to look at, so I added comments.

* web: remove admin-to-user component reference(s)

There was only one: AppIcon.  This has been moved to `components`.

Touching the LibraryApplications page triggered a cyclomatic
complexity check.  Extracting the expansion block and streamlining
the class and style declarations with lit directives helped.

* web: remove admin from elements

This commit removes the two references from `elements` to `admin`: the list of UserEvents and a
reference to the FlowSearch type, used by the Forms manager to decide how to extract a value.
For FlowSearch, a different convention for detecting the type was implemented (instances of the
object have a unique fieldname for the value holder).  UserEvents and ObjectChangelog have been
moved to `components` as they're clearly dependent upon the API.

This defers work on removing Admin from Components, as that is (again) references going the
wrong way, but that can happen later.

* web: remove admin-to-user component reference(s) (#6856)

There was only one: AppIcon.  This has been moved to `components`.

Touching the LibraryApplications page triggered a cyclomatic
complexity check.  Extracting the expansion block and streamlining
the class and style declarations with lit directives helped.

* This was supposed to be merged.

* web: remove `./element`⇢`./user` references

The offender here is UserDevicesList, which despite being in `elements` is only
used by the admin/user/UserViewPage.  The problem is that UserDevicesList,
despite being in `admin`, inherits from `user`, so moving it would have created
a new admin⇢user reference, and the whole point of this exercise is to get rid
of references that point "up" from the foundational pieces to the views, or
that refer to components in sibling applications.

After examining UserDevicesList, I realized that *every feature* of MFADevicesList
had been overridden: the rows, the columns, the toolbar, and the endpoint all had
custom overrides.  Nothing was left of MFADevicesList after that.   Even the
property that the web component used had been completely changed.  The only thing
they had in common was that they both inherited from `Table<Device>`.

Refactoring UserDevicesList so that it inherited directly from `Table<Device>` and
then moving it into `./admin/users` was the obvious and correct step.

Both used the same label table, so that went into the `common/labels` folder.

Along the way, I cleaned up a few minor details. Just little things, like the repeated invocation
of:

```
new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorAdminMETHODDestroy({ id: device.pk });
```

This is repeated five times, once for each Method.  By creating these:

```
        const api = new AuthenticatorsApi(DEFAULT_CONFIG);
        const id = { id: device.pk };
```

The method invocation could be just `api.authenticatorsMETHODDestroy(id)`, which is easier on the
eyes.  See the MFADevicesPage for the full example.

Similarly,

```
return [
   new TableColumn(msg("Name"), ""),
   new TableColumn(msg("Type"), ""),
   new TableColumn("")
];
   ```

is more straightforward as:

```
const headers = [msg("Name"), msg("Type"), ""];
return headers.map((th) => new TableColumn(th, ""));
```

We've labeled what we're working with, and web developers ought to know that `th` is the HTML code
for `table header`.

I've had to alter what files are scanned in pre-commit mode; it doesn't handle renamed files very well,
and at the moment a file that is renamed is not scanned, as its "new" name is not straightforwardly
displayed, not even by `git porcelain`.

* web: make the table of column headers look like a table

* web: build failure thanks to local cache; fixed

* Update web/src/common/labels.ts

Co-authored-by: Jens L. <jens@goauthentik.io>
Signed-off-by: Ken Sternberg <133134217+kensternberg-authentik@users.noreply.github.com>

---------

Signed-off-by: Ken Sternberg <133134217+kensternberg-authentik@users.noreply.github.com>
Co-authored-by: Jens L. <jens@goauthentik.io>
This commit is contained in:
Ken Sternberg 2023-09-14 10:15:15 -07:00 committed by GitHub
parent 1aebfd2370
commit a9398c92ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 48 additions and 62 deletions

View File

@ -15,7 +15,7 @@
"build-proxy": "run-s build-locales rollup:build-proxy", "build-proxy": "run-s build-locales rollup:build-proxy",
"watch": "run-s build-locales rollup:watch", "watch": "run-s build-locales rollup:watch",
"lint": "eslint . --max-warnings 0 --fix", "lint": "eslint . --max-warnings 0 --fix",
"lint:precommit": "eslint --max-warnings 0 --config ./.eslintrc.precommit.json $(git status --porcelain | cut -c2- | grep '^[M?]' | cut -c7- | grep -E '\\.(ts|js|tsx|jsx)$') ", "lint:precommit": "eslint --max-warnings 0 --config ./.eslintrc.precommit.json $(git status --porcelain | grep '^[M?][M?]' | cut -c8- | grep -E '\\.(ts|js|tsx|jsx)$') ",
"lint:spelling": "codespell -D - -D ../.github/codespell-dictionary.txt -I ../.github/codespell-words.txt -S './src/locales/**' ./src -s", "lint:spelling": "codespell -D - -D ../.github/codespell-dictionary.txt -I ../.github/codespell-words.txt -S './src/locales/**' ./src -s",
"lit-analyse": "lit-analyzer src", "lit-analyse": "lit-analyzer src",
"precommit": "run-s tsc lit-analyse lint:precommit lint:spelling prettier", "precommit": "run-s tsc lit-analyse lint:precommit lint:spelling prettier",

View File

@ -1,8 +1,8 @@
import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { deviceTypeName } from "@goauthentik/common/labels";
import "@goauthentik/elements/forms/DeleteBulkForm"; import "@goauthentik/elements/forms/DeleteBulkForm";
import { PaginatedResponse } from "@goauthentik/elements/table/Table"; import { PaginatedResponse } from "@goauthentik/elements/table/Table";
import { TableColumn } from "@goauthentik/elements/table/Table"; import { Table, TableColumn } from "@goauthentik/elements/table/Table";
import { MFADevicesPage, deviceTypeName } from "@goauthentik/user/user-settings/mfa/MFADevicesPage";
import { msg } from "@lit/localize"; import { msg } from "@lit/localize";
import { TemplateResult, html } from "lit"; import { TemplateResult, html } from "lit";
@ -11,7 +11,7 @@ import { customElement, property } from "lit/decorators.js";
import { AuthenticatorsApi, Device } from "@goauthentik/api"; import { AuthenticatorsApi, Device } from "@goauthentik/api";
@customElement("ak-user-device-list") @customElement("ak-user-device-list")
export class UserDeviceList extends MFADevicesPage { export class UserDeviceList extends Table<Device> {
@property({ type: Number }) @property({ type: Number })
userId?: number; userId?: number;
@ -36,41 +36,34 @@ export class UserDeviceList extends MFADevicesPage {
}); });
} }
columns(): TableColumn[] {
// prettier-ignore
return [
msg("Name"),
msg("Type"),
msg("Confirmed")
].map((th) => new TableColumn(th, ""))
}
async deleteWrapper(device: Device) { async deleteWrapper(device: Device) {
const api = new AuthenticatorsApi(DEFAULT_CONFIG);
const id = { id: device.pk };
switch (device.type) { switch (device.type) {
case "authentik_stages_authenticator_duo.DuoDevice": case "authentik_stages_authenticator_duo.DuoDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAdminDuoDestroy({ return api.authenticatorsAdminDuoDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_sms.SMSDevice": case "authentik_stages_authenticator_sms.SMSDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAdminSmsDestroy({ return api.authenticatorsAdminSmsDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_totp.TOTPDevice": case "authentik_stages_authenticator_totp.TOTPDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAdminTotpDestroy({ return api.authenticatorsAdminTotpDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_static.StaticDevice": case "authentik_stages_authenticator_static.StaticDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAdminStaticDestroy({ return api.authenticatorsAdminStaticDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_webauthn.WebAuthnDevice": case "authentik_stages_authenticator_webauthn.WebAuthnDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAdminWebauthnDestroy({ return api.authenticatorsAdminWebauthnDestroy(id);
id: device.pk,
});
default: default:
break; break;
} }
} }
columns(): TableColumn[] {
return [
new TableColumn(msg("Name"), ""),
new TableColumn(msg("Type"), ""),
new TableColumn(msg("Confirmed"), ""),
];
}
renderToolbar(): TemplateResult { renderToolbar(): TemplateResult {
return html` <ak-spinner-button return html` <ak-spinner-button
.callAction=${() => { .callAction=${() => {

View File

@ -21,7 +21,6 @@ import { showMessage } from "@goauthentik/elements/messages/MessageContainer";
import "@goauthentik/elements/oauth/UserRefreshList"; import "@goauthentik/elements/oauth/UserRefreshList";
import "@goauthentik/elements/user/SessionList"; import "@goauthentik/elements/user/SessionList";
import "@goauthentik/elements/user/UserConsentList"; import "@goauthentik/elements/user/UserConsentList";
import "@goauthentik/elements/user/UserDevicesList";
import { msg, str } from "@lit/localize"; import { msg, str } from "@lit/localize";
import { CSSResult, TemplateResult, html } from "lit"; import { CSSResult, TemplateResult, html } from "lit";
@ -40,6 +39,8 @@ import PFSizing from "@patternfly/patternfly/utilities/Sizing/sizing.css";
import { CapabilitiesEnum, CoreApi, User } from "@goauthentik/api"; import { CapabilitiesEnum, CoreApi, User } from "@goauthentik/api";
import "./UserDevicesList";
@customElement("ak-user-view") @customElement("ak-user-view")
export class UserViewPage extends AKElement { export class UserViewPage extends AKElement {
@property({ type: Number }) @property({ type: Number })

View File

@ -1,6 +1,6 @@
import { msg } from "@lit/localize"; import { msg } from "@lit/localize";
import { EventActions, IntentEnum, SeverityEnum } from "@goauthentik/api"; import { Device, EventActions, IntentEnum, SeverityEnum } from "@goauthentik/api";
/* Various tables in the API for which we need to supply labels */ /* Various tables in the API for which we need to supply labels */
@ -56,3 +56,12 @@ export const severityEnumToLabel = new Map<SeverityEnum | null | undefined, stri
export const severityToLabel = (severity: SeverityEnum | null | undefined) => export const severityToLabel = (severity: SeverityEnum | null | undefined) =>
severityEnumToLabel.get(severity) ?? msg("Unknown severity"); severityEnumToLabel.get(severity) ?? msg("Unknown severity");
// TODO: Add verbose_name field to now vendored OTP devices
export const deviceTypeToLabel = new Map<string, string>([
["authentik_stages_authenticator_static.StaticDevice", msg("Static tokens")],
["authentik_stages_authenticator_totp.TOTPDevice", msg("TOTP Device")],
]);
export const deviceTypeName = (device: Device) =>
deviceTypeToLabel.get(device.type) ?? device?.verboseName ?? "";

View File

@ -1,4 +1,5 @@
import { AndNext, DEFAULT_CONFIG } from "@goauthentik/common/api/config"; import { AndNext, DEFAULT_CONFIG } from "@goauthentik/common/api/config";
import { deviceTypeName } from "@goauthentik/common/labels";
import "@goauthentik/elements/buttons/Dropdown"; import "@goauthentik/elements/buttons/Dropdown";
import "@goauthentik/elements/buttons/ModalButton"; import "@goauthentik/elements/buttons/ModalButton";
import "@goauthentik/elements/buttons/TokenCopyButton"; import "@goauthentik/elements/buttons/TokenCopyButton";
@ -15,23 +16,8 @@ import { ifDefined } from "lit/directives/if-defined.js";
import { AuthenticatorsApi, Device, UserSetting } from "@goauthentik/api"; import { AuthenticatorsApi, Device, UserSetting } from "@goauthentik/api";
export function stageToAuthenticatorName(stage: UserSetting): string { export const stageToAuthenticatorName = (stage: UserSetting) =>
if (stage.title) { stage.title ?? `Invalid stage component ${stage.component}`;
return stage.title;
}
return `Invalid stage component ${stage.component}`;
}
export function deviceTypeName(device: Device): string {
switch (device.type) {
case "authentik_stages_authenticator_static.StaticDevice":
return msg("Static tokens");
case "authentik_stages_authenticator_totp.TOTPDevice":
return msg("TOTP Device");
default:
return device.verboseName;
}
}
@customElement("ak-user-settings-mfa") @customElement("ak-user-settings-mfa")
export class MFADevicesPage extends Table<Device> { export class MFADevicesPage extends Table<Device> {
@ -57,7 +43,12 @@ export class MFADevicesPage extends Table<Device> {
} }
columns(): TableColumn[] { columns(): TableColumn[] {
return [new TableColumn(msg("Name")), new TableColumn(msg("Type")), new TableColumn("")]; // prettier-ignore
return [
msg("Name"),
msg("Type"),
""
].map((th) => new TableColumn(th, ""));
} }
renderToolbar(): TemplateResult { renderToolbar(): TemplateResult {
@ -93,27 +84,19 @@ export class MFADevicesPage extends Table<Device> {
} }
async deleteWrapper(device: Device) { async deleteWrapper(device: Device) {
const api = new AuthenticatorsApi(DEFAULT_CONFIG);
const id = { id: device.pk };
switch (device.type) { switch (device.type) {
case "authentik_stages_authenticator_duo.DuoDevice": case "authentik_stages_authenticator_duo.DuoDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsDuoDestroy({ return api.authenticatorsDuoDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_sms.SMSDevice": case "authentik_stages_authenticator_sms.SMSDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsSmsDestroy({ return api.authenticatorsSmsDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_totp.TOTPDevice": case "authentik_stages_authenticator_totp.TOTPDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsTotpDestroy({ return api.authenticatorsTotpDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_static.StaticDevice": case "authentik_stages_authenticator_static.StaticDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsStaticDestroy({ return api.authenticatorsStaticDestroy(id);
id: device.pk,
});
case "authentik_stages_authenticator_webauthn.WebAuthnDevice": case "authentik_stages_authenticator_webauthn.WebAuthnDevice":
return new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsWebauthnDestroy({ return api.authenticatorsWebauthnDestroy(id);
id: device.pk,
});
default: default:
break; break;
} }