Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sessions from inactive providers can still be used #1458

Merged
merged 3 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions packages/cron-jobs-core/src/creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@ const createCallback = <TAllowedNames extends string, TName extends TAllowedName
* We are not using the runOnInit method as we want to run the job only once we start the cron job schedule manually.
* This allows us to always run it once we start it. Additionally it will not run the callback if only the cron job file is imported.
*/
const 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}`,
);
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,
Expand Down Expand Up @@ -90,7 +93,7 @@ export const createCronJobCreator = <TAllowedNames extends string = string>(
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`);
Expand All @@ -102,6 +105,8 @@ export const createCronJobCreator = <TAllowedNames extends string = string>(
// 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<TExpression> extends true
? typeof returnValue
: "Invalid cron expression";
: TExpression extends "never"
? typeof returnValue
: "Invalid cron expression";
};
};
1 change: 1 addition & 0 deletions packages/cron-jobs-core/src/expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
10 changes: 5 additions & 5 deletions packages/cron-jobs-core/src/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,33 +34,33 @@ export const createJobGroupCreator = <TAllowedNames extends string = string>(

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) => {
const job = jobRegistry.get(name as string);
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() {
Expand Down
2 changes: 2 additions & 0 deletions packages/cron-jobs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/cron-jobs/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -26,6 +27,7 @@ export const jobGroup = createCronJobGroup({
rssFeeds: rssFeedsJob,
indexerManager: indexerManagerJob,
healthMonitoring: healthMonitoringJob,
sessionCleanup: sessionCleanupJob,
});

export type JobGroupKeys = ReturnType<(typeof jobGroup)["getKeys"]>[number];
Expand Down
34 changes: 34 additions & 0 deletions packages/cron-jobs/src/jobs/session-cleanup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
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));

logger.info(`Deleted sessions for inactive providers count=${userIds.length}`);
Meierschlumpf marked this conversation as resolved.
Show resolved Hide resolved
});
3 changes: 3 additions & 0 deletions packages/translation/src/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,9 @@
},
"dnsHole": {
"label": "DNS Hole Data"
},
"sessionCleanup": {
"label": "Session Cleanup"
}
}
},
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.