From 8945bc0dc10060411efbe638bd8ad33cae8de529 Mon Sep 17 00:00:00 2001 From: x032205 Date: Tue, 26 Aug 2025 20:40:16 -0400 Subject: [PATCH] Review fixes --- .../20250815022242_identity-lockouts.ts | 22 +++++++++----- .../db/schemas/identity-universal-auths.ts | 4 +-- .../ee/services/audit-log/audit-log-types.ts | 8 ++--- backend/src/lib/api-docs/constants.ts | 10 ++++--- .../v1/identity-universal-auth-router.ts | 30 ++++++++++++------- .../identity-ua/identity-ua-service.ts | 18 +++++------ .../services/identity-ua/identity-ua-types.ts | 8 ++--- .../src/hooks/api/identities/mutations.tsx | 16 +++++----- frontend/src/hooks/api/identities/types.ts | 12 ++++---- .../IdentitySection/IdentityModal.tsx | 4 +-- .../IdentityUniversalAuthForm.tsx | 19 +++++++----- .../ViewIdentityUniversalAuthContent.tsx | 5 ++-- 12 files changed, 89 insertions(+), 67 deletions(-) diff --git a/backend/src/db/migrations/20250815022242_identity-lockouts.ts b/backend/src/db/migrations/20250815022242_identity-lockouts.ts index a0e661d2d..e6a20f02c 100644 --- a/backend/src/db/migrations/20250815022242_identity-lockouts.ts +++ b/backend/src/db/migrations/20250815022242_identity-lockouts.ts @@ -6,8 +6,11 @@ export async function up(knex: Knex): Promise { if (await knex.schema.hasTable(TableName.IdentityUniversalAuth)) { const hasLockoutEnabled = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutEnabled"); const hasLockoutThreshold = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutThreshold"); - const hasLockoutDuration = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutDuration"); - const hasLockoutCounterReset = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutCounterReset"); + const hasLockoutDuration = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutDurationSeconds"); + const hasLockoutCounterReset = await knex.schema.hasColumn( + TableName.IdentityUniversalAuth, + "lockoutCounterResetSeconds" + ); await knex.schema.alterTable(TableName.IdentityUniversalAuth, async (t) => { if (!hasLockoutEnabled) { @@ -17,10 +20,10 @@ export async function up(knex: Knex): Promise { t.integer("lockoutThreshold").notNullable().defaultTo(3); } if (!hasLockoutDuration) { - t.integer("lockoutDuration").notNullable().defaultTo(300); // 5 minutes (in seconds) + t.integer("lockoutDurationSeconds").notNullable().defaultTo(300); // 5 minutes } if (!hasLockoutCounterReset) { - t.integer("lockoutCounterReset").notNullable().defaultTo(30); // 30 seconds + t.integer("lockoutCounterResetSeconds").notNullable().defaultTo(30); // 30 seconds } }); } @@ -30,8 +33,11 @@ export async function down(knex: Knex): Promise { if (await knex.schema.hasTable(TableName.IdentityUniversalAuth)) { const hasLockoutEnabled = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutEnabled"); const hasLockoutThreshold = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutThreshold"); - const hasLockoutDuration = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutDuration"); - const hasLockoutCounterReset = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutCounterReset"); + const hasLockoutDuration = await knex.schema.hasColumn(TableName.IdentityUniversalAuth, "lockoutDurationSeconds"); + const hasLockoutCounterReset = await knex.schema.hasColumn( + TableName.IdentityUniversalAuth, + "lockoutCounterResetSeconds" + ); await knex.schema.alterTable(TableName.IdentityUniversalAuth, (t) => { if (hasLockoutEnabled) { @@ -41,10 +47,10 @@ export async function down(knex: Knex): Promise { t.dropColumn("lockoutThreshold"); } if (hasLockoutDuration) { - t.dropColumn("lockoutDuration"); + t.dropColumn("lockoutDurationSeconds"); } if (hasLockoutCounterReset) { - t.dropColumn("lockoutCounterReset"); + t.dropColumn("lockoutCounterResetSeconds"); } }); } diff --git a/backend/src/db/schemas/identity-universal-auths.ts b/backend/src/db/schemas/identity-universal-auths.ts index e42c886c6..29e8314aa 100644 --- a/backend/src/db/schemas/identity-universal-auths.ts +++ b/backend/src/db/schemas/identity-universal-auths.ts @@ -21,8 +21,8 @@ export const IdentityUniversalAuthsSchema = z.object({ accessTokenPeriod: z.coerce.number().default(0), lockoutEnabled: z.boolean().default(true), lockoutThreshold: z.number().default(3), - lockoutDuration: z.number().default(300), - lockoutCounterReset: z.number().default(30) + lockoutDurationSeconds: z.number().default(300), + lockoutCounterResetSeconds: z.number().default(30) }); export type TIdentityUniversalAuths = z.infer; diff --git a/backend/src/ee/services/audit-log/audit-log-types.ts b/backend/src/ee/services/audit-log/audit-log-types.ts index 622bac031..f96468bc7 100644 --- a/backend/src/ee/services/audit-log/audit-log-types.ts +++ b/backend/src/ee/services/audit-log/audit-log-types.ts @@ -869,8 +869,8 @@ interface AddIdentityUniversalAuthEvent { accessTokenTrustedIps: Array; lockoutEnabled: boolean; lockoutThreshold: number; - lockoutDuration: number; - lockoutCounterReset: number; + lockoutDurationSeconds: number; + lockoutCounterResetSeconds: number; }; } @@ -885,8 +885,8 @@ interface UpdateIdentityUniversalAuthEvent { accessTokenTrustedIps?: Array; lockoutEnabled?: boolean; lockoutThreshold?: number; - lockoutDuration?: number; - lockoutCounterReset?: number; + lockoutDurationSeconds?: number; + lockoutCounterResetSeconds?: number; }; } diff --git a/backend/src/lib/api-docs/constants.ts b/backend/src/lib/api-docs/constants.ts index f520d87da..7ecaeff77 100644 --- a/backend/src/lib/api-docs/constants.ts +++ b/backend/src/lib/api-docs/constants.ts @@ -169,8 +169,9 @@ export const UNIVERSAL_AUTH = { "The period for an access token in seconds. This value will be referenced at renewal time. Default value is 0.", lockoutEnabled: "Whether the lockout feature is enabled.", lockoutThreshold: "The amount of times login must fail before locking the identity auth method.", - lockoutDuration: "How long an identity auth method lockout lasts.", - lockoutCounterReset: "How long to wait from the most recent failed login until resetting the lockout counter." + lockoutDurationSeconds: "How long an identity auth method lockout lasts.", + lockoutCounterResetSeconds: + "How long to wait from the most recent failed login until resetting the lockout counter." }, RETRIEVE: { identityId: "The ID of the identity to retrieve the auth method for." @@ -188,8 +189,9 @@ export const UNIVERSAL_AUTH = { accessTokenPeriod: "The new period for an access token in seconds.", lockoutEnabled: "Whether the lockout feature is enabled.", lockoutThreshold: "The amount of times login must fail before locking the identity auth method.", - lockoutDuration: "How long an identity auth method lockout lasts.", - lockoutCounterReset: "How long to wait from the most recent failed login until resetting the lockout counter." + lockoutDurationSeconds: "How long an identity auth method lockout lasts.", + lockoutCounterResetSeconds: + "How long to wait from the most recent failed login until resetting the lockout counter." }, CREATE_CLIENT_SECRET: { identityId: "The ID of the identity to create a client secret for.", diff --git a/backend/src/server/routes/v1/identity-universal-auth-router.ts b/backend/src/server/routes/v1/identity-universal-auth-router.ts index a3d332e0c..6d911a88e 100644 --- a/backend/src/server/routes/v1/identity-universal-auth-router.ts +++ b/backend/src/server/routes/v1/identity-universal-auth-router.ts @@ -140,13 +140,18 @@ export const registerIdentityUaRouter = async (server: FastifyZodProvider) => { accessTokenPeriod: z.number().int().min(0).default(0).describe(UNIVERSAL_AUTH.ATTACH.accessTokenPeriod), lockoutEnabled: z.boolean().default(true).describe(UNIVERSAL_AUTH.ATTACH.lockoutEnabled), lockoutThreshold: z.number().min(1).max(30).default(3).describe(UNIVERSAL_AUTH.ATTACH.lockoutThreshold), - lockoutDuration: z.number().min(30).max(86400).default(300).describe(UNIVERSAL_AUTH.ATTACH.lockoutDuration), - lockoutCounterReset: z + lockoutDurationSeconds: z + .number() + .min(30) + .max(86400) + .default(300) + .describe(UNIVERSAL_AUTH.ATTACH.lockoutDurationSeconds), + lockoutCounterResetSeconds: z .number() .min(5) .max(3600) .default(30) - .describe(UNIVERSAL_AUTH.ATTACH.lockoutCounterReset) + .describe(UNIVERSAL_AUTH.ATTACH.lockoutCounterResetSeconds) }) .refine( (val) => val.accessTokenTTL <= val.accessTokenMaxTTL, @@ -183,8 +188,8 @@ export const registerIdentityUaRouter = async (server: FastifyZodProvider) => { accessTokenNumUsesLimit: identityUniversalAuth.accessTokenNumUsesLimit, lockoutEnabled: identityUniversalAuth.lockoutEnabled, lockoutThreshold: identityUniversalAuth.lockoutThreshold, - lockoutDuration: identityUniversalAuth.lockoutDuration, - lockoutCounterReset: identityUniversalAuth.lockoutCounterReset + lockoutDurationSeconds: identityUniversalAuth.lockoutDurationSeconds, + lockoutCounterResetSeconds: identityUniversalAuth.lockoutCounterResetSeconds } } }); @@ -259,13 +264,18 @@ export const registerIdentityUaRouter = async (server: FastifyZodProvider) => { .describe(UNIVERSAL_AUTH.UPDATE.accessTokenPeriod), lockoutEnabled: z.boolean().optional().describe(UNIVERSAL_AUTH.UPDATE.lockoutEnabled), lockoutThreshold: z.number().min(1).max(30).optional().describe(UNIVERSAL_AUTH.UPDATE.lockoutThreshold), - lockoutDuration: z.number().min(30).max(86400).optional().describe(UNIVERSAL_AUTH.UPDATE.lockoutDuration), - lockoutCounterReset: z + lockoutDurationSeconds: z + .number() + .min(30) + .max(86400) + .optional() + .describe(UNIVERSAL_AUTH.UPDATE.lockoutDurationSeconds), + lockoutCounterResetSeconds: z .number() .min(5) .max(3600) .optional() - .describe(UNIVERSAL_AUTH.UPDATE.lockoutCounterReset) + .describe(UNIVERSAL_AUTH.UPDATE.lockoutCounterResetSeconds) }) .refine( (val) => (val.accessTokenMaxTTL && val.accessTokenTTL ? val.accessTokenTTL <= val.accessTokenMaxTTL : true), @@ -301,8 +311,8 @@ export const registerIdentityUaRouter = async (server: FastifyZodProvider) => { accessTokenNumUsesLimit: identityUniversalAuth.accessTokenNumUsesLimit, lockoutEnabled: identityUniversalAuth.lockoutEnabled, lockoutThreshold: identityUniversalAuth.lockoutThreshold, - lockoutDuration: identityUniversalAuth.lockoutDuration, - lockoutCounterReset: identityUniversalAuth.lockoutCounterReset + lockoutDurationSeconds: identityUniversalAuth.lockoutDurationSeconds, + lockoutCounterResetSeconds: identityUniversalAuth.lockoutCounterResetSeconds } } }); diff --git a/backend/src/services/identity-ua/identity-ua-service.ts b/backend/src/services/identity-ua/identity-ua-service.ts index 02594c22f..3aeb5e83c 100644 --- a/backend/src/services/identity-ua/identity-ua-service.ts +++ b/backend/src/services/identity-ua/identity-ua-service.ts @@ -131,7 +131,7 @@ export const identityUaServiceFactory = ({ await keyStore.setItemWithExpiry( LOCKOUT_KEY, - lockout.lockedOut ? identityUa.lockoutDuration : identityUa.lockoutCounterReset, + lockout.lockedOut ? identityUa.lockoutDurationSeconds : identityUa.lockoutCounterResetSeconds, JSON.stringify(lockout) ); } @@ -251,8 +251,8 @@ export const identityUaServiceFactory = ({ accessTokenPeriod, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }: TAttachUaDTO) => { await validateIdentityUpdateForSuperAdminPrivileges(identityId, isActorSuperAdmin); @@ -325,8 +325,8 @@ export const identityUaServiceFactory = ({ accessTokenPeriod, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }, tx ); @@ -349,8 +349,8 @@ export const identityUaServiceFactory = ({ actorOrgId, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }: TUpdateUaDTO) => { const identityMembershipOrg = await identityOrgMembershipDAL.findOne({ identityId }); if (!identityMembershipOrg) throw new NotFoundError({ message: `Failed to find identity with ID ${identityId}` }); @@ -429,8 +429,8 @@ export const identityUaServiceFactory = ({ : undefined, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }); return { ...updatedUaAuth, orgId: identityMembershipOrg.orgId }; }; diff --git a/backend/src/services/identity-ua/identity-ua-types.ts b/backend/src/services/identity-ua/identity-ua-types.ts index 9c3cc09c2..8e7644b58 100644 --- a/backend/src/services/identity-ua/identity-ua-types.ts +++ b/backend/src/services/identity-ua/identity-ua-types.ts @@ -11,8 +11,8 @@ export type TAttachUaDTO = { isActorSuperAdmin?: boolean; lockoutEnabled: boolean; lockoutThreshold: number; - lockoutDuration: number; - lockoutCounterReset: number; + lockoutDurationSeconds: number; + lockoutCounterResetSeconds: number; } & Omit; export type TUpdateUaDTO = { @@ -25,8 +25,8 @@ export type TUpdateUaDTO = { accessTokenTrustedIps?: { ipAddress: string }[]; lockoutEnabled?: boolean; lockoutThreshold?: number; - lockoutDuration?: number; - lockoutCounterReset?: number; + lockoutDurationSeconds?: number; + lockoutCounterResetSeconds?: number; } & Omit; export type TGetUaDTO = { diff --git a/frontend/src/hooks/api/identities/mutations.tsx b/frontend/src/hooks/api/identities/mutations.tsx index ca903bb48..5189f187c 100644 --- a/frontend/src/hooks/api/identities/mutations.tsx +++ b/frontend/src/hooks/api/identities/mutations.tsx @@ -152,8 +152,8 @@ export const useAddIdentityUniversalAuth = () => { accessTokenTrustedIps, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }) => { const { data: { identityUniversalAuth } @@ -165,8 +165,8 @@ export const useAddIdentityUniversalAuth = () => { accessTokenTrustedIps, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }); return identityUniversalAuth; }, @@ -195,8 +195,8 @@ export const useUpdateIdentityUniversalAuth = () => { accessTokenPeriod, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }) => { const { data: { identityUniversalAuth } @@ -209,8 +209,8 @@ export const useUpdateIdentityUniversalAuth = () => { accessTokenPeriod, lockoutEnabled, lockoutThreshold, - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }); return identityUniversalAuth; }, diff --git a/frontend/src/hooks/api/identities/types.ts b/frontend/src/hooks/api/identities/types.ts index 8f5ec2b8e..36f9eae4e 100644 --- a/frontend/src/hooks/api/identities/types.ts +++ b/frontend/src/hooks/api/identities/types.ts @@ -116,8 +116,8 @@ export type IdentityUniversalAuth = { accessTokenPeriod: number; lockoutEnabled: boolean; lockoutThreshold: number; - lockoutDuration: number; - lockoutCounterReset: number; + lockoutDurationSeconds: number; + lockoutCounterResetSeconds: number; }; export type AddIdentityUniversalAuthDTO = { @@ -135,8 +135,8 @@ export type AddIdentityUniversalAuthDTO = { }[]; lockoutEnabled: boolean; lockoutThreshold: number; - lockoutDuration: number; - lockoutCounterReset: number; + lockoutDurationSeconds: number; + lockoutCounterResetSeconds: number; }; export type UpdateIdentityUniversalAuthDTO = { @@ -154,8 +154,8 @@ export type UpdateIdentityUniversalAuthDTO = { }[]; lockoutEnabled?: boolean; lockoutThreshold?: number; - lockoutDuration?: number; - lockoutCounterReset?: number; + lockoutDurationSeconds?: number; + lockoutCounterResetSeconds?: number; }; export type DeleteIdentityUniversalAuthDTO = { diff --git a/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityModal.tsx b/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityModal.tsx index 1d080be7d..4c7fd69cc 100644 --- a/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityModal.tsx +++ b/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityModal.tsx @@ -151,8 +151,8 @@ export const IdentityModal = ({ popUp, handlePopUpToggle }: Props) => { accessTokenPeriod: 0, lockoutEnabled: true, lockoutThreshold: 3, - lockoutDuration: 300, - lockoutCounterReset: 30 + lockoutDurationSeconds: 300, + lockoutCounterResetSeconds: 30 }); handlePopUpToggle("identity", false); diff --git a/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityUniversalAuthForm.tsx b/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityUniversalAuthForm.tsx index 4a2802e08..9681461e6 100644 --- a/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityUniversalAuthForm.tsx +++ b/frontend/src/pages/organization/AccessManagementPage/components/OrgIdentityTab/components/IdentitySection/IdentityUniversalAuthForm.tsx @@ -215,8 +215,8 @@ export const IdentityUniversalAuthForm = ({ useEffect(() => { if (data) { - const lockoutDurationObj = getObjectFromSeconds(data.lockoutDuration); - const lockoutCounterResetObj = getObjectFromSeconds(data.lockoutCounterReset); + const lockoutDurationObj = getObjectFromSeconds(data.lockoutDurationSeconds); + const lockoutCounterResetObj = getObjectFromSeconds(data.lockoutCounterResetSeconds); reset({ accessTokenTTL: String(data.accessTokenTTL), @@ -279,8 +279,11 @@ export const IdentityUniversalAuthForm = ({ try { if (!identityId) return; - const lockoutDuration = durationToSeconds(Number(lockoutDurationValue), lockoutDurationUnit); - const lockoutCounterReset = durationToSeconds( + const lockoutDurationSeconds = durationToSeconds( + Number(lockoutDurationValue), + lockoutDurationUnit + ); + const lockoutCounterResetSeconds = durationToSeconds( Number(lockoutCounterResetValue), lockoutCounterResetUnit ); @@ -298,8 +301,8 @@ export const IdentityUniversalAuthForm = ({ accessTokenPeriod: Number(accessTokenPeriod), lockoutEnabled, lockoutThreshold: Number(lockoutThreshold), - lockoutDuration, - lockoutCounterReset + lockoutDurationSeconds, + lockoutCounterResetSeconds }); } else { // create new universal auth configuration @@ -315,8 +318,8 @@ export const IdentityUniversalAuthForm = ({ accessTokenPeriod: Number(accessTokenPeriod), lockoutEnabled, lockoutThreshold: Number(lockoutThreshold), - lockoutDuration: Number(lockoutDuration), - lockoutCounterReset: Number(lockoutCounterReset) + lockoutDurationSeconds: Number(lockoutDurationSeconds), + lockoutCounterResetSeconds: Number(lockoutCounterResetSeconds) }); } diff --git a/frontend/src/pages/organization/IdentityDetailsByIDPage/components/ViewIdentityAuthModal/ViewIdentityUniversalAuthContent.tsx b/frontend/src/pages/organization/IdentityDetailsByIDPage/components/ViewIdentityAuthModal/ViewIdentityUniversalAuthContent.tsx index f31e59cd8..70f7e6948 100644 --- a/frontend/src/pages/organization/IdentityDetailsByIDPage/components/ViewIdentityAuthModal/ViewIdentityUniversalAuthContent.tsx +++ b/frontend/src/pages/organization/IdentityDetailsByIDPage/components/ViewIdentityAuthModal/ViewIdentityUniversalAuthContent.tsx @@ -1,6 +1,7 @@ import { useState } from "react"; import { faBan, faCheck, faCopy } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; +import ms from "ms"; import { createNotification } from "@app/components/notifications"; import { OrgPermissionCan } from "@app/components/permissions"; @@ -135,10 +136,10 @@ export const ViewIdentityUniversalAuthContent = ({ {data.lockoutThreshold} - {data.lockoutDuration} seconds + {ms(data.lockoutDurationSeconds * 1000, { long: true })} - {data.lockoutCounterReset} seconds + {ms(data.lockoutCounterResetSeconds * 1000, { long: true })}