feat: add support for onExistingPasswordProvided workflow event
#35
Conversation
WalkthroughThis pull request introduces new functionalities to the Kinde library by adding two key methods in 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 (
|
DanielRivers
force-pushed
the
feat/existing_password_workflow
branch
from
02f2be6 to
30c5918
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🛑 Comments failed to post (3)
lib/types.ts (1)
58-65: 🛠️ Refactor suggestion
Fix typos in documentation comments and define binding interfaces.
The documentation comments contain typos and the binding types should be explicitly defined:
- "extenal" should be "external" in the documentation
- Empty object types (
{}) should be replaced with explicit interfaces/** - * Exposes the fetch method to call signed extenal APIs to the workflow + * Exposes the fetch method to call signed external APIs to the workflow */ - "kinde.secureFetch"?: {}, + "kinde.secureFetch"?: { + readonly signedRequests: boolean; + }; /** - * Exposes the fetch method to call access the manipulate the Kinde widget + * Exposes methods to manipulate the Kinde widget */ - "kinde.widget"?: {}, + "kinde.widget"?: { + readonly formValidation: boolean; + };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Exposes the fetch method to call signed external APIs to the workflow */ "kinde.secureFetch"?: { readonly signedRequests: boolean; }; /** * Exposes methods to manipulate the Kinde widget */ "kinde.widget"?: { readonly formValidation: boolean; };🧰 Tools
🪛 Biome (1.9.4)
[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)
🪛 GitHub Actions: Build and test
[warning] Code formatting issues detected. Run Prettier with --write to fix.
lib/main.ts (2)
276-301:
⚠️ Potential issueFix incorrect fetch method call.
There's a bug in the implementation: the function checks for
kinde.secureFetchbut useskinde.fetch.- const result = await kinde.fetch(url, options); + const result = await kinde.secureFetch(url, options);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Fetch data from a secure external API * * Encryption keys can be setup in the Kinde dashboard under workflows > encryption keys * @param url URL of the API * @param options Fetch options */ // eslint-disable-next-line @typescript-eslint/no-explicit-any 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; }🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues detected. Run Prettier with --write to fix.
31-41: 🛠️ Refactor suggestion
Improve type safety in namespace declarations.
The function signatures use
anytype which is flagged by eslint. Consider using generic types for better type safety.- export function secureFetch(url: string, options: unknown): Promise<any>; + export function secureFetch<T = unknown>(url: string, options: KindeFetchOptions): Promise<T>; namespace widget { - export function invalidateFormField(fieldName: string, message: string): void; + export function invalidateFormField(fieldName: keyof HTMLFormControlsCollection, message: string): void; }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 eslint
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Actions: Build and test
[warning] Code formatting issues detected. Run Prettier with --write to fix.
DanielRivers
force-pushed
the
feat/existing_password_workflow
branch
from
30c5918 to
3e5c203
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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] 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-88: LGTM!The new enum value follows the established naming pattern and is self-documenting.
93-94: LGTM!The WorkflowEvents union type is correctly updated to include the new event type.
135-147: Review security implications of password handling.Storing passwords in the event context could pose security risks. Consider:
- Using a one-time token instead of the actual password
- Adding documentation about secure password handling
- Implementing automatic cleanup of sensitive data
Run this script to check for other instances of password handling:
lib/main.ts (2)
31-32: LGTM!The secureFetch declaration follows the established pattern of the fetch method.
39-46: LGTM!The widget namespace and invalidateFormField implementation follow the established patterns with proper error handling.
Also applies to: 243-253
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/types.ts (1)
63-64: Fix typo in comment.The comment has grammatical errors.
- * Exposes the fetch method to call access the manipulate the Kinde widget + * Exposes methods to manipulate the Kinde widgetlib/main.ts (1)
280-305: Consider enhancing encryption key setup documentation.The implementation looks good, but the documentation could be more helpful.
/** * Fetch data from a secure external API * - * Encryption keys can be setup in the Kinde dashboard under workflows > encryption keys + * To use this feature: + * 1. Set up encryption keys in the Kinde dashboard: Workflows > Encryption Keys + * 2. Add the "kinde.secureFetch" binding to your workflow settings * @param url URL of the API * @param options Fetch 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
📓 Learnings (1)
lib/types.ts (1)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#35
File: lib/types.ts:58-65
Timestamp: 2025-01-15T20:41:03.218Z
Learning: In the Kinde infrastructure, the `kinde.secureFetch` and `kinde.widget` bindings in `WorkflowSettings` are designed as simple feature flags using empty object types `{}`. They should not contain additional configuration properties.
🪛 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 (7)
lib/types.ts (4)
55-55: LGTM!Fixed typo in comment from "extenal" to "external".
88-88: LGTM!The new enum value follows the existing naming pattern and is descriptive.
93-94: LGTM!The new event type is correctly added to the union type, maintaining type safety.
135-147: Review security implications of password handling.The event includes the password in its context. While this might be necessary for the workflow, ensure that:
- The password is not logged or persisted anywhere
- The password is transmitted securely
- The password is cleared from memory after use
Run this script to check for potential password logging:
✅ Verification successful
Type definition appears to be safely structured
The
onExistingPasswordProvidedtype is only used in type definitions and follows the standard event structure pattern in the codebase. No actual implementation code was found that could potentially mishandle the password. The password field is properly scoped within the auth context of the event.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential password logging in the codebase # Test: Search for logging of password or context. Expect: No instances of password being logged. echo "Checking for direct password logging..." rg -i "console\.(log|info|debug|warn|error).*password" echo "Checking for context logging that might expose password..." rg -i "console\.(log|info|debug|warn|error).*context"Length of output: 328
Script:
#!/bin/bash # Expand search for password handling patterns echo "Checking for logging library imports..." rg -i "import.*log|require.*log" echo "Checking password-related function calls..." ast-grep --pattern 'password' echo "Checking for password cleanup patterns..." rg -i "password.*=.*null|password.*=.*undefined" echo "Checking for secure transmission patterns..." rg -i "https|ssl|tls|secure|encrypt"Length of output: 73926
Script:
#!/bin/bash # Search for usage of the event type and password handling echo "Checking for event type usage..." rg -l "onExistingPasswordProvided" echo "Checking for password-related functions..." ast-grep --pattern 'function $FNAME($$$) { $$$ password $$$ }' echo "Checking for event handlers..." rg -A 5 "ExistingPasswordProvided"Length of output: 1217
lib/main.ts (3)
31-32: LGTM!The
secureFetchdeclaration follows the same pattern as the existingfetchdeclaration.
39-46: LGTM!The
widgetnamespace declaration is well-structured and follows the existing pattern.
243-253: LGTM!The implementation is well-documented and includes proper error handling.
Explain your changes
Add support for
onExistingPasswordProvidedwhich includeskinde.secureFetchandkinde.widgetsupportChecklist
🛟 If you need help, consider asking for advice over in the Kinde community.