-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/new password provided #38
Conversation
WalkthroughThe pull request introduces enhancements to the Changes
Possibly related PRs
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
d9ae7fe
to
8104fa1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/types.ts (2)
55-65
: Consider defining explicit binding interfaces.While the empty object type
{}
is used consistently throughout the file for bindings, it's generally recommended to define explicit interfaces for better type safety and documentation.- "kinde.secureFetch"?: {}; + "kinde.secureFetch"?: { + readonly fetch: (url: string, options: KindeFetchOptions) => Promise<unknown>; + }; - "kinde.widget"?: {}; + "kinde.widget"?: { + readonly invalidateFormField: (fieldName: string, message: string) => void; + };🧰 Tools
🪛 Biome (1.9.4)
[error] 57-57: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 61-61: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 65-65: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
151-165
: Enhance password validation context.The type properly captures password change scenarios, but consider:
- Adding validation rules as constants (e.g., minimum length, complexity)
- Including additional context like password policy version
- Adding JSDoc to document security requirements
export type onNewPasswordProvided = EventBase & { context: { auth: { firstPassword: string; secondPassword: string; newPasswordReason: "reset" | "initial"; + passwordPolicyVersion?: string; // Track which password rules were in effect + passwordValidationRules?: { + minLength: number; + requiresUppercase: boolean; + requiresNumbers: boolean; + requiresSpecialChars: boolean; + }; }; user: { id: string; }; workflow: { trigger: WorkflowTrigger.NewPasswordProvided; }; }; };lib/main.ts (1)
280-305
: Reduce code duplication in fetch implementations.The
secureFetch
function largely duplicates the logic from the regularfetch
function. Consider extracting the common logic into a shared helper function.+async function baseFetch<T>( + fetchFn: typeof kinde.fetch | typeof kinde.secureFetch, + url: string, + options: KindeFetchOptions +): Promise<T> { + if (!options.responseFormat) { + options.responseFormat = "json"; + } + + const result = await fetchFn(url, options); + + return { + data: result?.json, + } as T; +} + export async function fetch<T = any>( url: string, options: KindeFetchOptions, ): Promise<T> { if (!kinde.fetch) { throw new Error("fetch binding not available"); } - if (!options.responseFormat) { - options.responseFormat = "json"; - } - const result = await kinde.fetch(url, options); - return { - data: result?.json, - } as T; + return baseFetch(kinde.fetch, url, options); } export async function secureFetch<T = any>( url: string, options: KindeFetchOptions, ): Promise<T> { if (!kinde.secureFetch) { throw new Error("secureFetch binding not available"); } - if (!options.responseFormat) { - options.responseFormat = "json"; - } - const result = await kinde.secureFetch(url, options); - return { - data: result?.json, - } as T; + return baseFetch(kinde.secureFetch, url, options); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/main.ts
(3 hunks)lib/types.ts
(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
lib/types.ts
[error] 57-57: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 61-61: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 65-65: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (5)
lib/types.ts (3)
88-89
: LGTM! New workflow triggers follow consistent naming patterns.The new password-related triggers maintain the established naming convention and domain prefix pattern.
94-96
: LGTM! Event types properly integrated.The new password-related events are correctly added to the WorkflowEvents union type.
137-149
: Review security implications of password handling.The type includes plain text password data. While this might be necessary for validation, ensure that:
- The password is never logged or persisted
- The data is handled securely in memory
- The password field is cleared as soon as possible after validation
Consider adding JSDoc comments to document these security requirements.
lib/main.ts (2)
31-45
: LGTM! Namespace declarations are consistent.The new namespace declarations follow the established pattern and maintain type safety.
243-253
: LGTM! Clean implementation of form field validation.The function follows the established patterns for binding checks and error handling.
8104fa1
to
df220ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/types.ts (1)
Line range hint
42-43
: Fix typo in documentation comment.The documentation comment for
"kinde.widget"
contains a grammatical error.- * Exposes the fetch method to call access the manipulate the Kinde widget + * Exposes methods to manipulate the Kinde widget
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/types.ts
(2 hunks)
🔇 Additional comments (2)
lib/types.ts (2)
89-89
: LGTM!The new workflow trigger follows the established naming pattern and is appropriately prefixed with "user:" for user-related events.
95-96
: LGTM!The WorkflowEvents type is correctly updated to include the new event type while maintaining consistency.
Explain your changes
Add
new_password_provided
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.