Add SDK methods for agent sub organization auth#523
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds organization-scoped authorization URLs and token switching to the JavaScript client. The async initialization is deferred to prevent state races. New organization auth APIs build parameterized authorize URLs and switch tokens across org boundaries using an ChangesOrganization-Scoped Authorization and Token Switching
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/javascript/src/__legacy__/client.ts (1)
869-877: ⚡ Quick winConsider applying the same RFC 6749 §2.3.1 logic to
refreshAccessToken.The
requestAccessTokenmethod now correctly omitsclient_idfrom the body when usingclient_secret_basic. However,refreshAccessToken(and similarlyrevokeAccessTokenat line 783) still unconditionally includesclient_idin the body. If a client configured withclient_secret_basicuses refresh tokens, it may encounter the same "MUST NOT use more than one authentication method" error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/javascript/src/__legacy__/client.ts` around lines 869 - 877, The refreshAccessToken body currently always includes client_id which can conflict with HTTP Basic auth; change refreshAccessToken (and similarly revokeAccessToken) to mirror requestAccessToken's RFC6749 §2.3.1 handling: when a client_secret is configured and HTTP Basic client authentication is used, do not include client_id in the POST body (leave authentication to the Authorization header), otherwise include client_id as before; keep pushing refresh_token and grant_type=refresh_token to the body and ensure any conditional check uses the same condition used in requestAccessToken (e.g., existence/non-empty configData.clientSecret or the same auth-method flag) so both refreshAccessToken and revokeAccessToken follow the same logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/src/AsgardeoJavaScriptClient.ts`:
- Around line 495-513: The TokenResponse.createdAt doc is incorrect: update the
documentation for TokenResponse.createdAt (in
packages/javascript/src/models/token.ts) to state the timestamp is a Unix
timestamp in milliseconds (not seconds), since the implementation in
AsgardeoJavaScriptClient.getToken/response mapping uses parsed.created_at ??
Date.now() (Date.now() returns milliseconds) and parsed.created_at is treated
the same; ensure the doc string explicitly says "milliseconds" to match the
createdAt value produced by that mapping.
- Around line 402-408: getOrgAuthorizationUrlWithPKCE currently just delegates
to getOrgAuthorizationUrl so it can return a URL without PKCE when the client
config has enablePKCE:false, violating its JSDoc guarantee; update
getOrgAuthorizationUrlWithPKCE to enforce PKCE by either validating client
config and throwing if enablePKCE is false or (preferable) forcing PKCE
generation by invoking getOrgAuthorizationUrl with options that ensure PKCE is
enabled (e.g., merge/override the incoming OrgAuthorizationUrlOptions to set the
PKCE-related flags/parameters required) so that getOrgAuthorizationUrlWithPKCE
always returns a URL containing PKCE parameters; reference
getOrgAuthorizationUrlWithPKCE, getOrgAuthorizationUrl and the client config
flag enablePKCE when making the change.
---
Nitpick comments:
In `@packages/javascript/src/__legacy__/client.ts`:
- Around line 869-877: The refreshAccessToken body currently always includes
client_id which can conflict with HTTP Basic auth; change refreshAccessToken
(and similarly revokeAccessToken) to mirror requestAccessToken's RFC6749 §2.3.1
handling: when a client_secret is configured and HTTP Basic client
authentication is used, do not include client_id in the POST body (leave
authentication to the Authorization header), otherwise include client_id as
before; keep pushing refresh_token and grant_type=refresh_token to the body and
ensure any conditional check uses the same condition used in requestAccessToken
(e.g., existence/non-empty configData.clientSecret or the same auth-method flag)
so both refreshAccessToken and revokeAccessToken follow the same logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ffefb884-c4c4-4874-ab35-ec0921344c2e
📒 Files selected for processing (6)
packages/javascript/src/AsgardeoJavaScriptClient.tspackages/javascript/src/__legacy__/client.tspackages/javascript/src/index.tspackages/javascript/src/models/agent.tspackages/javascript/src/models/organization.tstsconfig.json
HasiniSama
force-pushed
the
agent-sharing
branch
4 times, most recently
from
ac8d541 to
d077281
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/javascript/src/index.ts (1)
250-263:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBreaking change: Logger utility functions exported as type-only cannot be called at runtime.
The logger functions
createLogger,createComponentLogger,createPackageLogger,createPackageComponentLogger,configure,debug,info,warn, anderrorare runtime utilities that consumers call directly. Exporting them viaexport type { ... }makes them unavailable at runtime.🐛 Proposed fix: Separate value and type exports for logger
-export type { - default as logger, - createLogger, - createComponentLogger, - createPackageLogger, - createPackageComponentLogger, - LogLevel, - configure as configureLogger, - debug, - info, - warn, - error, -} from './utils/logger'; -export type {LoggerConfig} from './utils/logger'; +export { + default as logger, + createLogger, + createComponentLogger, + createPackageLogger, + createPackageComponentLogger, + configure as configureLogger, + debug, + info, + warn, + error, +} from './utils/logger'; +export type {LogLevel, LoggerConfig} from './utils/logger';Note: If
LogLevelis an enum that needs runtime access, it should also remain a value export.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/javascript/src/index.ts` around lines 250 - 263, The current export block uses "export type" which removes runtime availability of logger utilities; change the exports to include value exports for the runtime symbols (createLogger, createComponentLogger, createPackageLogger, createPackageComponentLogger, configure as configureLogger, debug, info, warn, error, and default logger) while keeping LoggerConfig and LogLevel types as types if needed; update the export statement in packages/javascript/src/index.ts to export the actual values (not type-only) so consumers can call those functions at runtime and ensure LogLevel remains a value export if it’s an enum.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/src/AsgardeoJavaScriptClient.ts`:
- Around line 567-569: When agent delegation is requested (options.agentConfig
is present) ensure options.agentConfig.agentID is validated (non-null, defined,
non-empty string) before assigning to customParam['requested_actor'] in
AsgardeoJavaScriptClient (the code that currently sets
customParam['requested_actor'] = options.agentConfig.agentID). If agentID is
missing or empty, throw a clear error or return a rejected promise indicating
the missing agentID so an invalid requested_actor value is never produced;
include the symbol names options.agentConfig, agentID, and
customParam['requested_actor'] to locate the change.
- Around line 411-417: The code destructures clientId from configData using an
unsafe double-cast and then calls body.set('client_id', clientId) which will
send "undefined" if clientId is missing; update the logic around configData and
clientId (and related clientSecret/hasSecret) to validate that clientId is a
non-empty string before calling body.set('client_id', ...), and if it is
missing/empty throw or return a clear error (or reject) so you never call
body.set with an undefined value; ensure you remove the unsafe "as unknown as
{clientId: string; ...}" cast and perform a runtime check (e.g., typeof clientId
=== 'string' && clientId.trim().length > 0) prior to using it.
In `@packages/javascript/src/index.ts`:
- Around line 35-49: Several API entries (getScim2Me, getSchemas,
getAllOrganizations, createOrganization, getMeOrganizations, getOrganization,
getBrandingPreference, executeEmbeddedUserOnboardingFlowV2) are exported as
type-only, so consumers get types but no runtime functions; change each to
export the value and the type separately (e.g., export { default as getScim2Me }
from './api/getScim2Me'; and then export type { GetScim2MeConfig } from
'./api/getScim2Me';) so the default function is available at runtime while
preserving the config/type exports; apply the same pattern for the entries
mentioned around the updateOrganization/updateMeProfile area (lines referenced
in the comment) so all API functions are value-exported and their types are
exported separately.
---
Outside diff comments:
In `@packages/javascript/src/index.ts`:
- Around line 250-263: The current export block uses "export type" which removes
runtime availability of logger utilities; change the exports to include value
exports for the runtime symbols (createLogger, createComponentLogger,
createPackageLogger, createPackageComponentLogger, configure as configureLogger,
debug, info, warn, error, and default logger) while keeping LoggerConfig and
LogLevel types as types if needed; update the export statement in
packages/javascript/src/index.ts to export the actual values (not type-only) so
consumers can call those functions at runtime and ensure LogLevel remains a
value export if it’s an enum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6be7a157-6598-4ab8-b48e-80f01b478175
📒 Files selected for processing (7)
packages/javascript/src/AsgardeoJavaScriptClient.tspackages/javascript/src/index.tspackages/javascript/src/models/agent.tspackages/javascript/src/models/organization.tspackages/javascript/src/models/token.tspackages/nextjs/src/AsgardeoNextClient.tstsconfig.json
✅ Files skipped from review due to trivial changes (2)
- tsconfig.json
- packages/javascript/src/models/token.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/javascript/src/models/agent.ts
- packages/javascript/src/models/organization.ts
🦋 Changeset detectedThe changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. |
pavinduLakshan
approved these changes
May 19, 2026
Contributor
Author
|
Fixes the audit warnings: #525 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
New method introduced:
Issue: wso2/product-is#27526
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation