feat: api v2 team invite link endpoint#101
Open
tomerqodo wants to merge 2 commits into
Conversation
Code Review by Qodo
1. Invite creation lacks audit log
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+30
to
+40
| export class TeamsInviteController { | ||
| @Post("/invite") | ||
| @Roles("TEAM_MEMBER") | ||
| @ApiOperation({ summary: "Create team invite link" }) | ||
| @HttpCode(HttpStatus.OK) | ||
| async createInvite( | ||
| @Param("teamId", ParseIntPipe) teamId: number | ||
| ): Promise<CreateInviteOutputDto> { | ||
| const result = await TeamService.createInvite(teamId); | ||
| return { status: SUCCESS_STATUS, data: result }; | ||
| } |
There was a problem hiding this comment.
1. Invite creation lacks audit log 📘 Rule violation ⛨ Security
• The new /v2/teams/:teamId/invite endpoint performs a security-sensitive action (creating an invite token/link) without writing an audit log entry. • This makes it difficult to reconstruct who generated invite links (user ID), for which team, and whether the operation succeeded or failed. • Add an audit/audit-log event capturing user ID, teamId, action name, timestamp, and outcome.
Agent prompt
## Issue description
The new team invite-link creation endpoint performs a critical security action (issuing invite tokens/links) without creating an audit trail entry that records who performed the action, when, on which team, and whether it succeeded.
## Issue Context
Compliance requires audit logs for critical actions to enable incident investigation and security forensics. This endpoint should write an audit/audit-log record on success/failure and include `userId`, `teamId`, action name, timestamp, and outcome.
## Fix Focus Areas
- apps/api/v2/src/modules/teams/invite/controllers/teams-invite.controller.ts[30-40]
- packages/features/ee/teams/services/teamService.ts[87-101]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| import { ApiHeader, ApiOperation, ApiTags as DocsTags } from "@nestjs/swagger"; | ||
|
|
||
| import { SUCCESS_STATUS } from "@calcom/platform-constants"; | ||
| import { TeamService } from "@calcom/features/ee/teams/services/teamService"; |
There was a problem hiding this comment.
2. teamservice breaks naming rule 📘 Rule violation ⛯ Reliability
• The PR introduces a new import from @calcom/features/ee/teams/services/teamService, which points to a non-PascalCase service filename. • Compliance requires service filenames to match the exported class name exactly in PascalCase (e.g., TeamService.ts). • This reduces discoverability and violates the repository’s service naming convention.
Agent prompt
## Issue description
A service class (`TeamService`) is defined in `teamService.ts`, which violates the naming convention requiring service filenames to match exported class names exactly in PascalCase.
## Issue Context
Compliance requires `<Entity>Service.ts` filenames with matching exported class names for consistency and discoverability.
## Fix Focus Areas
- apps/api/v2/src/modules/teams/invite/controllers/teams-invite.controller.ts[20-22]
- packages/features/ee/teams/services/teamService.ts[55-60]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
87
to
91
| const token = randomBytes(32).toString("hex"); | ||
| await prisma.verificationToken.create({ | ||
| const newToken = await prisma.verificationToken.create({ | ||
| data: { | ||
| identifier: `invite-link-for-teamId-${teamId}`, | ||
| token, |
There was a problem hiding this comment.
3. Wrong token returned 🐞 Bug ✓ Correctness
• TeamService.createInvite() now returns a constant identifier string instead of the random invite token, breaking clients that must use the token to accept invites. • The returned token will not match the inviteLink (which is built with the real random token), so consumers/tests expecting inviteLink to contain the returned token will fail. • Because identifier is constant per team, repeated calls can return the same “token” value even though a new DB token row is created each time.
Agent prompt
### Issue description
`TeamService.createInvite()` returns `newToken.identifier` as the response token, but `inviteLink` is built using the random `token`. This breaks the API contract (returned token is not usable to accept invites) and makes `inviteLink` not contain the returned token.
### Issue Context
`identifier` is a constant pattern (`invite-link-for-teamId-${teamId}`) and is not the invite token. Multiple callers (including the new API v2 tests) assume the returned `token` is the actual verification token string.
### Fix Focus Areas
- packages/features/ee/teams/services/teamService.ts[87-101]
### Expected fix
- Change the return value to `token: newToken.token` (or just `token`) so it matches the token embedded into `inviteLink`.
- Optionally rename variables to reduce confusion (e.g., `createdToken` vs `token`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
104
to
109
| private static async buildInviteLink(token: string, isOrgContext: boolean): Promise<string> { | ||
| const teamInviteLink = `${WEBAPP_URL}/teams?token=${token}`; | ||
| if (!isOrgContext) { | ||
| if (isOrgContext) { | ||
| return teamInviteLink; | ||
| } | ||
| const gettingStartedPath = await OnboardingPathService.getGettingStartedPathWhenInvited(prisma); |
There was a problem hiding this comment.
4. Org invite url inverted 🐞 Bug ✓ Correctness
• buildInviteLink() condition was flipped so org-context invites now return /teams?token=..., which requires an authenticated session and redirects unauthenticated users to login. • This contradicts the documented invite behavior (signup URL with callbackUrl) and likely breaks org/team-in-org invites for new users. • The previous behavior (org-context → /signup?token=...&callbackUrl=...) aligns with the API docs and supports new-user signup.
Agent prompt
### Issue description
`buildInviteLink()` now returns `/teams?token=...` for `isOrgContext === true`. The `/teams` page requires login when a token is present, so org-context invite links won’t work for unauthenticated/new users and contradict documented behavior.
### Issue Context
The API docs for org/team invites describe a signup link with `callbackUrl`.
### Fix Focus Areas
- packages/features/ee/teams/services/teamService.ts[104-111]
### Expected fix
- Restore the previous condition (likely `if (!isOrgContext) return teamInviteLink;`), so org-context returns the `/signup?token=...&callbackUrl=...` link.
- Consider renaming `teamInviteLink`/`orgInviteLink` variables to better reflect which context they serve.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+31
to
+33
| @Post("/invite") | ||
| @Roles("TEAM_MEMBER") | ||
| @ApiOperation({ summary: "Create team invite link" }) |
There was a problem hiding this comment.
5. Non-admin can invite 🐞 Bug ⛨ Security
• The new /v2/teams/:teamId/invite endpoint uses @Roles("TEAM_MEMBER"), which (per RolesGuard
hierarchy) allows OWNER/ADMIN/MEMBER — meaning regular members can create invite links.
• This is a privilege escalation: any team member could generate an invite link and add arbitrary
users.
• The newly-added e2e test expects a MEMBER to get 403, so the current implementation will fail CI.
Agent prompt
### Issue description
The new Teams Invite endpoint uses `@Roles("TEAM_MEMBER")`, which allows any team member to create invite links. This is an authorization bypass/privilege escalation and also contradicts the new e2e test expectation (MEMBER should be 403).
### Issue Context
RolesGuard treats the decorator value as the minimum required role.
### Fix Focus Areas
- apps/api/v2/src/modules/teams/invite/controllers/teams-invite.controller.ts[31-33]
### Expected fix
- Change the decorator to `@Roles("TEAM_ADMIN")` (allowing ADMIN + OWNER) to match existing invite controller behavior.
- If needed, align with PBAC checks similar to TRPC (`fallbackRoles: [OWNER, ADMIN]`) for consistency.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Benchmark PR from agentic-review-benchmarks#6