From afdf269869eeba51946504d932ff905576b065a8 Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Fri, 22 Dec 2023 13:33:23 -0800 Subject: [PATCH] web: funnel an API down to a single module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Summary:** Calls to _retrieve_, _create_, and _update_ Authenticators have been isolated into a module functions and code accessing those features have been revised to use those functions. **This commit** Isolates the Authenticators APIs for `authenticatorsAllList`, `authenticatorsAdminAllList`, `authenticatorsDestroy` and `authenticatorsUpdate` into a connector module, and updates client code to use them. This eliminates the duplication of `deleteAuthenticatorDevice`, the code for which was in both *Admin* and *User*, and creates a single source of truth for the DeviceType strings that identify Device subtypes. **Details** One thing that's bothered me a lot is the way our APIs, starting on the Django side, start with a base model and then "spread out" to incorporate many different inheritance models: different kinds of Providers, different kinds of Outposts, and different kinds of authentication devices. In a proper object-oriented system, we'd be granted access to the base class and have the power to use it effectively as a generic, switchboarding on some RTTI or value type to handle the differences between the concrete child types. The code generator we use does not provide that base model to UI clients; "funnelling down" to create a sort of artificial base class on the client and then fanning back out is an impractical and error-prone exercise. But we still do a lot of switchboarding, which has three problems: - Adding subtypes touches a lot of different code - Distant implementations can miss a specific instance of a subtype - Repeated use of strings as type handles can introduce spelling errors - The [last line effect](https://link.springer.com/article/10.1007/s10664-016-9489-6) can introduce new and subtle bugs when cut-and-pasting prior examples - Instances of the *same code* in multiple locations make for maintenance headaches This patch introduces the concept of **connectors**, modules that provide CRUD functions for common operations performed on objects of a specific subtype. It is *not* meant to replace concrete class displays or manipulations, such as (using providers as an example) `ProxyProviderViewPage` or HTML that is customized for a specific concrete type of provider. For retrieving lists, deleting instances that can be addressed from the base class, or updating objects — any place where which call among a collection of isomorphic APIs must be specifieda using `switch/case` statements — the connector provides a single source of truth for how to recognize the child types, which `switch/case` statements provide the utility, and what must be done to update them. --- web/src/admin/users/UserDevicesTable.ts | 61 ++++++---------- web/src/connectors/authenticators.ts | 72 +++++++++++++++++++ .../user/user-settings/mfa/MFADeviceForm.ts | 52 +++----------- .../user/user-settings/mfa/MFADevicesPage.ts | 30 +++----- web/tsconfig.json | 1 + 5 files changed, 112 insertions(+), 104 deletions(-) create mode 100644 web/src/connectors/authenticators.ts diff --git a/web/src/admin/users/UserDevicesTable.ts b/web/src/admin/users/UserDevicesTable.ts index 8fc0e1acc..d3f0cc47d 100644 --- a/web/src/admin/users/UserDevicesTable.ts +++ b/web/src/admin/users/UserDevicesTable.ts @@ -1,5 +1,8 @@ -import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; import { deviceTypeName } from "@goauthentik/common/labels"; +import { + destroyAuthenticatorDevice, + retrieveAuthenticatorsAdminAllList, +} from "@goauthentik/connectors/authenticators"; import "@goauthentik/elements/forms/DeleteBulkForm"; import { PaginatedResponse } from "@goauthentik/elements/table/Table"; import { Table, TableColumn } from "@goauthentik/elements/table/Table"; @@ -8,7 +11,7 @@ import { msg } from "@lit/localize"; import { TemplateResult, html } from "lit"; import { customElement, property } from "lit/decorators.js"; -import { AuthenticatorsApi, Device } from "@goauthentik/api"; +import { Device } from "@goauthentik/api"; @customElement("ak-user-device-table") export class UserDeviceTable extends Table { @@ -18,24 +21,22 @@ export class UserDeviceTable extends Table { checkbox = true; async apiEndpoint(): Promise> { - return new AuthenticatorsApi(DEFAULT_CONFIG) - .authenticatorsAdminAllList({ - user: this.userId, - }) - .then((res) => { - return { - pagination: { - count: res.length, - current: 1, - totalPages: 1, - startIndex: 1, - endIndex: res.length, - next: 0, - previous: 0, - }, - results: res, - }; - }); + if (!this.userId) { + throw new Error(`Attempted to retrieve authenticator list for undefined user`); + } + const results = await retrieveAuthenticatorsAdminAllList(this.userId); + return { + pagination: { + count: results.length, + current: 1, + totalPages: 1, + startIndex: 1, + endIndex: results.length, + next: 0, + previous: 0, + }, + results, + }; } columns(): TableColumn[] { @@ -48,25 +49,7 @@ export class UserDeviceTable extends Table { } async deleteWrapper(device: Device) { - const api = new AuthenticatorsApi(DEFAULT_CONFIG); - switch (device.type.toLowerCase()) { - case "authentik_stages_authenticator_duo.duodevice": - return api.authenticatorsAdminDuoDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_sms.smsdevice": - return api.authenticatorsAdminSmsDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_totp.totpdevice": - return api.authenticatorsAdminTotpDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_static.staticdevice": - return api.authenticatorsAdminStaticDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_webauthn.webauthndevice": - return api.authenticatorsAdminWebauthnDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_mobile.mobiledevice": - return api.authenticatorsMobileDestroy({ - uuid: device.pk, - }); - default: - break; - } + return destroyAuthenticatorDevice(device.type, device.pk); } renderToolbarSelected(): TemplateResult { diff --git a/web/src/connectors/authenticators.ts b/web/src/connectors/authenticators.ts new file mode 100644 index 000000000..ab786f95c --- /dev/null +++ b/web/src/connectors/authenticators.ts @@ -0,0 +1,72 @@ +import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; + +import { AuthenticatorsApi, Device } from "@goauthentik/api"; + +enum DeviceType { + Duo = "authentik_stages_authenticator_duo.duodevice", + SMS = "authentik_stages_authenticator_sms.smsdevice", + TOTP = "authentik_stages_authenticator_totp.totpdevice", + Static = "authentik_stages_authenticator_static.staticdevice", + WebAuthn = "authentik_stages_authenticator_webauthn.webauthndevice", + Mobile = "authentik_stages_authenticator_mobile.mobiledevice", +} + +const api = () => new AuthenticatorsApi(DEFAULT_CONFIG); + +// prettier-ignore +function exhaustiveGuard(_value: string): never { + throw new Error( + `Authenticator Device List encountered unknown device type: ${JSON.stringify(_value, null, 2,)}` + ); +} + +export const retrieveAuthenticatorsAllList = () => api().authenticatorsAllList(); + +export const retrieveAuthenticatorsAdminAllList = (user: number) => + api().authenticatorsAdminAllList({ user }); + +export async function destroyAuthenticatorDevice(deviceType: string, id: number | string) { + id = typeof id === "string" ? parseInt(id, 10) : id; + deviceType = deviceType.toLowerCase(); + switch (deviceType) { + case DeviceType.Duo: + return api().authenticatorsDuoDestroy({ id }); + case DeviceType.SMS: + return api().authenticatorsSmsDestroy({ id }); + case DeviceType.TOTP: + return api().authenticatorsTotpDestroy({ id }); + case DeviceType.Static: + return api().authenticatorsStaticDestroy({ id }); + case DeviceType.WebAuthn: + return api().authenticatorsWebauthnDestroy({ id }); + case DeviceType.Mobile: + return api().authenticatorsMobileDestroy({ uuid: `${id}` }); + default: + return exhaustiveGuard(deviceType); + } +} + +export async function updateAuthenticatorDevice( + deviceType: string, + id: number | string, + device: Device, +) { + id = typeof id === "string" ? parseInt(id, 10) : id; + deviceType = deviceType.toLowerCase(); + switch (deviceType) { + case DeviceType.Duo: + return api().authenticatorsDuoUpdate({ id, duoDeviceRequest: device }); + case DeviceType.SMS: + return api().authenticatorsSmsUpdate({ id, sMSDeviceRequest: device }); + case DeviceType.TOTP: + return api().authenticatorsTotpUpdate({ id, tOTPDeviceRequest: device }); + case DeviceType.Static: + return api().authenticatorsStaticUpdate({ id, staticDeviceRequest: device }); + case DeviceType.WebAuthn: + return api().authenticatorsWebauthnUpdate({ id, webAuthnDeviceRequest: device }); + case DeviceType.Mobile: + return api().authenticatorsMobileUpdate({ uuid: `${id}`, mobileDeviceRequest: device }); + default: + return exhaustiveGuard(deviceType); + } +} diff --git a/web/src/user/user-settings/mfa/MFADeviceForm.ts b/web/src/user/user-settings/mfa/MFADeviceForm.ts index c711d2ec6..3f112c672 100644 --- a/web/src/user/user-settings/mfa/MFADeviceForm.ts +++ b/web/src/user/user-settings/mfa/MFADeviceForm.ts @@ -1,4 +1,7 @@ -import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; +import { + retrieveAuthenticatorsAllList, + updateAuthenticatorDevice, +} from "@goauthentik/connectors/authenticators"; import "@goauthentik/elements/forms/HorizontalFormElement"; import { ModelForm } from "@goauthentik/elements/forms/ModelForm"; @@ -7,7 +10,7 @@ import { TemplateResult, html } from "lit"; import { customElement, property } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; -import { AuthenticatorsApi, Device } from "@goauthentik/api"; +import { Device } from "@goauthentik/api"; @customElement("ak-user-mfa-form") export class MFADeviceForm extends ModelForm { @@ -15,8 +18,7 @@ export class MFADeviceForm extends ModelForm { deviceType!: string; async loadInstance(pk: string): Promise { - const devices = await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAllList(); - return devices.filter((device) => { + return (await retrieveAuthenticatorsAllList()).filter((device) => { return device.pk === pk && device.type === this.deviceType; })[0]; } @@ -26,46 +28,10 @@ export class MFADeviceForm extends ModelForm { } async send(device: Device): Promise { - switch (this.instance?.type.toLowerCase()) { - case "authentik_stages_authenticator_duo.duodevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsDuoUpdate({ - id: parseInt(this.instance?.pk, 10), - duoDeviceRequest: device, - }); - break; - case "authentik_stages_authenticator_sms.smsdevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsSmsUpdate({ - id: parseInt(this.instance?.pk, 10), - sMSDeviceRequest: device, - }); - break; - case "authentik_stages_authenticator_totp.totpdevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsTotpUpdate({ - id: parseInt(this.instance?.pk, 10), - tOTPDeviceRequest: device, - }); - break; - case "authentik_stages_authenticator_static.staticdevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsStaticUpdate({ - id: parseInt(this.instance?.pk, 10), - staticDeviceRequest: device, - }); - break; - case "authentik_stages_authenticator_webauthn.webauthndevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsWebauthnUpdate({ - id: parseInt(this.instance?.pk, 10), - webAuthnDeviceRequest: device, - }); - break; - case "authentik_stages_authenticator_mobile.mobiledevice": - await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsMobileUpdate({ - uuid: this.instance?.pk, - mobileDeviceRequest: device, - }); - break; - default: - break; + if (!this.instance) { + return device; } + await updateAuthenticatorDevice(this.instance.type, this.instance.pk, device); return device; } diff --git a/web/src/user/user-settings/mfa/MFADevicesPage.ts b/web/src/user/user-settings/mfa/MFADevicesPage.ts index 318555721..06254f40e 100644 --- a/web/src/user/user-settings/mfa/MFADevicesPage.ts +++ b/web/src/user/user-settings/mfa/MFADevicesPage.ts @@ -1,5 +1,9 @@ -import { AndNext, DEFAULT_CONFIG } from "@goauthentik/common/api/config"; +import { AndNext } from "@goauthentik/common/api/config"; import { deviceTypeName } from "@goauthentik/common/labels"; +import { + destroyAuthenticatorDevice, + retrieveAuthenticatorsAllList, +} from "@goauthentik/connectors/authenticators"; import "@goauthentik/elements/buttons/Dropdown"; import "@goauthentik/elements/buttons/ModalButton"; import "@goauthentik/elements/buttons/TokenCopyButton"; @@ -14,7 +18,7 @@ import { TemplateResult, html } from "lit"; import { customElement, property } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; -import { AuthenticatorsApi, Device, UserSetting } from "@goauthentik/api"; +import { Device, UserSetting } from "@goauthentik/api"; export const stageToAuthenticatorName = (stage: UserSetting) => stage.title ?? `Invalid stage component ${stage.component}`; @@ -27,7 +31,7 @@ export class MFADevicesPage extends Table { checkbox = true; async apiEndpoint(): Promise> { - const devices = await new AuthenticatorsApi(DEFAULT_CONFIG).authenticatorsAllList(); + const devices = await retrieveAuthenticatorsAllList(); return { pagination: { current: 0, @@ -84,25 +88,7 @@ export class MFADevicesPage extends Table { } async deleteWrapper(device: Device) { - const api = new AuthenticatorsApi(DEFAULT_CONFIG); - switch (device.type.toLowerCase()) { - case "authentik_stages_authenticator_duo.duodevice": - return api.authenticatorsDuoDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_sms.smsdevice": - return api.authenticatorsSmsDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_totp.totpdevice": - return api.authenticatorsTotpDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_static.staticdevice": - return api.authenticatorsStaticDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_webauthn.webauthndevice": - return api.authenticatorsWebauthnDestroy({ id: parseInt(device.pk, 10) }); - case "authentik_stages_authenticator_mobile.mobiledevice": - return api.authenticatorsMobileDestroy({ - uuid: device.pk, - }); - default: - break; - } + return destroyAuthenticatorDevice(device.type, device.pk); } renderToolbarSelected(): TemplateResult { diff --git a/web/tsconfig.json b/web/tsconfig.json index 7e9327c55..601a7c970 100644 --- a/web/tsconfig.json +++ b/web/tsconfig.json @@ -6,6 +6,7 @@ "@goauthentik/admin/*": ["src/admin/*"], "@goauthentik/common/*": ["src/common/*"], "@goauthentik/components/*": ["src/components/*"], + "@goauthentik/connectors/*": ["src/connectors/*"], "@goauthentik/docs/*": ["../website/docs/*"], "@goauthentik/elements/*": ["src/elements/*"], "@goauthentik/flow/*": ["src/flow/*"],