-
Notifications
You must be signed in to change notification settings - Fork 7
feat: setup switch org method and tests #157
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as App
participant SO as switchOrg
participant GA as generateAuthUrl
Caller->>SO: switchOrg({ domain, orgCode, redirectURL })
alt missing domain/orgCode/redirectURL
SO-->>Caller: throw Error("<field> is required")
else valid inputs
SO->>GA: generateAuthUrl(domain, IssuerRouteTypes.login, { prompt: PromptTypes.none, orgCode, redirectURL })
GA-->>SO: { url, state, nonce, codeChallenge, codeVerifier }
SO-->>Caller: { url, state, nonce, codeChallenge, codeVerifier }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 (6)
lib/utils/switchOrg.test.ts (2)
26-38
: Error message assertions are brittle by exact string.Consider matching by RegExp or inline snapshot to reduce churn if wording changes slightly.
-).rejects.toThrow("Org code is required for switchOrg"); +).rejects.toThrow(/Org code is required/i);
63-69
: Nice: URL param-level assertions.Optional: also assert code_challenge_method=S256 to lock in PKCE expectations from the mock.
+ expect(result.url.searchParams.get("code_challenge_method")).toBe("S256");
lib/utils/switchOrg.ts (4)
4-8
: Export the params type for consumers.Exposing the shape improves DX and avoids anonymous inlined types in d.ts.
-interface SwitchOrgParams { +export interface SwitchOrgParams { domain: string; orgCode: string; redirectURL: string; }
10-14
: TSDoc: fix @param usage and add param names.Current tag is unhelpful for IDE hovers.
-/** - * - * @param SwitchOrgParams - * @returns URL to redirect to which will auto switch the user's active organization without prompting - */ +/** + * Builds a silent-login URL to switch the active organization without prompting. + * + * @param params.domain Kinde auth domain (e.g. https://your-domain.kinde.com) + * @param params.orgCode Organization code to switch to + * @param params.redirectURL Callback URL after auth completes + * @returns URL + PKCE metadata from generateAuthUrl + */
26-33
: Consider validating domain as well.A falsy or malformed domain will defer failure downstream. At minimum, check presence; optionally, parse with URL for early feedback.
}> => { + if (!domain) { + throw new Error("Domain is required for switchOrg"); + } if (!orgCode) { throw new Error("Org code is required for switchOrg"); } if (!redirectURL) { throw new Error("Redirect URL is required for switchOrg"); }
34-41
: Avoid unnecessary await on return.Returning the promise directly is equivalent and slightly cleaner.
- return await generateAuthUrl(domain, IssuerRouteTypes.login, loginOptions); + return generateAuthUrl(domain, IssuerRouteTypes.login, loginOptions);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/utils/switchOrg.test.ts
(1 hunks)lib/utils/switchOrg.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-25T23:53:26.124Z
Learnt from: DanielRivers
PR: kinde-oss/js-utils#15
File: lib/utils/generateAuthUrl.test.ts:104-104
Timestamp: 2024-10-25T23:53:26.124Z
Learning: In `lib/utils/generateAuthUrl.test.ts`, the code is test code and may not require strict adherence to PKCE specifications.
Applied to files:
lib/utils/switchOrg.test.ts
📚 Learning: 2024-10-08T23:57:58.113Z
Learnt from: DanielRivers
PR: kinde-oss/js-utils#8
File: lib/utils/token/testUtils/index.ts:3-39
Timestamp: 2024-10-08T23:57:58.113Z
Learning: The file `lib/utils/token/testUtils/index.ts` is a test utility, not production code.
Applied to files:
lib/utils/switchOrg.test.ts
🧬 Code graph analysis (2)
lib/utils/switchOrg.test.ts (1)
lib/utils/switchOrg.ts (1)
switchOrg
(15-41)
lib/utils/switchOrg.ts (1)
lib/types.ts (1)
LoginOptions
(111-228)
🔇 Additional comments (3)
lib/utils/switchOrg.test.ts (3)
4-16
: Mock setup looks good.Module factory + post-mock import pattern is correct, deterministic, and isolated.
40-56
: Great behavioral verification of generateAuthUrl call.Asserting the route, prompt=none, and payload wiring provides solid coverage for the happy path.
1-3
: Confirmed: PromptTypes and IssuerRouteTypes are exported enums (runtime values); imports will work at runtime.
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 (5)
lib/utils/switchOrg.ts (5)
1-1
: Use a type-only import for LoginOptionsPrevents an unnecessary value import in emitted JS.
-import { IssuerRouteTypes, LoginOptions, PromptTypes } from "../types"; +import { IssuerRouteTypes, PromptTypes, type LoginOptions } from "../types";
4-9
: Export SwitchOrgParamsHandy for consumers and keeps the public API typed.
-interface SwitchOrgParams { +export interface SwitchOrgParams { domain: string; orgCode: string; redirectURL: string; }
10-14
: Fix JSDoc @param usage and document throwsClearer docs and better editor hints.
/** - * - * @param SwitchOrgParams - * @returns URL to redirect to which will auto switch the user's active organization without prompting + * Switch the active organization without prompting. + * @param params - { domain, orgCode, redirectURL } + * @returns OAuth URL bundle to redirect to (auto-switches active org). + * @throws Error when domain, orgCode, or redirectURL are missing. */
15-25
: Derive return type and drop async/awaitKeeps types in sync with generateAuthUrl and removes an extra microtask.
-export const switchOrg = async ({ +export const switchOrg = ({ domain, orgCode, redirectURL, -}: SwitchOrgParams): Promise<{ - url: URL; - state: string; - nonce: string; - codeChallenge: string; - codeVerifier: string; -}> => { +}: SwitchOrgParams): Promise<Awaited<ReturnType<typeof generateAuthUrl>>> => { @@ - return await generateAuthUrl(domain, IssuerRouteTypes.login, loginOptions); + return generateAuthUrl(domain, IssuerRouteTypes.login, loginOptions);Also applies to: 40-41
26-33
: Also validate domain; align error text with param namesPrevents passing an empty domain and standardizes messages.
+ if (!domain) { + throw new Error("domain is required for switchOrg"); + } if (!orgCode) { - throw new Error("Org code is required for switchOrg"); + throw new Error("orgCode is required for switchOrg"); } if (!redirectURL) { - throw new Error("Redirect URL is required for switchOrg"); + throw new Error("redirectURL is required for switchOrg"); }Please confirm whether domain should be a bare tenant domain (e.g., "acme.kinde.com") or a full URL. If a full URL is expected, we can add a URL parse/https check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/utils/switchOrg.test.ts
(1 hunks)lib/utils/switchOrg.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/utils/switchOrg.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/utils/switchOrg.ts (1)
lib/types.ts (1)
LoginOptions
(111-228)
🔇 Additional comments (1)
lib/utils/switchOrg.ts (1)
34-38
: LGTM: correct options (prompt=none, orgCode, redirectURL)Matches the intended silent org switch behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 (4)
lib/utils/switchOrg.ts (4)
4-8
: Consider documenting param fields.Brief JSDoc on each field (expected formats, examples) will reduce misuse, e.g., whether
domain
expects a full host vs. a base tenant domain and whetherredirectURL
must be absolute.
10-15
: Tighten the JSDoc wording.Nit: explicitly list what the “OAuth URL bundle” contains (url, state, nonce, codeChallenge, codeVerifier) to help callers.
16-20
: Simplify the return type.
ReturnType<typeof generateAuthUrl>
already captures the promise type;Promise<Awaited<...>>
is redundant.Apply this diff:
-}: SwitchOrgParams): Promise<Awaited<ReturnType<typeof generateAuthUrl>>> => { +}: SwitchOrgParams): ReturnType<typeof generateAuthUrl> => {
21-38
: Harden input validation (trim, absolute URL) and pass normalized values.Current checks allow whitespace-only strings and don’t validate URL shape. Trimming and asserting an absolute URL prevents subtle bugs.
Apply this diff:
export const switchOrg = ({ domain, orgCode, redirectURL, }: SwitchOrgParams): Promise<Awaited<ReturnType<typeof generateAuthUrl>>> => { - if (!domain) { + const domainTrimmed = domain?.trim(); + const orgCodeTrimmed = orgCode?.trim(); + const redirectURLTrimmed = redirectURL?.trim(); + + if (!domainTrimmed) { throw new Error("domain is required for switchOrg"); } - if (!orgCode) { + if (!orgCodeTrimmed) { throw new Error("orgCode is required for switchOrg"); } - if (!redirectURL) { + if (!redirectURLTrimmed) { throw new Error("redirectURL is required for switchOrg"); } + // Must be an absolute URL + try { + // eslint-disable-next-line no-new + new URL(redirectURLTrimmed); + } catch { + throw new Error("redirectURL must be an absolute URL for switchOrg"); + } const loginOptions: LoginOptions = { prompt: PromptTypes.none, - orgCode, - redirectURL, + orgCode: orgCodeTrimmed, + redirectURL: redirectURLTrimmed, }; - return generateAuthUrl(domain, IssuerRouteTypes.login, loginOptions); + return generateAuthUrl(domainTrimmed, IssuerRouteTypes.login, loginOptions); };If you apply the earlier return-type simplification, update the function signature accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/utils/switchOrg.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/utils/switchOrg.ts (1)
lib/types.ts (1)
LoginOptions
(111-228)
🔇 Additional comments (1)
lib/utils/switchOrg.ts (1)
1-3
: Imports look clean and correct.Tree-shake friendly (type-only import), and dependencies are minimal.
Explain your changes
Setup new method to allow users to call switchOrg with their domain and organization and change org without prompting.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.