Skip to content

Commit 7012138

Browse files
ryanbas21ryanbas21
committed
chore(davinci-client): address pr feedback on password collector design
- Route reducer by field.type so PASSWORD always produces PasswordCollector and PASSWORD_VERIFY always produces ValidatedPasswordCollector, instead of using passwordPolicy presence as the discriminator - Extract password-policy rule functions into password-policy.rules.ts - Order client.validate() so PasswordCollector error precedes ValidatedPasswordCollector success, matching the surrounding error-then-success convention - Drop dead root-level passwordPolicy from mock-form-fields data
1 parent fd7d62d commit 7012138

10 files changed

Lines changed: 154 additions & 189 deletions

‎.changeset/embed-password-policy-in-component.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
'@forgerock/davinci-client': minor
33
---
44

5-
Add `ValidatedPasswordCollector` alongside `PasswordCollector`. The new collector is emitted whenever a password field carries a `passwordPolicy` — the presence of the policy is the sole discriminator between the two types, regardless of the server-side field tag (`PASSWORD` vs `PASSWORD_VERIFY`). `ValidatedPasswordCollector.output.passwordPolicy` is required; consumers can render password requirements directly from the collector.
5+
Add `ValidatedPasswordCollector` alongside `PasswordCollector`. The reducer routes by `field.type`: `PASSWORD` always produces a `PasswordCollector`, `PASSWORD_VERIFY` always produces a `ValidatedPasswordCollector`. `ValidatedPasswordCollector.output.passwordPolicy` carries the embedded policy from the field; when the field has no policy, an empty policy object is emitted and the validator treats it as no rules. Consumers can render password requirements directly from the collector.
66

77
Both collectors now expose a `verify: boolean` on `output` (defaults to `false`), propagated from the field when the server sends `verify: true`.
88

‎packages/davinci-client/api-report/davinci-client.api.md‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
269269
resume: (input: {
270270
continueToken: string;
271271
}) => Promise<InternalErrorResponse | NodeStates>;
272-
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode>;
272+
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
273273
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
274274
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
275275
poll: (collector: PollingCollector) => Poller;
@@ -280,15 +280,15 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
280280
collectors: Collectors[];
281281
description?: string;
282282
name?: string;
283-
status: "error";
284-
} | {
285-
status: "failure";
283+
status: "continue";
286284
} | {
287285
action: string;
288286
collectors: Collectors[];
289287
description?: string;
290288
name?: string;
291-
status: "continue";
289+
status: "error";
290+
} | {
291+
status: "failure";
292292
} | {
293293
authorization?: {
294294
code?: string;
@@ -299,8 +299,16 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
299299
getCollectors: () => Collectors[];
300300
getError: () => DaVinciError | null;
301301
getErrorCollectors: () => CollectorErrors[];
302-
getNode: () => StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode;
302+
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
303303
getServer: () => {
304+
_links?: Links;
305+
id?: string;
306+
interactionId?: string;
307+
interactionToken?: string;
308+
href?: string;
309+
eventName?: string;
310+
status: "continue";
311+
} | {
304312
status: "start";
305313
} | {
306314
_links?: Links;
@@ -317,14 +325,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
317325
interactionId?: string;
318326
interactionToken?: string;
319327
status: "failure";
320-
} | {
321-
_links?: Links;
322-
id?: string;
323-
interactionId?: string;
324-
interactionToken?: string;
325-
href?: string;
326-
eventName?: string;
327-
status: "continue";
328328
} | {
329329
_links?: Links;
330330
eventName?: string;

‎packages/davinci-client/api-report/davinci-client.types.api.md‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
269269
resume: (input: {
270270
continueToken: string;
271271
}) => Promise<InternalErrorResponse | NodeStates>;
272-
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode>;
272+
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
273273
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
274274
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
275275
poll: (collector: PollingCollector) => Poller;
@@ -280,15 +280,15 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
280280
collectors: Collectors[];
281281
description?: string;
282282
name?: string;
283-
status: "error";
284-
} | {
285-
status: "failure";
283+
status: "continue";
286284
} | {
287285
action: string;
288286
collectors: Collectors[];
289287
description?: string;
290288
name?: string;
291-
status: "continue";
289+
status: "error";
290+
} | {
291+
status: "failure";
292292
} | {
293293
authorization?: {
294294
code?: string;
@@ -299,8 +299,16 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
299299
getCollectors: () => Collectors[];
300300
getError: () => DaVinciError | null;
301301
getErrorCollectors: () => CollectorErrors[];
302-
getNode: () => StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode;
302+
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
303303
getServer: () => {
304+
_links?: Links;
305+
id?: string;
306+
interactionId?: string;
307+
interactionToken?: string;
308+
href?: string;
309+
eventName?: string;
310+
status: "continue";
311+
} | {
304312
status: "start";
305313
} | {
306314
_links?: Links;
@@ -317,14 +325,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
317325
interactionId?: string;
318326
interactionToken?: string;
319327
status: "failure";
320-
} | {
321-
_links?: Links;
322-
id?: string;
323-
interactionId?: string;
324-
interactionToken?: string;
325-
href?: string;
326-
eventName?: string;
327-
status: "continue";
328328
} | {
329329
_links?: Links;
330330
eventName?: string;

‎packages/davinci-client/src/lib/client.store.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ import type {
5555
Validator,
5656
Poller,
5757
} from './client.types.js';
58-
import { returnPasswordPolicyValidator, returnValidator } from './collector.utils.js';
58+
import { returnValidator } from './collector.utils.js';
59+
import { returnPasswordPolicyValidator } from './password-policy.rules.js';
5960
import type { ContinueNode, StartNode } from './node.types.js';
6061

6162
/**
@@ -390,10 +391,6 @@ export async function davinci<ActionType extends ActionTypes = ActionTypes>({
390391
return handleUpdateValidateError('Collector not found', 'state_error', log.error);
391392
}
392393

393-
if (collectorToUpdate.type === 'ValidatedPasswordCollector') {
394-
return returnPasswordPolicyValidator(collectorToUpdate);
395-
}
396-
397394
if (collectorToUpdate.type === 'PasswordCollector') {
398395
return handleUpdateValidateError(
399396
'PasswordCollector cannot be validated; pass a ValidatedPasswordCollector',
@@ -402,6 +399,10 @@ export async function davinci<ActionType extends ActionTypes = ActionTypes>({
402399
);
403400
}
404401

402+
if (collectorToUpdate.type === 'ValidatedPasswordCollector') {
403+
return returnPasswordPolicyValidator(collectorToUpdate);
404+
}
405+
405406
if (
406407
collectorToUpdate.category !== 'ValidatedSingleValueCollector' &&
407408
collectorToUpdate.category !== 'ObjectValueCollector' &&

‎packages/davinci-client/src/lib/collector.utils.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
returnSubmitCollector,
1313
returnSingleValueCollector,
1414
returnPasswordCollector,
15-
returnPasswordPolicyValidator,
1615
returnValidatedPasswordCollector,
1716
returnTextCollector,
1817
returnSingleSelectCollector,
@@ -27,6 +26,7 @@ import {
2726
returnQrCodeCollector,
2827
returnAgreementCollector,
2928
} from './collector.utils.js';
29+
import { returnPasswordPolicyValidator } from './password-policy.rules.js';
3030
import type {
3131
DaVinciField,
3232
DeviceAuthenticationField,

‎packages/davinci-client/src/lib/collector.utils.ts‎

Lines changed: 8 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
* This software may be modified and distributed under the terms
55
* of the MIT license. See the LICENSE file for details.
66
*/
7-
import { Array as Arr, Option, pipe } from 'effect';
8-
97
/**
108
* Import the required types
119
*/
@@ -43,7 +41,6 @@ import type {
4341
FidoRegistrationField,
4442
MultiSelectField,
4543
PasswordField,
46-
PasswordPolicy,
4744
PhoneNumberField,
4845
ProtectField,
4946
QrCodeField,
@@ -190,9 +187,9 @@ export function returnSingleValueCollector<
190187
},
191188
} as InferSingleValueCollectorType<CollectorType>;
192189
} else if (collectorType === 'ValidatedPasswordCollector') {
193-
// passwordPolicy is the discriminator — callers must only request this variant when
194-
// the field carries one. Fallback to an empty object keeps the factory total in the
195-
// face of misuse; the policy validator will treat an empty policy as no rules.
190+
// Reducer routes by `field.type === 'PASSWORD_VERIFY'`, so a policy may or may not be
191+
// present. Fallback to an empty object keeps the factory total; the policy validator
192+
// treats an empty policy as no rules.
196193
const passwordPolicy =
197194
'passwordPolicy' in field && field.passwordPolicy ? field.passwordPolicy : {};
198195
const verify = 'verify' in field ? field.verify === true : false;
@@ -477,11 +474,11 @@ export function returnPasswordCollector(field: PasswordField, idx: number): Pass
477474
}
478475

479476
/**
480-
* @function returnValidatedPasswordCollector - Creates a ValidatedPasswordCollector. The
481-
* presence of a `passwordPolicy` on the incoming field is the sole discriminator between
482-
* this variant and {@link returnPasswordCollector}; callers are responsible for selecting
483-
* the right factory based on `field.passwordPolicy`.
484-
* @param {PasswordField} field - The PASSWORD / PASSWORD_VERIFY field carrying a passwordPolicy.
477+
* @function returnValidatedPasswordCollector - Creates a ValidatedPasswordCollector for
478+
* `PASSWORD_VERIFY` fields. The reducer routes by `field.type`, so this factory is the
479+
* target for every `PASSWORD_VERIFY`. When the field carries no `passwordPolicy`, the
480+
* collector is emitted with an empty policy and the validator treats it as no-op.
481+
* @param {PasswordField} field - The PASSWORD_VERIFY field; may or may not carry a passwordPolicy.
485482
* @param {number} idx - The index of the field in the form.
486483
* @returns {ValidatedPasswordCollector} The constructed ValidatedPasswordCollector.
487484
*/
@@ -888,96 +885,6 @@ export function returnValidator(
888885
};
889886
}
890887

891-
/**
892-
* A single policy check: given the policy and a candidate value, produce zero or more
893-
* human-readable error strings. Rules are pure and independent — new ones can be added
894-
* by extending `passwordPolicyRules` below.
895-
*/
896-
type PasswordPolicyRule = (policy: PasswordPolicy, value: string) => readonly string[];
897-
898-
const countChars = (value: string): ReadonlyMap<string, number> => {
899-
const counts = new Map<string, number>();
900-
for (const ch of value) counts.set(ch, (counts.get(ch) ?? 0) + 1);
901-
return counts;
902-
};
903-
904-
const formatLengthMessage = (min?: number, max?: number): string => {
905-
if (min != null && max != null) return `Password must be between ${min} and ${max} characters`;
906-
if (min != null) return `Password must be at least ${min} characters`;
907-
return `Password must be at most ${max} characters`;
908-
};
909-
910-
const lengthRule: PasswordPolicyRule = (policy, value) => {
911-
const length = policy.length;
912-
if (!length) return [];
913-
const { min, max } = length;
914-
if (min == null && max == null) return [];
915-
const outOfRange = (min != null && value.length < min) || (max != null && value.length > max);
916-
return outOfRange ? [formatLengthMessage(min, max)] : [];
917-
};
918-
919-
const minUniqueCharactersRule: PasswordPolicyRule = (policy, value) => {
920-
const min = policy.minUniqueCharacters;
921-
if (min == null) return [];
922-
return new Set(value).size < min
923-
? [`Password must contain at least ${min} unique characters`]
924-
: [];
925-
};
926-
927-
const maxRepeatedCharactersRule: PasswordPolicyRule = (policy, value) => {
928-
const max = policy.maxRepeatedCharacters;
929-
if (max == null) return [];
930-
const maxCount = pipe(
931-
countChars(value),
932-
(counts) => Array.from(counts.values()),
933-
Arr.reduce(0, (acc, n) => (n > acc ? n : acc)),
934-
);
935-
return maxCount > max ? [`Password cannot repeat any character more than ${max} times`] : [];
936-
};
937-
938-
const minCharactersRule: PasswordPolicyRule = (policy, value) => {
939-
if (!policy.minCharacters) return [];
940-
return pipe(
941-
Object.entries(policy.minCharacters),
942-
Arr.filterMap(([charset, min]) => {
943-
const members = new Set(charset);
944-
let hits = 0;
945-
for (const ch of value) if (members.has(ch)) hits += 1;
946-
return hits < min
947-
? Option.some(`Password must contain at least ${min} character(s) from "${charset}"`)
948-
: Option.none();
949-
}),
950-
);
951-
};
952-
953-
const passwordPolicyRules: readonly PasswordPolicyRule[] = [
954-
lengthRule,
955-
minUniqueCharactersRule,
956-
maxRepeatedCharactersRule,
957-
minCharactersRule,
958-
];
959-
960-
/**
961-
* @function returnPasswordPolicyValidator - Creates a validator function that checks a candidate
962-
* value against the `passwordPolicy` embedded on a `ValidatedPasswordCollector`. Rules mirror the
963-
* native SDKs: length bounds, minimum unique characters, maximum repeated character occurrences,
964-
* and per-charset minimums. Returns `[]` when no policy is present on the collector.
965-
* @param {ValidatedPasswordCollector} collector - The collector whose output may carry a passwordPolicy.
966-
* @returns {(value: string) => string[]} - A validator that returns human-readable error strings.
967-
*/
968-
export function returnPasswordPolicyValidator(
969-
collector: ValidatedPasswordCollector,
970-
): (value: string) => string[] {
971-
const policy = collector.output.passwordPolicy;
972-
return (value: string) =>
973-
policy
974-
? pipe(
975-
passwordPolicyRules,
976-
Arr.flatMap((rule) => rule(policy, value)),
977-
)
978-
: [];
979-
}
980-
981888
export function returnUnknownCollector(field: Record<string, unknown>, idx: number) {
982889
return {
983890
category: 'UnknownCollector',

‎packages/davinci-client/src/lib/mock-data/mock-form-fields.data.ts‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -206,43 +206,6 @@ export const obj = {
206206
region: 'CA',
207207
themeId: 'activeTheme',
208208
formId: 'f0cf83ab-f8f4-4f4a-9260-8f7d27061fa7',
209-
passwordPolicy: {
210-
id: '39cad7af-3c2f-4672-9c3f-c47e5169e582',
211-
environment: {
212-
id: '02fb4743-189a-4bc7-9d6c-a919edfe6447',
213-
},
214-
name: 'Standard',
215-
description: 'A standard policy that incorporates industry best practices',
216-
excludesProfileData: true,
217-
notSimilarToCurrent: true,
218-
excludesCommonlyUsed: true,
219-
maxAgeDays: 182,
220-
minAgeDays: 1,
221-
maxRepeatedCharacters: 2,
222-
minUniqueCharacters: 5,
223-
history: {
224-
count: 6,
225-
retentionDays: 365,
226-
},
227-
lockout: {
228-
failureCount: 5,
229-
durationSeconds: 900,
230-
},
231-
length: {
232-
min: 8,
233-
max: 255,
234-
},
235-
minCharacters: {
236-
'~!@#$%^&*()-_=+[]{}|;:,.<>/?': 1,
237-
'0123456789': 1,
238-
ABCDEFGHIJKLMNOPQRSTUVWXYZ: 1,
239-
abcdefghijklmnopqrstuvwxyz: 1,
240-
},
241-
populationCount: 1,
242-
createdAt: '2024-01-03T19:50:39.586Z',
243-
updatedAt: '2024-01-03T19:50:39.586Z',
244-
default: true,
245-
},
246209
isResponseCompatibleWithMobileAndWebSdks: true,
247210
fieldTypes: [
248211
'LABEL',

0 commit comments

Comments
 (0)