From 0387418d89663ea764bb058d6edbc6ded4849d7a Mon Sep 17 00:00:00 2001 From: Piyush Gupta Date: Fri, 28 Nov 2025 20:16:28 +0530 Subject: [PATCH] fix: greptile review comments --- ...-scope-org-id-to-identity-access-tokens.ts | 20 ++++++++----- .../v1/identity-universal-auth-router.ts | 12 ++++---- .../src/services/auth/auth-login-service.ts | 29 ++++++++++++++++++- .../identity-alicloud-auth-service.ts | 2 +- .../identity-aws-auth-service.ts | 2 +- .../identity-azure-auth-service.ts | 2 +- .../identity-gcp-auth-service.ts | 2 +- .../identity-jwt-auth-service.ts | 2 +- .../identity-kubernetes-auth-service.ts | 2 +- .../identity-ldap-auth-service.ts | 2 +- .../identity-oci-auth-service.ts | 2 +- .../identity-oidc-auth-service.ts | 2 +- .../identity-tls-cert-auth-service.ts | 2 +- .../identity-token-auth-service.ts | 2 +- .../identity-ua/identity-ua-service.ts | 4 +-- 15 files changed, 60 insertions(+), 27 deletions(-) diff --git a/backend/src/db/migrations/20251127192155_adds-scope-org-id-to-identity-access-tokens.ts b/backend/src/db/migrations/20251127192155_adds-scope-org-id-to-identity-access-tokens.ts index cc55c1fb6..dd7f0c3a0 100644 --- a/backend/src/db/migrations/20251127192155_adds-scope-org-id-to-identity-access-tokens.ts +++ b/backend/src/db/migrations/20251127192155_adds-scope-org-id-to-identity-access-tokens.ts @@ -3,14 +3,20 @@ import { Knex } from "knex"; import { TableName } from "../schemas"; export async function up(knex: Knex): Promise { - await knex.schema.alterTable(TableName.IdentityAccessToken, (t) => { - t.uuid("scopeOrgId").notNullable(); - t.foreign("scopeOrgId").references("id").inTable(TableName.Organization).onDelete("CASCADE"); - }); + const hasScopeOrgIdColumn = await knex.schema.hasColumn(TableName.IdentityAccessToken, "scopeOrgId"); + if (!hasScopeOrgIdColumn) { + await knex.schema.alterTable(TableName.IdentityAccessToken, (t) => { + t.uuid("scopeOrgId"); + t.foreign("scopeOrgId").references("id").inTable(TableName.Organization).onDelete("CASCADE"); + }); + } } export async function down(knex: Knex): Promise { - await knex.schema.alterTable(TableName.IdentityAccessToken, (t) => { - t.dropColumn("scopeOrgId"); - }); + const hasScopeOrgIdColumn = await knex.schema.hasColumn(TableName.IdentityAccessToken, "scopeOrgId"); + if (hasScopeOrgIdColumn) { + await knex.schema.alterTable(TableName.IdentityAccessToken, (t) => { + t.dropColumn("scopeOrgId"); + }); + } } diff --git a/backend/src/server/routes/v1/identity-universal-auth-router.ts b/backend/src/server/routes/v1/identity-universal-auth-router.ts index 63ff18449..2e779a6c0 100644 --- a/backend/src/server/routes/v1/identity-universal-auth-router.ts +++ b/backend/src/server/routes/v1/identity-universal-auth-router.ts @@ -56,12 +56,12 @@ export const registerIdentityUaRouter = async (server: FastifyZodProvider) => { identity, accessTokenTTL, accessTokenMaxTTL - } = await server.services.identityUa.login( - req.body.clientId, - req.body.clientSecret, - req.realIp, - req.body.subOrganizationName - ); + } = await server.services.identityUa.login({ + clientId: req.body.clientId, + clientSecret: req.body.clientSecret, + ip: req.realIp, + subOrganizationName: req.body.subOrganizationName + }); await server.services.auditLog.createAuditLog({ ...req.auditLogInfo, diff --git a/backend/src/services/auth/auth-login-service.ts b/backend/src/services/auth/auth-login-service.ts index 4c063e547..374aef30d 100644 --- a/backend/src/services/auth/auth-login-service.ts +++ b/backend/src/services/auth/auth-login-service.ts @@ -748,6 +748,33 @@ export const authLoginServiceFactory = ({ }); } + if (!subOrg.rootOrgId) { + throw new BadRequestError({ + message: "Invalid sub-organization" + }); + } + + const rootOrg = await orgDAL.findById(subOrg.rootOrgId); + + if (!rootOrg) { + throw new BadRequestError({ + message: "Invalid root organization" + }); + } + + const rootOrgMembership = await membershipUserDAL.findOne({ + actorUserId: user.id, + scopeOrgId: rootOrg.id, + scope: AccessScope.Organization, + status: OrgMembershipStatus.Accepted + }); + + if (!rootOrgMembership) { + throw new ForbiddenRequestError({ + message: "User does not have access to the root organization" + }); + } + const subOrgmembershipRole = await membershipRoleDAL.findOne({ membershipId: userSubOrgMembership.id }); // Check if authEnforced is true and the current auth method is not an enforced method @@ -816,7 +843,7 @@ export const authLoginServiceFactory = ({ user, userAgent, ip: ipAddress, - ...(subOrg.rootOrgId && { organizationId: subOrg.rootOrgId }), + organizationId: rootOrg.id, subOrganizationId, isMfaVerified: decodedToken.isMfaVerified, mfaMethod: decodedToken.mfaMethod diff --git a/backend/src/services/identity-alicloud-auth/identity-alicloud-auth-service.ts b/backend/src/services/identity-alicloud-auth/identity-alicloud-auth-service.ts index 8a8de83cb..866f68aa1 100644 --- a/backend/src/services/identity-alicloud-auth/identity-alicloud-auth-service.ts +++ b/backend/src/services/identity-alicloud-auth/identity-alicloud-auth-service.ts @@ -90,7 +90,7 @@ export const identityAliCloudAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts b/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts index a21f86112..f17d61616 100644 --- a/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts +++ b/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts @@ -129,7 +129,7 @@ export const identityAwsAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts b/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts index 41eea9ae3..345cb20d1 100644 --- a/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts +++ b/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts @@ -86,7 +86,7 @@ export const identityAzureAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts b/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts index 5cf962f13..eac756730 100644 --- a/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts +++ b/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts @@ -82,7 +82,7 @@ export const identityGcpAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts b/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts index 2782fad0b..8e413f331 100644 --- a/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts +++ b/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts @@ -96,7 +96,7 @@ export const identityJwtAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts b/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts index 78de755ab..2ed1d5140 100644 --- a/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts +++ b/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts @@ -208,7 +208,7 @@ export const identityKubernetesAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts b/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts index 69c126827..befc308e5 100644 --- a/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts +++ b/backend/src/services/identity-ldap-auth/identity-ldap-auth-service.ts @@ -177,7 +177,7 @@ export const identityLdapAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-oci-auth/identity-oci-auth-service.ts b/backend/src/services/identity-oci-auth/identity-oci-auth-service.ts index 44c9f6212..131f64263 100644 --- a/backend/src/services/identity-oci-auth/identity-oci-auth-service.ts +++ b/backend/src/services/identity-oci-auth/identity-oci-auth-service.ts @@ -86,7 +86,7 @@ export const identityOciAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts b/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts index 385f3844f..e41f9b311 100644 --- a/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts +++ b/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts @@ -97,7 +97,7 @@ export const identityOidcAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-tls-cert-auth/identity-tls-cert-auth-service.ts b/backend/src/services/identity-tls-cert-auth/identity-tls-cert-auth-service.ts index 678a6a317..a8604917f 100644 --- a/backend/src/services/identity-tls-cert-auth/identity-tls-cert-auth-service.ts +++ b/backend/src/services/identity-tls-cert-auth/identity-tls-cert-auth-service.ts @@ -95,7 +95,7 @@ export const identityTlsCertAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-token-auth/identity-token-auth-service.ts b/backend/src/services/identity-token-auth/identity-token-auth-service.ts index 160f8076c..a16c52021 100644 --- a/backend/src/services/identity-token-auth/identity-token-auth-service.ts +++ b/backend/src/services/identity-token-auth/identity-token-auth-service.ts @@ -515,7 +515,7 @@ export const identityTokenAuthServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization, diff --git a/backend/src/services/identity-ua/identity-ua-service.ts b/backend/src/services/identity-ua/identity-ua-service.ts index 1fbc8b197..29b515d06 100644 --- a/backend/src/services/identity-ua/identity-ua-service.ts +++ b/backend/src/services/identity-ua/identity-ua-service.ts @@ -93,7 +93,7 @@ export const identityUaServiceFactory = ({ const org = await orgDAL.findById(identity.orgId); const isSubOrg = !!(org.rootOrgId || org.parentOrgId); - const rootOrgId = isSubOrg ? org.rootOrgId || org.id : org.id; + const rootOrgId = isSubOrg ? org.rootOrgId || "" : org.id; // Resolve sub-organization if specified let scopeOrgId = rootOrgId; @@ -101,7 +101,7 @@ export const identityUaServiceFactory = ({ const subOrg = await orgDAL.findOne({ slug: subOrganizationName }); if (subOrg) { - if (!isSubOrg || (isSubOrg && subOrg.rootOrgId === rootOrgId)) { + if (subOrg.rootOrgId === rootOrgId) { // Verify identity has membership in the sub-organization const subOrgMembership = await membershipIdentityDAL.findOne({ scope: AccessScope.Organization,