From 04d961b83222560a7745ab6676a46571f72c1d88 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 16 May 2024 15:39:32 +0530 Subject: [PATCH 1/6] feat: added dal to remove expired token for queue and fixed token validation check missing num uses increment and maxTTL failed check --- .../identity-access-token-dal.ts | 45 +++++++++++- .../identity-access-token-service.ts | 70 ++++++++++++------- 2 files changed, 88 insertions(+), 27 deletions(-) diff --git a/backend/src/services/identity-access-token/identity-access-token-dal.ts b/backend/src/services/identity-access-token/identity-access-token-dal.ts index 42fb5bba5..448f2dd46 100644 --- a/backend/src/services/identity-access-token/identity-access-token-dal.ts +++ b/backend/src/services/identity-access-token/identity-access-token-dal.ts @@ -37,5 +37,48 @@ export const identityAccessTokenDALFactory = (db: TDbClient) => { } }; - return { ...identityAccessTokenOrm, findOne }; + const removeExpiredTokens = async (tx?: Knex) => { + try { + const docs = (tx || db)(TableName.IdentityAccessToken) + .where({ + isAccessTokenRevoked: true + }) + .orWhere((qb) => { + void qb + .where("accessTokenNumUsesLimit", ">", 0) + .andWhere( + "accessTokenNumUses", + ">", + db.ref("accessTokenNumUsesLimit").withSchema(TableName.IdentityAccessToken) + ); + }) + .orWhere((qb) => { + void qb.where("accessTokenTTL", ">", 0).andWhere((qb2) => { + void qb2 + .where((qb3) => { + void qb3 + .whereNotNull("accessTokenLastRenewedAt") + // accessTokenLastRenewedAt + convert_integer_to_seconds(accessTokenTTL) < present_date + .andWhereRaw( + `"${TableName.IdentityAccessToken}"."accessTokenLastRenewedAt" + make_interval(secs => "${TableName.IdentityAccessToken}"."accessTokenTTL") < NOW()` + ); + }) + .orWhere((qb3) => { + void qb3 + .whereNull("accessTokenLastRenewedAt") + // created + convert_integer_to_seconds(accessTokenTTL) < present_date + .andWhereRaw( + `"${TableName.IdentityAccessToken}"."createdAt" + make_interval(secs => "${TableName.IdentityAccessToken}"."accessTokenTTL") < NOW()` + ); + }); + }); + }) + .delete(); + return await docs; + } catch (error) { + throw new DatabaseError({ error, name: "IdentityAccesTokenPrune" }); + } + }; + + return { ...identityAccessTokenOrm, findOne, removeExpiredTokens }; }; diff --git a/backend/src/services/identity-access-token/identity-access-token-service.ts b/backend/src/services/identity-access-token/identity-access-token-service.ts index 4b53c8174..79d7d4708 100644 --- a/backend/src/services/identity-access-token/identity-access-token-service.ts +++ b/backend/src/services/identity-access-token/identity-access-token-service.ts @@ -21,17 +21,18 @@ export const identityAccessTokenServiceFactory = ({ identityAccessTokenDAL, identityOrgMembershipDAL }: TIdentityAccessTokenServiceFactoryDep) => { - const validateAccessTokenExp = (identityAccessToken: TIdentityAccessTokens) => { + const validateAccessTokenExp = async (identityAccessToken: TIdentityAccessTokens) => { const { + id: tokenId, accessTokenTTL, accessTokenNumUses, accessTokenNumUsesLimit, accessTokenLastRenewedAt, - accessTokenMaxTTL, createdAt: accessTokenCreatedAt } = identityAccessToken; if (accessTokenNumUsesLimit > 0 && accessTokenNumUses > 0 && accessTokenNumUses >= accessTokenNumUsesLimit) { + await identityAccessTokenDAL.deleteById(tokenId); throw new BadRequestError({ message: "Unable to renew because access token number of uses limit reached" }); @@ -46,41 +47,26 @@ export const identityAccessTokenServiceFactory = ({ const ttlInMilliseconds = Number(accessTokenTTL) * 1000; const expirationDate = new Date(accessTokenRenewed.getTime() + ttlInMilliseconds); - if (currentDate > expirationDate) + if (currentDate > expirationDate) { + await identityAccessTokenDAL.deleteById(tokenId); throw new UnauthorizedError({ message: "Failed to renew MI access token due to TTL expiration" }); + } } else { // access token has never been renewed const accessTokenCreated = new Date(accessTokenCreatedAt); const ttlInMilliseconds = Number(accessTokenTTL) * 1000; const expirationDate = new Date(accessTokenCreated.getTime() + ttlInMilliseconds); - if (currentDate > expirationDate) + if (currentDate > expirationDate) { + await identityAccessTokenDAL.deleteById(tokenId); throw new UnauthorizedError({ message: "Failed to renew MI access token due to TTL expiration" }); + } } } - - // max ttl checks - if (Number(accessTokenMaxTTL) > 0) { - const accessTokenCreated = new Date(accessTokenCreatedAt); - const ttlInMilliseconds = Number(accessTokenMaxTTL) * 1000; - const currentDate = new Date(); - const expirationDate = new Date(accessTokenCreated.getTime() + ttlInMilliseconds); - - if (currentDate > expirationDate) - throw new UnauthorizedError({ - message: "Failed to renew MI access token due to Max TTL expiration" - }); - - const extendToDate = new Date(currentDate.getTime() + Number(accessTokenTTL)); - if (extendToDate > expirationDate) - throw new UnauthorizedError({ - message: "Failed to renew MI access token past its Max TTL expiration" - }); - } }; const renewAccessToken = async ({ accessToken }: TRenewAccessTokenDTO) => { @@ -97,7 +83,32 @@ export const identityAccessTokenServiceFactory = ({ }); if (!identityAccessToken) throw new UnauthorizedError(); - validateAccessTokenExp(identityAccessToken); + await validateAccessTokenExp(identityAccessToken); + + const { accessTokenMaxTTL, createdAt: accessTokenCreatedAt, accessTokenTTL } = identityAccessToken; + + // max ttl checks - will it go above max ttl + if (Number(accessTokenMaxTTL) > 0) { + const accessTokenCreated = new Date(accessTokenCreatedAt); + const ttlInMilliseconds = Number(accessTokenMaxTTL) * 1000; + const currentDate = new Date(); + const expirationDate = new Date(accessTokenCreated.getTime() + ttlInMilliseconds); + + if (currentDate > expirationDate) { + await identityAccessTokenDAL.deleteById(identityAccessToken.id); + throw new UnauthorizedError({ + message: "Failed to renew MI access token due to Max TTL expiration" + }); + } + + const extendToDate = new Date(currentDate.getTime() + Number(accessTokenTTL * 1000)); + if (extendToDate > expirationDate) { + await identityAccessTokenDAL.deleteById(identityAccessToken.id); + throw new UnauthorizedError({ + message: "Failed to renew MI access token past its Max TTL expiration" + }); + } + } const updatedIdentityAccessToken = await identityAccessTokenDAL.updateById(identityAccessToken.id, { accessTokenLastRenewedAt: new Date() @@ -113,7 +124,7 @@ export const identityAccessTokenServiceFactory = ({ }); if (!identityAccessToken) throw new UnauthorizedError(); - if (ipAddress) { + if (ipAddress && identityAccessToken) { checkIPAgainstBlocklist({ ipAddress, trustedIps: identityAccessToken?.accessTokenTrustedIps as TIp[] @@ -128,7 +139,14 @@ export const identityAccessTokenServiceFactory = ({ throw new UnauthorizedError({ message: "Identity does not belong to any organization" }); } - validateAccessTokenExp(identityAccessToken); + await validateAccessTokenExp(identityAccessToken); + + await identityAccessTokenDAL.updateById(identityAccessToken.id, { + accessTokenLastUsedAt: new Date(), + $incr: { + accessTokenNumUses: 1 + } + }); return { ...identityAccessToken, orgId: identityOrgMembership.orgId }; }; From 08e7815ec13db296b53b49ad865a78ea39497e66 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 16 May 2024 15:40:20 +0530 Subject: [PATCH 2/6] feat: added increment and decrement ops in update knex orm --- backend/src/lib/knex/index.ts | 56 +++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/backend/src/lib/knex/index.ts b/backend/src/lib/knex/index.ts index d78020809..bf057cc73 100644 --- a/backend/src/lib/knex/index.ts +++ b/backend/src/lib/knex/index.ts @@ -104,24 +104,68 @@ export const ormify = (db: Kne throw new DatabaseError({ error, name: "Create" }); } }, - updateById: async (id: string, data: Tables[Tname]["update"], tx?: Knex) => { + updateById: async ( + id: string, + { + $incr, + $decr, + ...data + }: Tables[Tname]["update"] & { + $incr?: { [x in keyof Partial]: number }; + $decr?: { [x in keyof Partial]: number }; + }, + tx?: Knex + ) => { try { - const [res] = await (tx || db)(tableName) + const query = (tx || db)(tableName) .where({ id } as never) .update(data as never) .returning("*"); - return res; + if ($incr) { + Object.entries($incr).forEach(([incrementField, incrementValue]) => { + void query.increment(incrementField, incrementValue); + }); + } + if ($decr) { + Object.entries($decr).forEach(([incrementField, incrementValue]) => { + void query.increment(incrementField, incrementValue); + }); + } + const [docs] = await query; + return docs; } catch (error) { throw new DatabaseError({ error, name: "Update by id" }); } }, - update: async (filter: TFindFilter, data: Tables[Tname]["update"], tx?: Knex) => { + update: async ( + filter: TFindFilter, + { + $incr, + $decr, + ...data + }: Tables[Tname]["update"] & { + $incr?: { [x in keyof Partial]: number }; + $decr?: { [x in keyof Partial]: number }; + }, + tx?: Knex + ) => { try { - const res = await (tx || db)(tableName) + const query = (tx || db)(tableName) .where(buildFindFilter(filter)) .update(data as never) .returning("*"); - return res; + // increment and decrement operation in update + if ($incr) { + Object.entries($incr).forEach(([incrementField, incrementValue]) => { + void query.increment(incrementField, incrementValue); + }); + } + if ($decr) { + Object.entries($decr).forEach(([incrementField, incrementValue]) => { + void query.increment(incrementField, incrementValue); + }); + } + return await query; } catch (error) { throw new DatabaseError({ error, name: "Update" }); } From 3ed5dd61093833a6dcdd314a90402f6ec292c918 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 16 May 2024 15:41:03 +0530 Subject: [PATCH 3/6] feat: removed audit log queue and switched to resource clean up queue --- .../ee/services/audit-log/audit-log-queue.ts | 31 +--------- backend/src/queue/queue-service.ts | 12 +++- backend/src/server/routes/index.ts | 8 ++- .../resource-cleanup-queue.ts | 58 +++++++++++++++++++ 4 files changed, 77 insertions(+), 32 deletions(-) create mode 100644 backend/src/services/resource-cleanup/resource-cleanup-queue.ts diff --git a/backend/src/ee/services/audit-log/audit-log-queue.ts b/backend/src/ee/services/audit-log/audit-log-queue.ts index 6c563b573..f93b391a5 100644 --- a/backend/src/ee/services/audit-log/audit-log-queue.ts +++ b/backend/src/ee/services/audit-log/audit-log-queue.ts @@ -3,7 +3,6 @@ import { RawAxiosRequestHeaders } from "axios"; import { SecretKeyEncoding } from "@app/db/schemas"; import { request } from "@app/lib/config/request"; import { infisicalSymmetricDecrypt } from "@app/lib/crypto/encryption"; -import { logger } from "@app/lib/logger"; import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue"; import { TProjectDALFactory } from "@app/services/project/project-dal"; @@ -113,35 +112,7 @@ export const auditLogQueueServiceFactory = ({ ); }); - queueService.start(QueueName.AuditLogPrune, async () => { - logger.info(`${QueueName.AuditLogPrune}: queue task started`); - await auditLogDAL.pruneAuditLog(); - logger.info(`${QueueName.AuditLogPrune}: queue task completed`); - }); - - // we do a repeat cron job in utc timezone at 12 Midnight each day - const startAuditLogPruneJob = async () => { - // clear previous job - await queueService.stopRepeatableJob( - QueueName.AuditLogPrune, - QueueJobs.AuditLogPrune, - { pattern: "0 0 * * *", utc: true }, - QueueName.AuditLogPrune // just a job id - ); - - await queueService.queue(QueueName.AuditLogPrune, QueueJobs.AuditLogPrune, undefined, { - delay: 5000, - jobId: QueueName.AuditLogPrune, - repeat: { pattern: "0 0 * * *", utc: true } - }); - }; - - queueService.listen(QueueName.AuditLogPrune, "failed", (err) => { - logger.error(err?.failedReason, `${QueueName.AuditLogPrune}: log pruning failed`); - }); - return { - pushToLog, - startAuditLogPruneJob + pushToLog }; }; diff --git a/backend/src/queue/queue-service.ts b/backend/src/queue/queue-service.ts index bc8ac88ff..9d85b6015 100644 --- a/backend/src/queue/queue-service.ts +++ b/backend/src/queue/queue-service.ts @@ -12,7 +12,9 @@ export enum QueueName { SecretRotation = "secret-rotation", SecretReminder = "secret-reminder", AuditLog = "audit-log", + // TODO(akhilmhdh): This will get removed later. For now this is kept to stop the repeatable queue AuditLogPrune = "audit-log-prune", + DailyResourceCleanUp = "daily-resource-cleanup", TelemetryInstanceStats = "telemtry-self-hosted-stats", IntegrationSync = "sync-integrations", SecretWebhook = "secret-webhook", @@ -26,7 +28,9 @@ export enum QueueJobs { SecretReminder = "secret-reminder-job", SecretRotation = "secret-rotation-job", AuditLog = "audit-log-job", + // TODO(akhilmhdh): This will get removed later. For now this is kept to stop the repeatable queue AuditLogPrune = "audit-log-prune-job", + DailyResourceCleanUp = "daily-resource-cleanup-job", SecWebhook = "secret-webhook-trigger", TelemetryInstanceStats = "telemetry-self-hosted-stats", IntegrationSync = "secret-integration-pull", @@ -55,6 +59,10 @@ export type TQueueJobTypes = { name: QueueJobs.AuditLog; payload: TCreateAuditLogDTO; }; + [QueueName.DailyResourceCleanUp]: { + name: QueueJobs.DailyResourceCleanUp; + payload: undefined; + }; [QueueName.AuditLogPrune]: { name: QueueJobs.AuditLogPrune; payload: undefined; @@ -172,7 +180,9 @@ export const queueServiceFactory = (redisUrl: string) => { jobId?: string ) => { const q = queueContainer[name]; - return q.removeRepeatable(job, repeatOpt, jobId); + if (q) { + return q.removeRepeatable(job, repeatOpt, jobId); + } }; const stopRepeatableJobByJobId = async (name: T, jobId: string) => { diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 3a050bcad..8ef6ee3be 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -113,6 +113,7 @@ import { projectMembershipServiceFactory } from "@app/services/project-membershi import { projectUserMembershipRoleDALFactory } from "@app/services/project-membership/project-user-membership-role-dal"; import { projectRoleDALFactory } from "@app/services/project-role/project-role-dal"; import { projectRoleServiceFactory } from "@app/services/project-role/project-role-service"; +import { dailyResourceCleanUpQueueServiceFactory } from "@app/services/resource-cleanup/resource-cleanup-queue"; import { secretDALFactory } from "@app/services/secret/secret-dal"; import { secretQueueFactory } from "@app/services/secret/secret-queue"; import { secretServiceFactory } from "@app/services/secret/secret-service"; @@ -757,14 +758,19 @@ export const registerRoutes = async ( folderDAL, licenseService }); + const dailyResourceCleanUp = dailyResourceCleanUpQueueServiceFactory({ + auditLogDAL, + queueService, + identityAccessTokenDAL + }); await superAdminService.initServerCfg(); // // setup the communication with license key server await licenseService.init(); - await auditLogQueue.startAuditLogPruneJob(); await telemetryQueue.startTelemetryCheck(); + await dailyResourceCleanUp.startCleanUp(); // inject all services server.decorate("services", { diff --git a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts new file mode 100644 index 000000000..3c8bcb1f7 --- /dev/null +++ b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts @@ -0,0 +1,58 @@ +import { TAuditLogDALFactory } from "@app/ee/services/audit-log/audit-log-dal"; +import { logger } from "@app/lib/logger"; +import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue"; + +import { TIdentityAccessTokenDALFactory } from "../identity-access-token/identity-access-token-dal"; + +type TDailyResourceCleanUpQueueServiceFactoryDep = { + auditLogDAL: Pick; + identityAccessTokenDAL: Pick; + queueService: TQueueServiceFactory; +}; + +export type TDailyResourceCleanUpQueueServiceFactory = ReturnType; + +export const dailyResourceCleanUpQueueServiceFactory = ({ + auditLogDAL, + queueService, + identityAccessTokenDAL +}: TDailyResourceCleanUpQueueServiceFactoryDep) => { + queueService.start(QueueName.DailyResourceCleanUp, async () => { + logger.info(`${QueueName.DailyResourceCleanUp}: queue task started`); + await auditLogDAL.pruneAuditLog(); + await identityAccessTokenDAL.removeExpiredTokens(); + logger.info(`${QueueName.DailyResourceCleanUp}: queue task completed`); + }); + + // we do a repeat cron job in utc timezone at 12 Midnight each day + const startCleanUp = async () => { + // TODO(akhilmhdh): remove later + await queueService.stopRepeatableJob( + QueueName.AuditLogPrune, + QueueJobs.AuditLogPrune, + { pattern: "0 0 * * *", utc: true }, + QueueName.AuditLogPrune // just a job id + ); + // clear previous job + await queueService.stopRepeatableJob( + QueueName.DailyResourceCleanUp, + QueueJobs.DailyResourceCleanUp, + { pattern: "0 0 * * *", utc: true }, + QueueName.DailyResourceCleanUp // just a job id + ); + + await queueService.queue(QueueName.DailyResourceCleanUp, QueueJobs.DailyResourceCleanUp, undefined, { + delay: 5000, + jobId: QueueName.DailyResourceCleanUp, + repeat: { pattern: "0 0 * * *", utc: true } + }); + }; + + queueService.listen(QueueName.DailyResourceCleanUp, "failed", (_, err) => { + logger.error(err, `${QueueName.DailyResourceCleanUp}: resource cleanup failed`); + }); + + return { + startCleanUp + }; +}; From 76c9d642a9b4d1c7ddcde1fc2afa3753914ffb85 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 16 May 2024 15:45:51 +0530 Subject: [PATCH 4/6] fix: resolved identity check failing due to comma seperated header in ip --- backend/src/server/plugins/ip.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/backend/src/server/plugins/ip.ts b/backend/src/server/plugins/ip.ts index b3c8171af..7b5838d57 100644 --- a/backend/src/server/plugins/ip.ts +++ b/backend/src/server/plugins/ip.ts @@ -6,6 +6,7 @@ const headersOrder = [ "cf-connecting-ip", // Cloudflare "Cf-Pseudo-IPv4", // Cloudflare "x-client-ip", // Most common + "x-envoy-external-address", // for envoy "x-forwarded-for", // Mostly used by proxies "fastly-client-ip", "true-client-ip", // Akamai and Cloudflare @@ -23,7 +24,21 @@ export const fastifyIp = fp(async (fastify) => { const forwardedIpHeader = headersOrder.find((header) => Boolean(req.headers[header])); const forwardedIp = forwardedIpHeader ? req.headers[forwardedIpHeader] : undefined; if (forwardedIp) { - req.realIp = Array.isArray(forwardedIp) ? forwardedIp[0] : forwardedIp; + if (Array.isArray(forwardedIp)) { + // eslint-disable-next-line + req.realIp = forwardedIp[0]; + return; + } + + if (forwardedIp.includes(",")) { + // the ip header when placed with load balancers that proxy request + // will attach the internal ips to header by appending with comma + // https://github.com/go-chi/chi/blob/master/middleware/realip.go + const clientIPFromProxy = forwardedIp.slice(0, forwardedIp.indexOf(",")).trim(); + req.realIp = clientIPFromProxy; + return; + } + req.realIp = forwardedIp; } else { req.realIp = req.ip; } From 1753cd76be02d0409b04e0f2fb571cd5dad4d364 Mon Sep 17 00:00:00 2001 From: Maidul Islam Date: Fri, 24 May 2024 12:43:14 -0400 Subject: [PATCH 5/6] update delete access token logic --- .../identity-access-token/identity-access-token-dal.ts | 4 ++-- .../src/services/resource-cleanup/resource-cleanup-queue.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/src/services/identity-access-token/identity-access-token-dal.ts b/backend/src/services/identity-access-token/identity-access-token-dal.ts index 448f2dd46..ee89fbdd8 100644 --- a/backend/src/services/identity-access-token/identity-access-token-dal.ts +++ b/backend/src/services/identity-access-token/identity-access-token-dal.ts @@ -48,7 +48,7 @@ export const identityAccessTokenDALFactory = (db: TDbClient) => { .where("accessTokenNumUsesLimit", ">", 0) .andWhere( "accessTokenNumUses", - ">", + ">=", db.ref("accessTokenNumUsesLimit").withSchema(TableName.IdentityAccessToken) ); }) @@ -76,7 +76,7 @@ export const identityAccessTokenDALFactory = (db: TDbClient) => { .delete(); return await docs; } catch (error) { - throw new DatabaseError({ error, name: "IdentityAccesTokenPrune" }); + throw new DatabaseError({ error, name: "IdentityAccessTokenPrune" }); } }; diff --git a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts index 3c8bcb1f7..a5e240839 100644 --- a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts +++ b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts @@ -44,7 +44,10 @@ export const dailyResourceCleanUpQueueServiceFactory = ({ await queueService.queue(QueueName.DailyResourceCleanUp, QueueJobs.DailyResourceCleanUp, undefined, { delay: 5000, jobId: QueueName.DailyResourceCleanUp, - repeat: { pattern: "0 0 * * *", utc: true } + repeat: { + every: 10000 + // limit: 100, + } }); }; From b6ff07b605ed8737159f4b5ee202ef6d43b41f5b Mon Sep 17 00:00:00 2001 From: Maidul Islam Date: Fri, 24 May 2024 12:45:19 -0400 Subject: [PATCH 6/6] revert repete cron --- .../src/services/resource-cleanup/resource-cleanup-queue.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts index a5e240839..3c8bcb1f7 100644 --- a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts +++ b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts @@ -44,10 +44,7 @@ export const dailyResourceCleanUpQueueServiceFactory = ({ await queueService.queue(QueueName.DailyResourceCleanUp, QueueJobs.DailyResourceCleanUp, undefined, { delay: 5000, jobId: QueueName.DailyResourceCleanUp, - repeat: { - every: 10000 - // limit: 100, - } + repeat: { pattern: "0 0 * * *", utc: true } }); };