fix: add confirmation to disable password login (#3829)

https://linear.app/unleash/issue/2-1071/prevent-users-from-disabling-password-authentication-when-there-are-no Improves the behavior of disabling password based login by adding some relevant information and a confirmation dialog with a warning. This felt better than trying to disable the toggle, by still allowing the end users to make the decision, except now it should be a properly informed decision with confirmation. ![image](https://github.com/Unleash/unleash/assets/14320932/2ca754d8-cfa2-4fda-984d-0c34b89750f3) - **Password based administrators**: Admin accounts that have a password set; - **Other administrators**: Other admin users that do not have a password. May be SSO, but may also be users that did not set a password yet; - **Admin service accounts**: Service accounts that have the admin root role. Depending on how you're using the SA this may not necessarily mean locking yourself out of an admin account, especially if you secured its token beforehand; - **Admin API tokens**: Similar to the above. If you secured an admin API token beforehand, you still have access to all features through the API; Each one of them link to the respective page inside Unleash (e.g. users page, service accounts page, tokens page...); If you try to disable and press "save", and only in that scenario, you are presented with the following confirmation dialog: ![image](https://github.com/Unleash/unleash/assets/14320932/5ad6d105-ad47-4d31-a1df-04737aed4e00)
2024-12-22 19:07:54 +01:00 · 2023-05-23 15:56:34 +01:00 · 2023-05-23 15:56:34 +01:00 · c0bcc50b28
commit c0bcc50b28
parent 1dba9d092b
14 changed files with 287 additions and 3 deletions
--- a/frontend/src/component/admin/auth/OidcAuth/OidcAuth.tsx
+++ b/frontend/src/component/admin/auth/OidcAuth/OidcAuth.tsx
@ -94,7 +94,7 @@ export const OidcAuth = () => {

    return (
        <PageContent>
-            <Grid container style={{ marginBottom: '1rem' }}>
+            <Grid container sx={{ mb: 3 }}>
                <Grid item md={12}>
                    <Alert severity="info">
                        Please read the{' '}
--- a/frontend/src/component/admin/auth/PasswordAuth/PasswordAuth.tsx
+++ b/frontend/src/component/admin/auth/PasswordAuth/PasswordAuth.tsx
@ -10,15 +10,22 @@ import useAuthSettingsApi, {
 } from 'hooks/api/actions/useAuthSettingsApi/useAuthSettingsApi';
 import useToast from 'hooks/useToast';
 import { formatUnknownError } from 'utils/formatUnknownError';
+import { useAdminCount } from 'hooks/api/getters/useAdminCount/useAdminCount';
+import { Link } from 'react-router-dom';
+import { useApiTokens } from 'hooks/api/getters/useApiTokens/useApiTokens';
+import { PasswordAuthDialog } from './PasswordAuthDialog';

 export const PasswordAuth = () => {
    const { setToastData, setToastApiError } = useToast();
-    const { config } = useAuthSettings('simple');
+    const { config, refetch } = useAuthSettings('simple');
    const [disablePasswordAuth, setDisablePasswordAuth] =
        useState<boolean>(false);
    const { updateSettings, errors, loading } =
        useAuthSettingsApi<ISimpleAuthSettings>('simple');
    const { hasAccess } = useContext(AccessContext);
+    const [confirmationOpen, setConfirmationOpen] = useState(false);
+    const { data: adminCount } = useAdminCount();
+    const { tokens } = useApiTokens();

    useEffect(() => {
        setDisablePasswordAuth(!!config.disabled);
@ -39,11 +46,20 @@ export const PasswordAuth = () => {
    const onSubmit = async (event: React.SyntheticEvent) => {
        event.preventDefault();

+        if (!config.disabled && disablePasswordAuth) {
+            setConfirmationOpen(true);
+        } else {
+            onConfirm();
+        }
+    };
+
+    const onConfirm = async () => {
        try {
            const settings: ISimpleAuthSettings = {
                disabled: disablePasswordAuth,
            };
            await updateSettings(settings);
+            refetch();
            setToastData({
                title: 'Successfully saved',
                text: 'Password authentication settings stored.',
@ -56,9 +72,30 @@ export const PasswordAuth = () => {
            setDisablePasswordAuth(config.disabled);
        }
    };
+
    return (
        <PageContent>
            <form onSubmit={onSubmit}>
+                <Alert severity="info" sx={{ mb: 3 }}>
+                    Overview of administrators on your Unleash instance:
+                    <br />
+                    <br />
+                    <strong>Password based administrators: </strong>{' '}
+                    <Link to="/admin/users">{adminCount?.password}</Link>
+                    <br />
+                    <strong>Other administrators: </strong>{' '}
+                    <Link to="/admin/users">{adminCount?.noPassword}</Link>
+                    <br />
+                    <strong>Admin service accounts: </strong>{' '}
+                    <Link to="/admin/service-accounts">
+                        {adminCount?.service}
+                    </Link>
+                    <br />
+                    <strong>Admin API tokens: </strong>{' '}
+                    <Link to="/admin/api">
+                        {tokens.filter(({ type }) => type === 'admin').length}
+                    </Link>
+                </Alert>
                <Grid container spacing={3} mb={2}>
                    <Grid item md={5}>
                        <strong>Password based login</strong>
@ -97,6 +134,16 @@ export const PasswordAuth = () => {
                        </p>
                    </Grid>
                </Grid>
+                <PasswordAuthDialog
+                    open={confirmationOpen}
+                    setOpen={setConfirmationOpen}
+                    onClick={() => {
+                        setConfirmationOpen(false);
+                        onConfirm();
+                    }}
+                    adminCount={adminCount!}
+                    tokens={tokens}
+                />
            </form>
        </PageContent>
    );
--- a/frontend/src/component/admin/auth/PasswordAuth/PasswordAuthDialog.tsx
+++ b/frontend/src/component/admin/auth/PasswordAuth/PasswordAuthDialog.tsx
@ -0,0 +1,53 @@
+import { Alert, Typography } from '@mui/material';
+import { Dialogue } from 'component/common/Dialogue/Dialogue';
+import { IAdminCount } from 'hooks/api/getters/useAdminCount/useAdminCount';
+import { IApiToken } from 'hooks/api/getters/useApiTokens/useApiTokens';
+
+interface IPasswordAuthDialogProps {
+    open: boolean;
+    setOpen: (open: boolean) => void;
+    onClick: () => void;
+    adminCount: IAdminCount;
+    tokens: IApiToken[];
+}
+
+export const PasswordAuthDialog = ({
+    open,
+    setOpen,
+    onClick,
+    adminCount,
+    tokens,
+}: IPasswordAuthDialogProps) => (
+    <Dialogue
+        open={open}
+        onClose={() => {
+            setOpen(false);
+        }}
+        onClick={onClick}
+        title="Disable password based login?"
+        primaryButtonText="Disable password based login"
+        secondaryButtonText="Cancel"
+    >
+        <Alert severity="warning">
+            <strong>Warning!</strong> Disabling password based login may lock
+            you out of the system permanently if you do not have any alternative
+            admin credentials (such as an admin SSO account or admin API token)
+            secured beforehand.
+            <br />
+            <br />
+            <strong>Password based administrators: </strong>{' '}
+            {adminCount?.password}
+            <br />
+            <strong>Other administrators: </strong> {adminCount?.noPassword}
+            <br />
+            <strong>Admin service accounts: </strong> {adminCount?.service}
+            <br />
+            <strong>Admin API tokens: </strong>{' '}
+            {tokens.filter(({ type }) => type === 'admin').length}
+        </Alert>
+        <Typography sx={{ mt: 3 }}>
+            You are about to disable password based login. Are you sure you want
+            to proceed?
+        </Typography>
+    </Dialogue>
+);
--- a/frontend/src/component/admin/auth/SamlAuth/SamlAuth.tsx
+++ b/frontend/src/component/admin/auth/SamlAuth/SamlAuth.tsx
@ -85,7 +85,7 @@ export const SamlAuth = () => {

    return (
        <PageContent>
-            <Grid container style={{ marginBottom: '1rem' }}>
+            <Grid container sx={{ mb: 3 }}>
                <Grid item md={12}>
                    <Alert severity="info">
                        Please read the{' '}
--- a/frontend/src/hooks/api/getters/useAdminCount/useAdminCount.ts
+++ b/frontend/src/hooks/api/getters/useAdminCount/useAdminCount.ts
@ -0,0 +1,29 @@
+import useSWR from 'swr';
+import { formatApiPath } from 'utils/formatPath';
+import handleErrorResponses from '../httpErrorResponseHandler';
+
+export interface IAdminCount {
+    password: number;
+    noPassword: number;
+    service: number;
+}
+
+export const useAdminCount = () => {
+    const { data, error, mutate } = useSWR<IAdminCount>(
+        formatApiPath(`api/admin/user-admin/admin-count`),
+        fetcher
+    );
+
+    return {
+        data,
+        loading: !error && !data,
+        refetch: () => mutate(),
+        error,
+    };
+};
+
+const fetcher = (path: string) => {
+    return fetch(path)
+        .then(handleErrorResponses('Admin count'))
+        .then(res => res.json());
+};
--- a/src/lib/db/account-store.ts
+++ b/src/lib/db/account-store.ts
@ -3,6 +3,7 @@ import User from '../types/user';

 import NotFoundError from '../error/notfound-error';
 import { IUserLookup } from '../types/stores/user-store';
+import { IAdminCount } from '../types/stores/account-store';
 import { IAccountStore } from '../types';
 import { Db } from './db';

@ -169,4 +170,31 @@ export class AccountStore implements IAccountStore {
            this.logger.error('Could not update lastSeen, error: ', err);
        }
    }
+
+    async getAdminCount(): Promise<IAdminCount> {
+        const adminCount = await this.activeAccounts()
+            .join('role_user as ru', 'users.id', 'ru.user_id')
+            .where(
+                'ru.role_id',
+                '=',
+                this.db.raw('(SELECT id FROM roles WHERE name = ?)', ['Admin']),
+            )
+            .select(
+                this.db.raw(
+                    'COUNT(CASE WHEN users.password_hash IS NOT NULL AND users.is_service = false THEN 1 END)::integer AS password',
+                ),
+                this.db.raw(
+                    'COUNT(CASE WHEN users.password_hash IS NULL AND users.is_service = false THEN 1 END)::integer AS no_password',
+                ),
+                this.db.raw(
+                    'COUNT(CASE WHEN users.is_service = true THEN 1 END)::integer AS service',
+                ),
+            );
+
+        return {
+            password: adminCount[0].password,
+            noPassword: adminCount[0].no_password,
+            service: adminCount[0].service,
+        };
+    }
 }
--- a/src/lib/openapi/index.ts
+++ b/src/lib/openapi/index.ts
@ -5,6 +5,7 @@ import {
    addonSchema,
    addonsSchema,
    addonTypeSchema,
+    adminCountSchema,
    adminFeaturesQuerySchema,
    apiTokenSchema,
    apiTokensSchema,
@ -182,6 +183,7 @@ interface OpenAPIV3DocumentWithServers extends OpenAPIV3.Document {

 // All schemas in `openapi/spec` should be listed here.
 export const schemas: UnleashSchemas = {
+    adminCountSchema,
    adminFeaturesQuerySchema,
    addonParameterSchema,
    addonSchema,
--- a/src/lib/openapi/spec/admin-count-schema.ts
+++ b/src/lib/openapi/spec/admin-count-schema.ts
@ -0,0 +1,28 @@
+import { FromSchema } from 'json-schema-to-ts';
+
+export const adminCountSchema = {
+    $id: '#/components/schemas/adminCountSchema',
+    type: 'object',
+    additionalProperties: false,
+    description: 'Contains total admin counts for an Unleash instance.',
+    required: ['password', 'noPassword', 'service'],
+    properties: {
+        password: {
+            type: 'number',
+            description: 'Total number of admins that have a password set.',
+        },
+        noPassword: {
+            type: 'number',
+            description:
+                'Total number of admins that do not have a password set. May be SSO, but may also be users that did not set a password yet.',
+        },
+        service: {
+            type: 'number',
+            description:
+                'Total number of service accounts that have the admin root role.',
+        },
+    },
+    components: {},
+} as const;
+
+export type AdminCountSchema = FromSchema<typeof adminCountSchema>;
--- a/src/lib/openapi/spec/index.ts
+++ b/src/lib/openapi/spec/index.ts
@ -136,3 +136,4 @@ export * from './upsert-segment-schema';
 export * from './batch-features-schema';
 export * from './token-string-list-schema';
 export * from './bulk-toggle-features-schema';
+export * from './admin-count-schema';
--- a/src/lib/routes/admin-api/user-admin.ts
+++ b/src/lib/routes/admin-api/user-admin.ts
@ -41,6 +41,10 @@ import { IGroup } from '../../types/group';
 import { IFlagResolver } from '../../types/experimental';
 import rateLimit from 'express-rate-limit';
 import { minutesToMilliseconds } from 'date-fns';
+import {
+    AdminCountSchema,
+    adminCountSchema,
+} from '../../openapi/spec/admin-count-schema';

 export default class UserAdminController extends Controller {
    private flagResolver: IFlagResolver;
@ -192,6 +196,22 @@ export default class UserAdminController extends Controller {
            ],
        });

+        this.route({
+            method: 'get',
+            path: '/admin-count',
+            handler: this.getAdminCount,
+            permission: ADMIN,
+            middleware: [
+                openApiService.validPath({
+                    tags: ['Users'],
+                    operationId: 'getAdminCount',
+                    responses: {
+                        200: createResponseSchema('adminCountSchema'),
+                    },
+                }),
+            ],
+        });
+
        this.route({
            method: 'post',
            path: '',
@ -498,4 +518,19 @@ export default class UserAdminController extends Controller {
        await this.userService.changePassword(+id, password);
        res.status(200).send();
    }
+
+    async getAdminCount(
+        req: Request,
+        res: Response<AdminCountSchema>,
+    ): Promise<void> {
+        console.log('user-admin controller');
+        const adminCount = await this.accountService.getAdminCount();
+
+        this.openApiService.respondWithValidation(
+            200,
+            res,
+            adminCountSchema.$id,
+            adminCount,
+        );
+    }
 }
--- a/src/lib/services/account-service.ts
+++ b/src/lib/services/account-service.ts
@ -5,6 +5,7 @@ import { IAccountStore, IUnleashStores } from '../types/stores';
 import { minutesToMilliseconds } from 'date-fns';
 import { AccessService } from './access-service';
 import { RoleName } from '../types/model';
+import { IAdminCount } from 'lib/types/stores/account-store';

 interface IUserWithRole extends IUser {
    rootRole: number;
@ -52,6 +53,10 @@ export class AccountService {
        return this.store.getAccountByPersonalAccessToken(secret);
    }

+    async getAdminCount(): Promise<IAdminCount> {
+        return this.store.getAdminCount();
+    }
+
    async updateLastSeen(): Promise<void> {
        if (this.lastSeenSecrets.size > 0) {
            const toStore = [...this.lastSeenSecrets];
--- a/src/lib/types/stores/account-store.ts
+++ b/src/lib/types/stores/account-store.ts
@ -7,6 +7,12 @@ export interface IUserLookup {
    email?: string;
 }

+export interface IAdminCount {
+    password: number;
+    noPassword: number;
+    service: number;
+}
+
 export interface IAccountStore extends Store<IUser, number> {
    hasAccount(idQuery: IUserLookup): Promise<number | undefined>;
    search(query: string): Promise<IUser[]>;
@ -15,4 +21,5 @@ export interface IAccountStore extends Store<IUser, number> {
    count(): Promise<number>;
    getAccountByPersonalAccessToken(secret: string): Promise<IUser>;
    markSeenAt(secrets: string[]): Promise<void>;
+    getAdminCount(): Promise<IAdminCount>;
 }
--- a/src/test/e2e/api/openapi/snapshots/openapi.e2e.test.ts.snap
+++ b/src/test/e2e/api/openapi/snapshots/openapi.e2e.test.ts.snap
@ -608,6 +608,30 @@ The provider you choose for your addon dictates what properties the \`parameters
        ],
        "type": "object",
      },
+      "adminCountSchema": {
+        "additionalProperties": false,
+        "description": "Contains total admin counts for an Unleash instance.",
+        "properties": {
+          "noPassword": {
+            "description": "Total number of admins that do not have a password set. May be SSO, but may also be users that did not set a password yet.",
+            "type": "number",
+          },
+          "password": {
+            "description": "Total number of admins that have a password set.",
+            "type": "number",
+          },
+          "service": {
+            "description": "Total number of service accounts that have the admin root role.",
+            "type": "number",
+          },
+        },
+        "required": [
+          "password",
+          "noPassword",
+          "service",
+        ],
+        "type": "object",
+      },
      "adminFeaturesQuerySchema": {
        "additionalProperties": false,
        "properties": {
@ -12522,6 +12546,26 @@ If the provided project does not exist, the list of events will be empty.",
        ],
      },
    },
+    "/api/admin/user-admin/admin-count": {
+      "get": {
+        "operationId": "getAdminCount",
+        "responses": {
+          "200": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/adminCountSchema",
+                },
+              },
+            },
+            "description": "adminCountSchema",
+          },
+        },
+        "tags": [
+          "Users",
+        ],
+      },
+    },
    "/api/admin/user-admin/reset-password": {
      "post": {
        "operationId": "resetUserPassword",
--- a/src/test/fixtures/fake-account-store.ts
+++ b/src/test/fixtures/fake-account-store.ts
@ -3,6 +3,7 @@ import {
    // ICreateUser,
    IUserLookup,
    IAccountStore,
+    IAdminCount,
 } from '../../lib/types/stores/account-store';

 export class FakeAccountStore implements IAccountStore {
@ -93,4 +94,8 @@ export class FakeAccountStore implements IAccountStore {
    async markSeenAt(secrets: string[]): Promise<void> {
        throw new Error('Not implemented');
    }
+
+    async getAdminCount(): Promise<IAdminCount> {
+        throw new Error('Not implemented');
+    }
 }