From 1105f0029fd1479e3ab6d8dae8524233c264ca21 Mon Sep 17 00:00:00 2001 From: Meier Lukas Date: Sun, 17 Nov 2024 21:30:46 +0100 Subject: [PATCH] fix: sessions from inactive providers can still be used (#1458) * fix: sessions from inactive providers can still be used * fix(lint): dependency keys not sorted * chore: address pull request feedback --- packages/cron-jobs-core/src/creator.ts | 25 +++++++----- packages/cron-jobs-core/src/expressions.ts | 1 + packages/cron-jobs-core/src/group.ts | 10 ++--- packages/cron-jobs/package.json | 2 + packages/cron-jobs/src/index.ts | 2 + .../cron-jobs/src/jobs/session-cleanup.ts | 38 +++++++++++++++++++ packages/translation/src/lang/en.json | 3 ++ pnpm-lock.yaml | 6 +++ 8 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 packages/cron-jobs/src/jobs/session-cleanup.ts diff --git a/packages/cron-jobs-core/src/creator.ts b/packages/cron-jobs-core/src/creator.ts index 62607377e..a25535db6 100644 --- a/packages/cron-jobs-core/src/creator.ts +++ b/packages/cron-jobs-core/src/creator.ts @@ -49,14 +49,17 @@ const createCallback = void catchingCallbackAsync(), { - scheduled: false, - name, - timezone: creatorOptions.timezone, - }); - creatorOptions.logger.logDebug( - `The cron job '${name}' was created with expression ${cronExpression} in timezone ${creatorOptions.timezone} and runOnStart ${options.runOnStart}`, - ); + let scheduledTask: cron.ScheduledTask | null = null; + if (cronExpression !== "never") { + scheduledTask = cron.schedule(cronExpression, () => void catchingCallbackAsync(), { + scheduled: false, + name, + timezone: creatorOptions.timezone, + }); + creatorOptions.logger.logDebug( + `The cron job '${name}' was created with expression ${cronExpression} in timezone ${creatorOptions.timezone} and runOnStart ${options.runOnStart}`, + ); + } return { name, @@ -90,7 +93,7 @@ export const createCronJobCreator = ( options: CreateCronJobOptions = { runOnStart: false }, ) => { creatorOptions.logger.logDebug(`Validating cron expression '${cronExpression}' for job: ${name}`); - if (!cron.validate(cronExpression)) { + if (cronExpression !== "never" && !cron.validate(cronExpression)) { throw new Error(`Invalid cron expression '${cronExpression}' for job '${name}'`); } creatorOptions.logger.logDebug(`Cron job expression '${cronExpression}' for job ${name} is valid`); @@ -102,6 +105,8 @@ export const createCronJobCreator = ( // This is a type guard to check if the cron expression is valid and give the user a type hint return returnValue as unknown as ValidateCron extends true ? typeof returnValue - : "Invalid cron expression"; + : TExpression extends "never" + ? typeof returnValue + : "Invalid cron expression"; }; }; diff --git a/packages/cron-jobs-core/src/expressions.ts b/packages/cron-jobs-core/src/expressions.ts index d3b187190..e8015b486 100644 --- a/packages/cron-jobs-core/src/expressions.ts +++ b/packages/cron-jobs-core/src/expressions.ts @@ -7,3 +7,4 @@ export const EVERY_10_MINUTES = checkCron("*/10 * * * *") satisfies string; export const EVERY_HOUR = checkCron("0 * * * *") satisfies string; export const EVERY_DAY = checkCron("0 0 * * */1") satisfies string; export const EVERY_WEEK = checkCron("0 0 * * 1") satisfies string; +export const NEVER = "never"; diff --git a/packages/cron-jobs-core/src/group.ts b/packages/cron-jobs-core/src/group.ts index 76dd1c765..af68ed566 100644 --- a/packages/cron-jobs-core/src/group.ts +++ b/packages/cron-jobs-core/src/group.ts @@ -34,13 +34,13 @@ export const createJobGroupCreator = ( options.logger.logInfo(`Starting schedule cron job ${job.name}.`); await job.onStartAsync(); - job.scheduledTask.start(); + job.scheduledTask?.start(); }, startAllAsync: async () => { for (const job of jobRegistry.values()) { options.logger.logInfo(`Starting schedule of cron job ${job.name}.`); await job.onStartAsync(); - job.scheduledTask.start(); + job.scheduledTask?.start(); } }, runManually: (name: keyof TJobs) => { @@ -48,19 +48,19 @@ export const createJobGroupCreator = ( if (!job) return; options.logger.logInfo(`Running schedule cron job ${job.name} manually.`); - job.scheduledTask.now(); + job.scheduledTask?.now(); }, stop: (name: keyof TJobs) => { const job = jobRegistry.get(name as string); if (!job) return; options.logger.logInfo(`Stopping schedule cron job ${job.name}.`); - job.scheduledTask.stop(); + job.scheduledTask?.stop(); }, stopAll: () => { for (const job of jobRegistry.values()) { options.logger.logInfo(`Stopping schedule cron job ${job.name}.`); - job.scheduledTask.stop(); + job.scheduledTask?.stop(); } }, getJobRegistry() { diff --git a/packages/cron-jobs/package.json b/packages/cron-jobs/package.json index eefd24d79..c08568c6f 100644 --- a/packages/cron-jobs/package.json +++ b/packages/cron-jobs/package.json @@ -24,10 +24,12 @@ "dependencies": { "@extractus/feed-extractor": "^7.1.3", "@homarr/analytics": "workspace:^0.1.0", + "@homarr/auth": "workspace:^0.1.0", "@homarr/common": "workspace:^0.1.0", "@homarr/cron-job-status": "workspace:^0.1.0", "@homarr/cron-jobs-core": "workspace:^0.1.0", "@homarr/db": "workspace:^0.1.0", + "@homarr/definitions": "workspace:^0.1.0", "@homarr/icons": "workspace:^0.1.0", "@homarr/integrations": "workspace:^0.1.0", "@homarr/log": "workspace:^0.1.0", diff --git a/packages/cron-jobs/src/index.ts b/packages/cron-jobs/src/index.ts index 7e2d357e0..8a358988f 100644 --- a/packages/cron-jobs/src/index.ts +++ b/packages/cron-jobs/src/index.ts @@ -11,6 +11,7 @@ import { mediaServerJob } from "./jobs/integrations/media-server"; import { pingJob } from "./jobs/ping"; import type { RssFeed } from "./jobs/rss-feeds"; import { rssFeedsJob } from "./jobs/rss-feeds"; +import { sessionCleanupJob } from "./jobs/session-cleanup"; import { createCronJobGroup } from "./lib"; export const jobGroup = createCronJobGroup({ @@ -26,6 +27,7 @@ export const jobGroup = createCronJobGroup({ rssFeeds: rssFeedsJob, indexerManager: indexerManagerJob, healthMonitoring: healthMonitoringJob, + sessionCleanup: sessionCleanupJob, }); export type JobGroupKeys = ReturnType<(typeof jobGroup)["getKeys"]>[number]; diff --git a/packages/cron-jobs/src/jobs/session-cleanup.ts b/packages/cron-jobs/src/jobs/session-cleanup.ts new file mode 100644 index 000000000..f9d382ad5 --- /dev/null +++ b/packages/cron-jobs/src/jobs/session-cleanup.ts @@ -0,0 +1,38 @@ +import { env } from "@homarr/auth/env.mjs"; +import { NEVER } from "@homarr/cron-jobs-core/expressions"; +import { db, eq, inArray } from "@homarr/db"; +import { sessions, users } from "@homarr/db/schema/sqlite"; +import { supportedAuthProviders } from "@homarr/definitions"; +import { logger } from "@homarr/log"; + +import { createCronJob } from "../lib"; + +/** + * Deletes sessions for users that have inactive auth providers. + * Sessions from other providers are deleted so they can no longer be used. + */ +export const sessionCleanupJob = createCronJob("sessionCleanup", NEVER, { + runOnStart: true, +}).withCallback(async () => { + const currentAuthProviders = env.AUTH_PROVIDERS; + + const inactiveAuthProviders = supportedAuthProviders.filter((provider) => !currentAuthProviders.includes(provider)); + const subQuery = db + .select({ id: users.id }) + .from(users) + .where(inArray(users.provider, inactiveAuthProviders)) + .as("sq"); + const sessionsWithInactiveProviders = await db + .select({ userId: sessions.userId }) + .from(sessions) + .rightJoin(subQuery, eq(sessions.userId, subQuery.id)); + + const userIds = sessionsWithInactiveProviders.map(({ userId }) => userId).filter((value) => value !== null); + await db.delete(sessions).where(inArray(sessions.userId, userIds)); + + if (sessionsWithInactiveProviders.length > 0) { + logger.info(`Deleted sessions for inactive providers count=${userIds.length}`); + } else { + logger.debug("No sessions to delete"); + } +}); diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index c10f45d8f..f693daaa7 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -2073,6 +2073,9 @@ }, "dnsHole": { "label": "DNS Hole Data" + }, + "sessionCleanup": { + "label": "Session Cleanup" } } }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c97864040..3a9d37213 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -749,6 +749,9 @@ importers: '@homarr/analytics': specifier: workspace:^0.1.0 version: link:../analytics + '@homarr/auth': + specifier: workspace:^0.1.0 + version: link:../auth '@homarr/common': specifier: workspace:^0.1.0 version: link:../common @@ -761,6 +764,9 @@ importers: '@homarr/db': specifier: workspace:^0.1.0 version: link:../db + '@homarr/definitions': + specifier: workspace:^0.1.0 + version: link:../definitions '@homarr/icons': specifier: workspace:^0.1.0 version: link:../icons