feat: implement team endpoints with WorkOS invitation API#2003
feat: implement team endpoints with WorkOS invitation API#2003
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: ebdc0f9 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 |
There was a problem hiding this comment.
📝 Info: Orphaned servechatattachmentsignedform.ts uses zod/v3 while rest of SDK uses zod/v4-mini
The file client/sdk/src/models/components/servechatattachmentsignedform.ts was added in this PR but imports from zod/v3 while every other file in the SDK uses zod/v4-mini. Additionally, this file is not exported from the components index (client/sdk/src/models/components/index.ts) and is not imported by any other file. It appears to be an orphaned artifact from code generation. Since the file is unreferenced, it has no runtime impact, but the inconsistent zod version could cause issues if it's exported in the future.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment has been minimized.
This comment has been minimized.
3519566 to
f455967
Compare
| // Also soft-delete the local relationship | ||
| if err := s.orgRepo.DeleteOrganizationUserRelationship(ctx, orgRepo.DeleteOrganizationUserRelationshipParams{ | ||
| OrganizationID: authCtx.ActiveOrganizationID, | ||
| UserID: payload.UserID, | ||
| }); err != nil { | ||
| s.logger.ErrorContext(ctx, "failed to delete local org-user relationship after WorkOS removal", | ||
| attr.SlogError(err), | ||
| attr.SlogOrganizationID(authCtx.ActiveOrganizationID), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🚩 RemoveMember performs WorkOS deletion before local DB cleanup — partial failure possible
In RemoveMember at server/internal/teams/impl.go:418-433, the WorkOS membership is deleted first, then the local organization_user_relationships row is soft-deleted. If the local DB delete fails (line 425-433), the error is only logged — the function returns nil (success). This means the WorkOS state and local DB can become inconsistent: the user is removed from WorkOS but still appears as a member locally. The current approach (log-and-continue) is a pragmatic choice since the WorkOS deletion already succeeded and can't be rolled back, but this inconsistency could cause issues if ListMembers relies solely on the local DB (which it does, at line 171). Consider adding a background reconciliation mechanism or at minimum returning an error that indicates partial success.
Was this helpful? React with 👍 or 👎 to provide feedback.
Add Goa design + generated code for 7 team endpoints: ListMembers, InviteMember, ListInvites, CancelInvite, ResendInvite, GetInviteInfo, RemoveMember. All implementations return "not implemented" — ready for WorkOS API calls to be wired in. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up all 7 stubbed team service endpoints to WorkOS user management: ListMembers, InviteMember, ListInvites, CancelInvite, ResendInvite, GetInviteInfo, and RemoveMember. Add WorkOS wrapper methods for the invitation lifecycle and org membership deletion, with pagination support for list operations. Include httptest-based tests for all WorkOS client methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Pass db to auth.New() to prevent latent nil-pointer dereference - Add org-scoped authorization to CancelInvite and ResendInvite (IDOR fix) - Return Gram-internal user IDs from ListMembers via GetUserByWorkosID lookup - Validate payload.OrganizationID matches active session org - Add requireWorkOS() helper for proper service error on nil WorkOS client - Add GetInvitation and GetUserByWorkosID methods for authorization checks - Fix exhaustruct, paralleltest, and testifylint lint violations - Regenerate Speakeasy SDK for new team endpoints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ResendInvite now calls resolveInviterName instead of returning empty string - Add integration tests for all team service endpoints covering: - Authorization flow (org access validation, IDOR protection) - Self-removal prevention - WorkOS-not-configured error handling - Inviter name resolution - Add NewTestManagerWithWorkOS helper for injecting WorkOS in tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…est routing - InviteMember now uses resolveInviterName() instead of raw email for the InvitedBy field, consistent with ListInvites and ResendInvite - Fix test ServeMux routing: register /resend sub-path as separate handler since Go's ServeMux requires exact path matches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Aligns with codebase convention of including org ID, user ID, and invite ID in .Log() calls for better production observability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ListMembers now uses ListOrgMemberships instead of ListUsersInOrg, so JoinedAt reflects when the user joined the org, not when their account was created - Restore SDK version to 0.32.12 (was regressed to 0.32.8 during rebase) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ListMembers now queries organization_user_relationships joined with users table, eliminating the WorkOS API call. This gives correct JoinedAt timestamps, avoids the unsynced-user skip problem, and reduces external API dependencies. Also seeds org-user relationship in InitAuthContext test helper to match the production login flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change status enum from "cancelled" to "revoked" to match WorkOS InvitationState values, avoiding the need for a mapping layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collect unique inviter user IDs and resolve each once, avoiding N+1 WorkOS API calls when multiple invites share the same inviter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…haustruct lint The FE labels the section "Pending Invites" and doesn't filter client-side, so the API should only return pending invites. Also adds missing struct fields to satisfy the exhaustruct linter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The inviter is always a Gram user (they logged in to send the invite), so we can look up their display_name via GetUserByWorkosID instead of making an external WorkOS GetUser API call per inviter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests that don't exercise WorkOS code paths (ListMembers, org mismatch checks, self-removal) now pass nil instead of spinning up a mock server. Remove unused workos field from testInstance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c538280 to
f7c5a78
Compare
- Use consistent invite IDs (inv_01, inv_02) instead of random names from iterative churn (inv_abc, inv_other, inv_foreign, inv_resend) - Extract testWorkOSInviterID constant for inviter WorkOS user IDs - Remove unused sessionManager field from testInstance - Consolidate WorkOS-not-configured test into TestInviteMember subtests - Add inviter name assertion to InviteMember test - Add org name assertion to GetInviteInfo test - Add revoked invite to ListInvites test data for better filtering coverage - Reuse single userRepo.Queries instance in seed helpers - Add comment explaining why seedWorkOSIDs is needed in GetInviteInfo - Use consistent org ID "org_different" in IDOR tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c8b8a2d to
ebdc0f9
Compare
|
|
||
| // NewForTest creates a WorkOS client backed by a custom usermanagement.Client. | ||
| // This is intended for use in tests where the endpoint and HTTP client are overridden. | ||
| func NewForTest(logger *slog.Logger, client *usermanagement.Client) *WorkOS { |
There was a problem hiding this comment.
Would you consider using the other WorkOS client we created in roles.go? My goal is to merge these 2 eventually, and if you use that one you can pass a custom client with opts instead of having this extra method:
type RoleClientOpts struct {
// Endpoint overrides the WorkOS base URL for both raw HTTP and SDK calls.
Endpoint string
// HTTPClient overrides the default retryable HTTP client.
HTTPClient *http.Client
}
| return &user.Data[0], nil | ||
| } | ||
|
|
||
| func (w *WorkOS) ListUsersInOrg(ctx context.Context, workOSOrgID string) ([]usermanagement.User, error) { |
There was a problem hiding this comment.
As per a comment in a previous PR, we shouldn't expose WorkOS types in our client - we should create our own types to wrap around it.
It may seem like a pain to manage these types, but they prevent leaky abstractions, and let you define the data model that's relevant for your app, instead of having to rely on external ones.
| } | ||
|
|
||
| func (w *WorkOS) ResendInvitation(ctx context.Context, invitationID string) (usermanagement.Invitation, error) { | ||
| if w == nil { |
There was a problem hiding this comment.
FWIW we shouldn't be spreading these checks through every method we have. I've created a stub.go that creates a stub workos client when we don't provide an API key to avoid creating nil clients.
If you want you can use that and start getting rid of these checks - if not I'm going to refactor this workos stuff a bit later anyway.
| sessions *sessions.Manager | ||
| auth *auth.Auth | ||
| orgRepo *orgRepo.Queries | ||
| userRepo *userRepo.Queries |
There was a problem hiding this comment.
Don't store queries in service struct (we need to add this in our skills), store the db connection and init queries when needed
| // getAuthContext extracts and validates the auth context from the request. | ||
| func (s *Service) getAuthContext(ctx context.Context) (*contextvalues.AuthContext, error) { | ||
| authCtx, ok := contextvalues.GetAuthContext(ctx) | ||
| if !ok || authCtx == nil || authCtx.ActiveOrganizationID == "" { |
There was a problem hiding this comment.
Got the same feedback - at this point ActiveOrganizationID should always be present
|
|
||
| // requireWorkOS returns the WorkOS client or a proper service error if not configured. | ||
| func (s *Service) requireWorkOS() (*workos.WorkOS, error) { | ||
| w := s.sessions.WorkOS() |
There was a problem hiding this comment.
Why are we storing workOS inside a session instead of injecting it in the service struct as a dependency?
| TeamInviteIDKey = attribute.Key("gram.team.invite.id") | ||
| TeamInviteStatusKey = attribute.Key("gram.team.invite.status") |
There was a problem hiding this comment.
| TeamInviteIDKey = attribute.Key("gram.team.invite.id") | |
| TeamInviteStatusKey = attribute.Key("gram.team.invite.status") | |
| TeamInviteIDKey = attribute.Key("gram.team_invite.id") | |
| TeamInviteStatusKey = attribute.Key("gram.team_invite.status") |
| return s.Authenticate(ctx, "") | ||
| } | ||
|
|
||
| func (s *Manager) WorkOS() *workos.WorkOS { |
There was a problem hiding this comment.
this should not be here. likewise for the billing method below. session manager is not to be treated as a dependency provider.
| } | ||
|
|
||
| // getAuthContext extracts and validates the auth context from the request. | ||
| func (s *Service) getAuthContext(ctx context.Context) (*contextvalues.AuthContext, error) { |
There was a problem hiding this comment.
This is not worth DRYing up. inline it for now and i'll come up with a better check as part of current work.
Summary
GetUserByWorkosIDSQL query for reverse-mapping WorkOS users to Gram usersGetOrganizationMetadataByWorkosIDSQL query for org name resolutionTest plan
🤖 Generated with Claude Code