fix: make workOS ID sync rely on correct ID space#2245
Conversation
The CTE was matching Speakeasy org UUIDs against WorkOS org IDs (org_...) which never matched in non-mock environments. Now the query accepts WorkOS org IDs directly and resolves them to Speakeasy org IDs via a JOIN on organization_metadata.workos_id. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the grants query fails (e.g. org membership not yet synced), the app no longer crashes. useRBAC now sets throwOnError: false and exposes the error state. A MembershipSyncGuard in both AppLayout and OrgLayout shows a recovery prompt suggesting the user log out and log back in to trigger re-synchronization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on in the case that the user winds up with a corrupt session.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🦋 Changeset detectedLatest commit: 1938353 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🚀 Preview Environment (PR #2245)Preview URL: https://pr-2245.dev.getgram.ai
Gram Preview Bot |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const MembershipSyncGuard = ({ children }: { children: React.ReactNode }) => { | ||
| const { error } = useRBAC(); | ||
| const client = useSdkClient(); | ||
|
|
||
| if (!error) return <>{children}</>; | ||
|
|
||
| return ( | ||
| <div className="flex h-full min-h-[400px] w-full items-center justify-center"> | ||
| <div className="flex max-w-md flex-col items-center gap-4 text-center"> | ||
| <div className="bg-muted flex h-12 w-12 items-center justify-center rounded-full"> | ||
| <Icon name="refresh-cw" className="text-muted-foreground h-5 w-5" /> | ||
| </div> | ||
| <h2 className="text-lg font-medium">Organization sync required</h2> | ||
| <p className="text-muted-foreground text-sm"> | ||
| Your organization membership needs to be re-synchronized. Please log | ||
| out and log back in to refresh your session. | ||
| </p> | ||
| <button | ||
| type="button" | ||
| className="bg-primary text-primary-foreground hover:bg-primary/90 mt-2 rounded-md px-4 py-2 text-sm font-medium" | ||
| onClick={async () => { | ||
| await client.auth.logout(); | ||
| window.location.href = "/login"; | ||
| }} | ||
| > | ||
| Log out | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🚩 MembershipSyncGuard blocks on any grants query error, not just membership issues
The MembershipSyncGuard at client/dashboard/src/components/app-layout.tsx:162-192 checks error from useRBAC() and blocks the entire content area with an "Organization sync required" message plus a logout button. However, error reflects ANY failure from the grants API — including transient network errors, server 500s, or rate limiting — not just broken org memberships. React Query's default 3 retries mitigate brief transient issues, but a sustained server outage would present a misleading error message asking users to log out (which won't help). Consider either (a) inspecting the error status code to distinguish membership errors from transient ones, or (b) adding a retry/refresh button alongside the logout option.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const { data, isLoading, error } = useGrants(undefined, undefined, { | ||
| staleTime: 30_000, | ||
| throwOnError: false, | ||
| }); |
There was a problem hiding this comment.
🔴 Grants query always enabled blocks all non-WorkOS org users with an error screen
Removing enabled: isRbacEnabled from useGrants causes the grants API call to fire for every user, regardless of whether RBAC is active. The server's ListGrants endpoint returns a 400 error for any organization without a workos_id set (server/internal/access/impl.go:783-784: "organization is not linked to WorkOS"). The MembershipSyncGuard in app-layout.tsx:166 checks only error (not isRbacEnabled), so it will block the entire app content for all non-WorkOS org users — including non-enterprise customers and local dev with the mock IDP in non-OIDC mode (where SSOConnectionID is nil per mock-speakeasy-idp/mockidp.go:896, so workos_id is never backfilled). These users see "Organization sync required" with only a logout button, effectively locking them out of the product.
Server-side error path for non-WorkOS orgs
In server/internal/access/impl.go:773-785, roleOrgContext checks:
if !org.WorkosID.Valid || org.WorkosID.String == "" {
return nil, "", oops.E(oops.CodeBadRequest, nil, "organization is not linked to WorkOS")
}This 400 error is caught by the client as a query error, which MembershipSyncGuard treats as a fatal membership issue.
Prompt for agents
The grants query was changed from conditionally enabled (only when isRbacEnabled) to always enabled. This causes the ListGrants endpoint to be called for all users, including those on non-WorkOS organizations where it returns a 400 error. The MembershipSyncGuard then blocks the entire app.
Two possible fixes:
1. Restore the `enabled: isRbacEnabled` option on the useGrants call in useRBAC.ts, and only expose the error when RBAC is enabled. This preserves the original behavior where non-RBAC users never call the endpoint.
2. Alternatively, update MembershipSyncGuard in app-layout.tsx to also check isRbacEnabled from useRBAC() before showing the error screen — i.e. only show the guard when RBAC is enabled AND the grants query fails. This way the guard is a no-op for non-enterprise / non-WorkOS users.
Option 1 is the simpler and safer fix. The relevant files are client/dashboard/src/hooks/useRBAC.ts (useGrants call around line 46) and client/dashboard/src/components/app-layout.tsx (MembershipSyncGuard around line 162).
Was this helpful? React with 👍 or 👎 to provide feedback.
| for _, org := range validateResp.Organizations { | ||
| idx, ok := membershipByOrgID[org.ID] | ||
| if !ok { | ||
| if org.SSOConnectionID == nil { | ||
| continue | ||
| } | ||
| orgMembership := memberships[idx] | ||
|
|
||
| // Link the organization to its WorkOS org ID if not already linked. | ||
| if _, err := s.orgRepo.SetOrgWorkosID(ctx, orgRepo.SetOrgWorkosIDParams{ | ||
| WorkosID: conv.ToPGText(orgMembership.OrganizationID), | ||
| WorkosID: conv.ToPGText(*org.SSOConnectionID), | ||
| OrganizationID: org.ID, | ||
| }); err != nil { | ||
| // SetOrgWorkosID only updates when workos_id IS NULL, so | ||
| // pgx.ErrNoRows means it was already linked — not an error. | ||
| if !errors.Is(err, pgx.ErrNoRows) { | ||
| s.logger.ErrorContext(ctx, "failed to set org workos ID", attr.SlogError(err)) | ||
| } | ||
| }); err != nil && !errors.Is(err, pgx.ErrNoRows) { | ||
| s.logger.ErrorContext(ctx, "failed to set org workos ID", attr.SlogError(err)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 SetOrgWorkosID now stores SSOConnectionID instead of WorkOS membership OrganizationID
The old code in speakeasyconnections.go stored orgMembership.OrganizationID (from WorkOS memberships) as workos_id. The new code at line 449 stores *org.SSOConnectionID (from the Speakeasy provider response). I verified via mock-speakeasy-idp/claims.go:41 that in OIDC mode, SSOConnectionID is set to the WorkOS org ID (claims.OrgID). Additionally, the new code only backfills workos_id for orgs with SSOConnectionID != nil, whereas the old code backfilled for any org matching a WorkOS membership. This means non-SSO orgs no longer get their workos_id auto-populated during login. The test setup compensates by pre-seeding workos_id (speakeasyconnections_test.go:198-202). This is a deliberate design change, not a bug, but it does change the production behavior: non-SSO orgs must have their workos_id set through another mechanism.
Was this helpful? React with 👍 or 👎 to provide feedback.
qstearns
force-pushed
the
fix/workos-id-sync
branch
from
04cf9ed to
b4951a4
Compare
| const { error } = useRBAC(); | ||
| const client = useSdkClient(); | ||
|
|
||
| if (!error) return <>{children}</>; |
There was a problem hiding this comment.
🟡 MembershipSyncGuard shows misleading "Organization sync required" message for any grants query failure
The MembershipSyncGuard at client/dashboard/src/components/app-layout.tsx:166 checks if (!error) without inspecting the error type or status code. Any failure of the listGrants API — including server 500s, network timeouts, or rate limiting — will display "Your organization membership needs to be re-synchronized. Please log out and log back in to refresh your session." with a logout button. This is misleading when the root cause is not a corrupt session; the user is directed to log out unnecessarily. While React Query's default 3 retries mitigate very transient issues, a sustained (but temporary) server problem surviving retries would still trigger this incorrect diagnosis. The guard should distinguish membership-specific errors (e.g., 403/404) from generic failures, and show a generic retry/error message for non-membership errors.
Prompt for agents
In MembershipSyncGuard (client/dashboard/src/components/app-layout.tsx:162-192), the component treats all errors from useRBAC() the same way, showing an 'Organization sync required' message and a logout button. This is misleading for transient server errors (500s, timeouts) that have nothing to do with org membership.
The fix should inspect the error type or HTTP status code to distinguish membership-related errors (e.g. 403 Forbidden, 404 Not Found from the listGrants endpoint indicating the user's role/org relationship is broken) from generic server errors (500, network errors). For membership errors, the current UI is appropriate. For other errors, either show a generic error message with a retry button, or don't block the outlet at all and let the downstream pages handle their own error states.
The error types are defined in client/sdk/src/react-query/grants.ts (GrantsQueryError union type) and include ServiceError, GramError, ConnectionError, etc. You can check the status code on ServiceError or GramError to determine if it's a membership issue.
Was this helpful? React with 👍 or 👎 to provide feedback.
The previous refactor dropped the SetOrgWorkosID call, leaving no production code path to initially populate workos_id on organization_metadata. This restores it: for each org with an SSOConnectionID in the validate response, we attempt to set workos_id before the CTE runs, so new orgs get linked on first login. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
qstearns
force-pushed
the
fix/workos-id-sync
branch
from
b4951a4 to
e9fac2e
Compare
Dangerous for the mock IDP to trick the robot into thinking workos ID and org ID are the same
3 participants
The previous iteration of this PR relied on a behavior of the Mock IDP that the workOS ID and the organization ID would be the same which is certainly not an assumption that holds in prod. This makes the behavior consistently use the WorkOS ID to reference the organizations.
We also add a better error handler to the dashboard in cases where we detect that the user is not set up with their SSO organization. We prompt a log out, which is required to cause another synchronization to capture new memberships from WorkOS