refactor: Add validations by checking ecosystem details present in user's token.#1578
Merged
pranalidhanavade merged 8 commits intomainfrom Mar 6, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughValidate and propagate JWT Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as "API Gateway"
participant Jwt as "JwtStrategy"
participant Guard as "EcosystemRolesGuard"
participant Token as "JWT (ecosystem_access)"
Client->>API: HTTP request (may include ecosystemId)
API->>Jwt: validate token, attach payload
Jwt->>Token: extract payload.ecosystem_access -> request.user.ecosystem_access
API->>Guard: guard invoked for route
Guard->>API: extract/trim ecosystemId from params/query/body
Guard->>Token: verify payload.ecosystem_access contains ecosystemId in lead/member lists
alt access allowed
Guard->>API: set request.user.selectedEcosystem = ecosystemId
Guard-->>API: allow request
else access denied
Guard-->>API: throw BadRequest/Forbidden
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts (1)
10-15: Consider reusingEcosystemAccesstype from jwt-payload.interface.ts.This
EcosystemRoleGroupinterface mirrors the structure ofEcosystemAccessdefined injwt-payload.interface.ts. Consider importing and reusing that type to avoid duplication and ensure consistency.♻️ Suggested refactor
+import { EcosystemAccess } from './jwt-payload.interface'; -interface EcosystemRoleGroup { - ecosystem_role?: { - lead?: string[]; - member?: string[]; - }; -}Then update line 92:
-const hasAccess = Object.values(ecosystemAccess).some((entry: EcosystemRoleGroup) => { +const hasAccess = Object.values(ecosystemAccess).some((entry: EcosystemAccess) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts` around lines 10 - 15, The local EcosystemRoleGroup interface duplicates the existing EcosystemAccess type in jwt-payload.interface.ts; remove the local interface and import EcosystemAccess, then update all type annotations that used EcosystemRoleGroup (e.g., the parameter/variable types referenced in ecosystem-roles.guard.ts, including the usage at the location currently annotated with EcosystemRoleGroup on the role-check logic) to use EcosystemAccess instead to avoid duplication and keep types consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/authz/jwt.strategy.ts`:
- Around line 110-112: The code can null-dereference userDetails when
payload.ecosystem_access exists but userDetails is null (e.g., when
isServiceToken is true); update the logic around userDetails in the JWT
validation flow (the validate function / jwt.strategy.ts) to ensure userDetails
is initialized before assigning ecosystem_access (either create a minimal
userDetails object when isServiceToken is true and payload.ecosystem_access
exists, or guard the assignment with a check like if (userDetails)
userDetails.ecosystem_access = payload.ecosystem_access), referencing the
symbols userDetails, isServiceToken, and payload.ecosystem_access so the fix is
applied at the correct assignment site.
In `@apps/api-gateway/src/ecosystem/ecosystem.controller.ts`:
- Around line 210-214: Confirm whether excluding OrgRoles.PLATFORM_ADMIN from
the `@Roles` decorator on this endpoint was intentional; if it was not, add
OrgRoles.PLATFORM_ADMIN to the `@Roles`(...) list alongside OrgRoles.OWNER and
OrgRoles.ECOSYSTEM_LEAD so PLATFORM_ADMIN users can access it (matching other
ecosystem endpoints), otherwise leave it out and add a clear comment explaining
the deliberate exclusion and update/add a guard or unit test that enforces this
intended behavior; refer to the `@Roles` decorator and the OrgRoles enum to locate
and change the authorization config.
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 401-406: The first call to updateEcosystemInvitationStatusByEmail
in ecosystem.service.ts passes arguments in the wrong order; change the
invocation that currently passes (orgId, userEmail, ecosystemId,
Invitation.PENDING) so it instead passes (userEmail, orgId, ecosystemId,
Invitation.PENDING) to match the method signature (email, orgId, ecosystemId,
status) used in updateEcosystemInvitationStatusByEmail.
---
Nitpick comments:
In `@apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts`:
- Around line 10-15: The local EcosystemRoleGroup interface duplicates the
existing EcosystemAccess type in jwt-payload.interface.ts; remove the local
interface and import EcosystemAccess, then update all type annotations that used
EcosystemRoleGroup (e.g., the parameter/variable types referenced in
ecosystem-roles.guard.ts, including the usage at the location currently
annotated with EcosystemRoleGroup on the role-check logic) to use
EcosystemAccess instead to avoid duplication and keep types consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9215cef-88da-48a9-8f7a-f9e3a9c33c3e
📒 Files selected for processing (7)
apps/api-gateway/src/authz/guards/ecosystem-roles.guard.tsapps/api-gateway/src/authz/jwt-payload.interface.tsapps/api-gateway/src/authz/jwt.strategy.tsapps/api-gateway/src/ecosystem/ecosystem.controller.tsapps/api-gateway/src/ecosystem/intent/intent.controller.tsapps/ecosystem/repositories/ecosystem.repository.tsapps/ecosystem/src/ecosystem.service.ts
💤 Files with no reviewable changes (1)
- apps/ecosystem/src/ecosystem.service.ts
shitrerohit
approved these changes
Mar 4, 2026
pranalidhanavade
force-pushed
the
refactor/check-ecosystem-exist-from-jwt-token
branch
from
3957692 to
3299222
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ecosystem/src/ecosystem.service.ts (1)
481-486:⚠️ Potential issue | 🔴 CriticalIncorrect parameter order - will update wrong invitation record.
This call site still uses the old parameter order
(orgId, userEmail, ecosystemId, status), but the repository method signature was changed to(email, orgId, ecosystemId, status). This will cause the method to search for an invitation withemail = orgIdandinvitedOrg = userEmail, which will either fail to find any record or update the wrong one.🐛 Proposed fix
const result = await this.ecosystemRepository.updateEcosystemInvitationStatusByEmail( - orgId, userEmail, + orgId, ecosystemId, status );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ecosystem/src/ecosystem.service.ts` around lines 481 - 486, The call to updateEcosystemInvitationStatusByEmail in ecosystem.service.ts is passing arguments in the old order (orgId, userEmail, ecosystemId, status) but the repository signature is now (email, orgId, ecosystemId, status); change the call to pass userEmail first, then orgId, then ecosystemId and status so the call becomes updateEcosystemInvitationStatusByEmail(userEmail, orgId, ecosystemId, status) to ensure the correct invitation record is updated.
🧹 Nitpick comments (1)
apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts (1)
10-15: Consider importing the type from jwt-payload.interface.ts.The
EcosystemRoleGroupinterface duplicates theEcosystemAccesstype structure defined injwt-payload.interface.ts. While functional, importing the existing type would improve maintainability and ensure consistency if the structure changes.♻️ Suggested refactor
+import { EcosystemAccess } from '../jwt-payload.interface'; import { Injectable } from '@nestjs/common'; import { OrgRoles } from 'libs/org-roles/enums'; import { ROLES_KEY } from '../decorators/roles.decorator'; import { Reflector } from '@nestjs/core'; import { ResponseMessages } from '@credebl/common/response-messages'; import { validate as isValidUUID } from 'uuid'; -interface EcosystemRoleGroup { - ecosystem_role?: { - lead?: string[]; - member?: string[]; - }; -}Then update line 92:
- const hasAccess = Object.values(ecosystemAccess).some((entry: EcosystemRoleGroup) => { + const hasAccess = Object.values(ecosystemAccess).some((entry: EcosystemAccess) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts` around lines 10 - 15, Replace the duplicated local interface EcosystemRoleGroup with the existing type from jwt-payload.interface.ts by removing the local declaration and importing EcosystemAccess; then update any usages (e.g., the type annotation that currently references EcosystemRoleGroup—seen near the code around line 92) to use EcosystemAccess instead so the guard consistently depends on the single shared type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 401-406: The call site in ecosystem.service.ts is passing
arguments to updateEcosystemInvitationStatusByEmail in the wrong order (orgId,
userEmail, ecosystemId, status); change the call to pass (email, orgId,
ecosystemId, status) by swapping the first two arguments so it becomes
(userEmail, orgId, ecosystemId, status) to match the repository method signature
updateEcosystemInvitationStatusByEmail(email: string, orgId: string,
ecosystemId: string, status: Invitation).
---
Outside diff comments:
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 481-486: The call to updateEcosystemInvitationStatusByEmail in
ecosystem.service.ts is passing arguments in the old order (orgId, userEmail,
ecosystemId, status) but the repository signature is now (email, orgId,
ecosystemId, status); change the call to pass userEmail first, then orgId, then
ecosystemId and status so the call becomes
updateEcosystemInvitationStatusByEmail(userEmail, orgId, ecosystemId, status) to
ensure the correct invitation record is updated.
---
Nitpick comments:
In `@apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts`:
- Around line 10-15: Replace the duplicated local interface EcosystemRoleGroup
with the existing type from jwt-payload.interface.ts by removing the local
declaration and importing EcosystemAccess; then update any usages (e.g., the
type annotation that currently references EcosystemRoleGroup—seen near the code
around line 92) to use EcosystemAccess instead so the guard consistently depends
on the single shared type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9ba89b2-9758-4072-99ea-7c99e3569394
📒 Files selected for processing (7)
apps/api-gateway/src/authz/guards/ecosystem-roles.guard.tsapps/api-gateway/src/authz/jwt-payload.interface.tsapps/api-gateway/src/authz/jwt.strategy.tsapps/api-gateway/src/ecosystem/ecosystem.controller.tsapps/api-gateway/src/ecosystem/intent/intent.controller.tsapps/ecosystem/repositories/ecosystem.repository.tsapps/ecosystem/src/ecosystem.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api-gateway/src/ecosystem/ecosystem.controller.ts
- apps/api-gateway/src/authz/jwt-payload.interface.ts
…er's token Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
pranalidhanavade
force-pushed
the
refactor/check-ecosystem-exist-from-jwt-token
branch
from
6d949f1 to
f034e8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/authz/guards/ecosystem-roles.guard.ts`:
- Around line 76-105: The current guard returns after checking membership
(ecosystemIdExists -> hasAccess) which allows plain members to pass even when
the endpoint requires a higher org role; modify the logic in
ecosystem-roles.guard.ts to (1) determine the required role for the route (e.g.,
OrgRoles.ECOSYSTEM_LEAD) instead of treating membership as sufficient, (2)
inspect user?.ecosystem_access (EcosystemRoleGroup) for the specific role list
(entry.ecosystem_role.lead vs .member) for the given ecosystemId, (3) only set
user.selectedEcosystem and return true if the required role is present,
otherwise throw a ForbiddenException with the existing ResponseMessages error,
and (4) ensure this check occurs before the early return that currently follows
hasAccess so role requirements are enforced.
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 1193-1218: The code sets finalOrgIds directly when orgId is
provided, allowing cross-ecosystem access; change the branch so that when orgId
is present you first validate membership by querying this.prisma.ecosystem_orgs
(use findFirst/findUnique) with both orgId and ecosystemId and the same
deletedAt/null and status ACTIVE checks, and if that membership record is
missing return { totalPages: 0, data: [] }; only then set finalOrgIds = [orgId].
This ensures finalOrgIds, orgId and ecosystemId are always scoped to the
ecosystem before building the whereClause (also apply same validation where
similar logic appears around the subsequent orgId handling).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa27a995-1a49-4d44-acec-a30341fd30e2
📒 Files selected for processing (10)
apps/api-gateway/src/authz/guards/ecosystem-roles.guard.tsapps/api-gateway/src/authz/jwt-payload.interface.tsapps/api-gateway/src/authz/jwt.strategy.tsapps/api-gateway/src/ecosystem/ecosystem.controller.tsapps/api-gateway/src/ecosystem/ecosystem.service.tsapps/api-gateway/src/ecosystem/intent/intent.controller.tsapps/ecosystem/interfaces/ecosystem.interfaces.tsapps/ecosystem/repositories/ecosystem.repository.tsapps/ecosystem/src/ecosystem.controller.tsapps/ecosystem/src/ecosystem.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api-gateway/src/authz/jwt-payload.interface.ts
- apps/api-gateway/src/authz/jwt.strategy.ts
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
|
sujitaw
approved these changes
Mar 6, 2026
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
3 participants
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.
What
Summary by CodeRabbit
New Features
Refactor
API Changes
Bug Fixes / Validation