feat: implement team endpoints with WorkOS invitation API#2003
feat: implement team endpoints with WorkOS invitation API#2003
Conversation
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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: ee7c372 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.
Devin Review found 4 potential issues.
🐛 1 issue in files not directly in the diff
🐛 invite_token is silently dropped during OAuth login flow and never reaches the callback (server/internal/auth/impl.go:497-508)
The PR adds an invite_token parameter to the login endpoint (server/design/auth/design.go:58) and it's parsed into LoginPayload.InviteToken (server/gen/auth/service.go:103). However, encodeStateParam() at server/internal/auth/impl.go:497-508 only encodes payload.Redirect into the OAuth state — the InviteToken field is never included in the loginState struct. Since the state parameter is the only way to persist data through the OAuth redirect round-trip, the invite token is permanently lost after the user authenticates. The callback handler at server/internal/auth/impl.go:120-196 has no way to retrieve or process the invite token.
View 1 additional finding in Devin Review.
| Method("getInviteInfo", func() { | ||
| Description("Get information about a team invite by its token. Used to display invite details before accepting.") | ||
|
|
||
| NoSecurity() | ||
|
|
||
| Payload(func() { | ||
| Required("token") | ||
| Attribute("token", String, "The invite token from the email link") | ||
| }) | ||
| Result(InviteInfoResult) | ||
|
|
||
| HTTP(func() { | ||
| GET("/rpc/teams.getInviteInfo") | ||
| Param("token") | ||
| }) | ||
|
|
||
| Meta("openapi:operationId", "getTeamInviteInfo") | ||
| Meta("openapi:extension:x-speakeasy-name-override", "getInviteInfo") | ||
| Meta("openapi:extension:x-speakeasy-react-hook", `{"name": "GetTeamInviteInfo"}`) | ||
| }) |
There was a problem hiding this comment.
🚩 GetInviteInfo endpoint is unauthenticated by design
The getInviteInfo method (server/internal/teams/impl.go:362) uses NoSecurity() in its Goa design (server/design/teams/design.go:121), meaning anyone with an invite token can retrieve the inviter name, organization name, invitee email, and invite status without authentication. This is a common pattern for invite-accept flows (the token acts as the secret), but it does expose organization metadata to anyone possessing the token. The token is also passed as a query parameter (Param("token")) which means it could appear in server access logs, browser history, and Referrer headers. This is worth reviewing if the invite tokens are long-lived or if the exposed metadata (org name, inviter display name) is considered sensitive.
Was this helpful? React with 👍 or 👎 to provide feedback.
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>
There was a problem hiding this comment.
🚩 New servechatattachmentsignedform.ts uses zod/v3 while all other new SDK files use zod/v4-mini
The new file client/sdk/src/models/components/servechatattachmentsignedform.ts imports from zod/v3 and uses z.ZodType<..., z.ZodTypeDef, ...> API patterns, while all other new SDK files in this PR use zod/v4-mini with z.ZodMiniType<..., ...>. This file appears unrelated to the teams feature (it's about chat attachments) and was likely pulled in as part of the SDK regeneration. Since this is code-generated, it may be intentional (different schema version), but the inconsistency is worth verifying doesn't cause runtime issues if zod/v3 isn't available.
Was this helpful? React with 👍 or 👎 to provide feedback.
- 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>
| inviterName := "" | ||
| if authCtx.Email != nil { | ||
| inviterName = *authCtx.Email | ||
| } |
There was a problem hiding this comment.
🟡 InviteMember returns inviter's email in the InvitedBy field instead of display name
InviteMember sets inviterName to authCtx.Email (line 262-263), which is the user's email address, not their display name. However, the invited_by field in the Goa design (server/design/teams/design.go:186) is documented as "Name of the user who sent the invite", and other methods that return the same TeamInvite type (ListInvites, ResendInvite) use resolveInviterName() which properly resolves the display name from WorkOS (e.g., "Jane Doe"). This means the same invite will show different values in InvitedBy depending on whether it's the immediate response from InviteMember (email: "jane@example.com") versus a subsequent ListInvites call (name: "Jane Doe").
| inviterName := "" | |
| if authCtx.Email != nil { | |
| inviterName = *authCtx.Email | |
| } | |
| inviterName := s.resolveInviterName(ctx, wos, inviterWorkOSID) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| func (s *Service) RemoveMember(ctx context.Context, payload *gen.RemoveMemberPayload) error { | ||
| authCtx, err := s.getAuthContext(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := s.validateOrgAccess(payload.OrganizationID, authCtx.ActiveOrganizationID); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if payload.UserID == authCtx.UserID { | ||
| return oops.E(oops.CodeBadRequest, nil, "cannot remove yourself from the organization") | ||
| } | ||
|
|
||
| wos, err := s.requireWorkOS() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| workosOrgID, err := s.getOrgWorkOSID(ctx, authCtx.ActiveOrganizationID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| memberWorkOSID, err := s.getUserWorkOSID(ctx, payload.UserID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| membership, err := wos.GetOrgMembership(ctx, memberWorkOSID, workosOrgID) | ||
| if err != nil { | ||
| return oops.E(oops.CodeGatewayError, err, "failed to find membership in WorkOS").Log(ctx, s.logger) | ||
| } | ||
| if membership == nil { | ||
| return oops.E(oops.CodeNotFound, nil, "user is not a member of this organization") | ||
| } | ||
|
|
||
| if err := wos.DeleteOrganizationMembership(ctx, membership.ID); err != nil { | ||
| return oops.E(oops.CodeGatewayError, err, "failed to remove member via WorkOS").Log(ctx, s.logger) | ||
| } | ||
|
|
||
| // 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), | ||
| ) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
🚩 No role-based authorization on sensitive team operations
The RemoveMember, InviteMember, CancelInvite, and ResendInvite endpoints only verify that the caller is an authenticated member of the organization (via getAuthContext + validateOrgAccess), but do not check for any admin/owner role. This means any org member can invite new members, remove other members, and manage invites. Whether this is intentional depends on the product's authorization model — if all org members should have equal management rights, this is fine. But if the intent is to restrict these operations to admins/owners, this is a significant authorization gap. The ListMembers and ListInvites endpoints being open to all members is reasonable.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for _, u := range users { | ||
| // Resolve WorkOS user to Gram user for consistent ID space | ||
| gramUser, err := s.userRepo.GetUserByWorkosID(ctx, pgtype.Text{String: u.ID, Valid: true}) | ||
| if err != nil { | ||
| // User exists in WorkOS but not synced to Gram yet — use WorkOS data | ||
| s.logger.WarnContext(ctx, "WorkOS user not found in Gram DB, skipping", | ||
| attr.SlogError(err), | ||
| ) | ||
| continue | ||
| } |
There was a problem hiding this comment.
🚩 ListMembers silently skips WorkOS users not synced to Gram DB
In ListMembers (line 199-205), when a WorkOS user cannot be resolved to a Gram user via GetUserByWorkosID, the user is silently skipped with a warning log. This means the member count shown to users could be lower than the actual org membership in WorkOS. This is a reasonable approach during the transition period where not all WorkOS users may be synced, but it could be confusing to org admins who see fewer members than expected. Consider whether showing these users with WorkOS-sourced data (email, name) instead of skipping them would be more appropriate.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
GetUserByWorkosIDSQL query for reverse-mapping WorkOS users to Gram usersGetOrganizationMetadataByWorkosIDSQL query for org name resolutionTest plan
🤖 Generated with Claude Code