feat(admin): add optional expiry to admin IP bans (#3163)

fluxer_api/src/api/admin/AdminRepository.ts

@@ -161,38 +161,43 @@ export class AdminRepository implements IAdminRepository {
 		return false;
 	}
 
-	async banIp(ip: string): Promise<void> {
-		if (isIpBanExempt(ip)) {
-			return;
-		}
-		const canonicalIp = canonicalizeBannedIpEntry(ip);
-		await upsertOne(
-			BannedIps.insert({
-				ip: canonicalIp,
-				ban_kind: 'permanent',
-				reason: 'platform_admin_enforcement',
-				expires_at: null,
-				created_at: new Date(),
-			}),
-		);
+	async banIp(ip: string, ttlSeconds: number | null = null): Promise<void> {
+		await this.writeIpBan(ip, 'platform_admin_enforcement', ttlSeconds);
 	}
 
 	async banIpTemp(ip: string, ttlSeconds: number): Promise<void> {
-		if (!Number.isInteger(ttlSeconds) || ttlSeconds <= 0) {
+		await this.writeIpBan(ip, 'abusive_api_access_patterns', ttlSeconds);
+	}
+
+	private async writeIpBan(ip: string, reason: string, ttlSeconds: number | null): Promise<void> {
+		if (ttlSeconds !== null && (!Number.isInteger(ttlSeconds) || ttlSeconds <= 0)) {
 			throw new RangeError('Temporary IP ban TTL must be a positive integer');
 		}
 		if (isIpBanExempt(ip)) {
 			return;
 		}
 		const canonicalIp = canonicalizeBannedIpEntry(ip);
+		const createdAt = new Date();
+		if (ttlSeconds === null) {
+			await upsertOne(
+				BannedIps.insert({
+					ip: canonicalIp,
+					ban_kind: 'permanent',
+					reason,
+					expires_at: null,
+					created_at: createdAt,
+				}),
+			);
+			return;
+		}
 		await upsertOne(
 			BannedIps.insertWithTtl(
 				{
 					ip: canonicalIp,
 					ban_kind: 'temporary_24h',
-					reason: 'abusive_api_access_patterns',
-					expires_at: new Date(Date.now() + ttlSeconds * 1000),
-					created_at: new Date(),
+					reason,
+					expires_at: new Date(createdAt.getTime() + ttlSeconds * 1000),
+					created_at: createdAt,
 				},
 				ttlSeconds,
 			),
@@ -228,13 +233,16 @@ export class AdminRepository implements IAdminRepository {
 			expires_at?: Date | null;
 			created_at?: Date | null;
 		}>(LOAD_ALL_BANNED_IPS_QUERY.bind({}));
-		return rows.map((row) => ({
-			ip: row.ip,
-			kind: parseBannedIpKind(row.ban_kind),
-			reason: row.reason ?? null,
-			expiresAt: row.expires_at ?? null,
-			createdAt: row.created_at ?? null,
-		}));
+		const now = Date.now();
+		return rows
+			.filter((row) => !row.expires_at || row.expires_at.getTime() > now)
+			.map((row) => ({
+				ip: row.ip,
+				kind: parseBannedIpKind(row.ban_kind),
+				reason: row.reason ?? null,
+				expiresAt: row.expires_at ?? null,
+				createdAt: row.created_at ?? null,
+			}));
 	}
 
 	async isEmailBanned(email: string): Promise<boolean> {

fluxer_api/src/api/admin/IAdminRepository.ts

@@ -43,7 +43,7 @@ export abstract class IAdminRepository {
 
 	abstract isIpBanned(ip: string): Promise<boolean>;
 
-	abstract banIp(ip: string): Promise<void>;
+	abstract banIp(ip: string, ttlSeconds?: number | null): Promise<void>;
 
 	abstract banIpTemp(ip: string, ttlSeconds: number): Promise<void>;
 

fluxer_api/src/api/admin/controllers/BanAdminController.ts

@@ -57,9 +57,9 @@ const BLOCKLIST_CATALOG = [
 	{
 		list_type: 'ip' as const,
 		description:
-			'IPv4/IPv6 addresses and CIDR ranges denied service. Applies to live connections and can be applied retroactively.',
+			'IPv4/IPv6 addresses and CIDR ranges denied service. Applies to live connections and can be applied retroactively. An entry can carry an expiry, after which it stops applying and is removed.',
 		value_field: 'ip',
-		fields: [],
+		fields: ['duration_hours'],
 		scoped: false,
 		supports_bulk_create: false,
 		supports_bulk_delete: false,
@@ -232,7 +232,7 @@ async function checkBlocklistEntry(
 	listType: AdminBlocklistListType,
 	entryValue: string,
 	scope: ProfileSubstringScope | undefined,
-): Promise<{banned: boolean}> {
+): Promise<{banned: boolean; expires_at?: string | null}> {
 	switch (listType) {
 		case 'ip':
 			return bans.checkIpBan({ip: entryValue});
@@ -507,7 +507,7 @@ export function BanAdminController(app: HonoApp) {
 					banned: result.banned,
 				},
 			});
-			return ctx.json(result);
+			return ctx.json({banned: result.banned, expires_at: result.expires_at ?? null});
 		},
 	);
 	app.patch(

fluxer_api/src/api/admin/services/AdminBanManagementService.ts

@@ -122,6 +122,7 @@ export class AdminBanManagementService {
 	async banIp(
 		data: {
 			ip: string;
+			duration_hours?: number;
 		},
 		adminUserId: UserID,
 		auditLogReason: string | null,
@@ -142,15 +143,24 @@ export class AdminBanManagementService {
 				message: 'This IP address is on the instance exemption list',
 			});
 		}
-		await adminRepository.banIp(data.ip);
-		ipBanCache.ban(data.ip);
+		const durationHours = data.duration_hours ?? 0;
+		const metadata = new Map([['ip', data.ip]]);
+		if (durationHours > 0) {
+			const ttlSeconds = durationHours * 3600;
+			await adminRepository.banIp(data.ip, ttlSeconds);
+			metadata.set('duration_hours', durationHours.toString());
+			metadata.set('expires_at', new Date(Date.now() + ttlSeconds * 1000).toISOString());
+		} else {
+			await adminRepository.banIp(data.ip);
+		}
+		await ipBanCache.refresh();
 		await cacheService.publish(IP_BAN_REFRESH_CHANNEL, 'refresh');
 		await this.createBlocklistAuditLog({
 			adminUserId,
 			targetType: 'ip',
 			action: 'ban_ip',
 			auditLogReason,
-			metadata: new Map([['ip', data.ip]]),
+			metadata,
 		});
 	}
 
@@ -177,9 +187,10 @@ export class AdminBanManagementService {
 
 	async checkIpBan(data: {ip: string}): Promise<{
 		banned: boolean;
+		expires_at: string | null;
 	}> {
-		const banned = ipBanCache.isBanned(data.ip);
-		return {banned};
+		const match = ipBanCache.getMatch(data.ip);
+		return {banned: match !== null, expires_at: toIsoString(match?.expiresAt)};
 	}
 
 	async banEmail(

fluxer_api/src/api/admin/tests/AdminIpBanExpiry.test.ts

@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {AdminRepository} from '@app/api/admin/AdminRepository';
+import type {AdminAuditLog} from '@app/api/admin/IAdminRepository';
+import {createTestAccount, setUserACLs, type TestAccount} from '@app/api/auth/tests/AuthTestUtils';
+import {ipBanCache} from '@app/api/middleware/IpBanMiddleware';
+import {getAdminRepository} from '@app/api/middleware/ServiceSingletons';
+import {type ApiTestHarness, createApiTestHarness} from '@app/api/test/ApiTestHarness';
+import {HTTP_STATUS} from '@app/api/test/TestConstants';
+import {createBuilder} from '@app/api/test/TestRequestBuilder';
+import {AdminACLs} from '@fluxer/constants/src/AdminACLs';
+import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest';
+
+interface BlocklistEntryPage {
+	items: Array<{value: string; reason: string | null; expires_at: string | null; created_at: string | null}>;
+}
+
+interface BlocklistCheck {
+	banned: boolean;
+	expires_at: string | null;
+}
+
+const HOUR_MS = 3_600_000;
+
+describe('Admin IP bans with an expiry', () => {
+	let harness: ApiTestHarness;
+	let admin: TestAccount;
+
+	beforeAll(async () => {
+		harness = await createApiTestHarness();
+	});
+
+	beforeEach(async () => {
+		await harness.reset();
+		admin = await setUserACLs(harness, await createTestAccount(harness), [
+			AdminACLs.AUTHENTICATE,
+			AdminACLs.BAN_IP_ADD,
+			AdminACLs.BAN_IP_CHECK,
+			AdminACLs.BAN_IP_REMOVE,
+		]);
+	});
+
+	afterAll(async () => {
+		await harness.shutdown();
+	});
+
+	async function addIpBan(body: Record<string, unknown>, status: number = HTTP_STATUS.NO_CONTENT): Promise<void> {
+		await createBuilder(harness, admin.token).post('/admin/blocklists/ip/entries').body(body).expect(status).execute();
+	}
+
+	async function listIpBans(): Promise<BlocklistEntryPage['items']> {
+		const page = await createBuilder<BlocklistEntryPage>(harness, admin.token)
+			.get('/admin/blocklists/ip/entries')
+			.expect(HTTP_STATUS.OK)
+			.execute();
+		return page.items;
+	}
+
+	async function checkIpBan(ip: string): Promise<BlocklistCheck> {
+		return createBuilder<BlocklistCheck>(harness, admin.token)
+			.get(`/admin/blocklists/ip/entries/${encodeURIComponent(ip)}`)
+			.expect(HTTP_STATUS.OK)
+			.execute();
+	}
+
+	async function banIpAuditLogs(): Promise<Array<AdminAuditLog>> {
+		const logs = await getAdminRepository().listAllAuditLogsPaginated(1000);
+		return logs.filter((log) => log.action === 'ban_ip');
+	}
+
+	it('stores an expiring ban that the listing and the check both report', async () => {
+		const before = Date.now();
+		await addIpBan({ip: '198.51.100.7', duration_hours: 24});
+
+		const [entry] = await listIpBans();
+		expect(entry?.value).toBe('198.51.100.7');
+		expect(entry?.reason).toBe('platform_admin_enforcement');
+		const expiresAt = Date.parse(entry?.expires_at ?? '');
+		expect(expiresAt).toBeGreaterThanOrEqual(before + 24 * HOUR_MS);
+		expect(expiresAt).toBeLessThanOrEqual(Date.now() + 24 * HOUR_MS);
+
+		const check = await checkIpBan('198.51.100.7');
+		expect(check.banned).toBe(true);
+		expect(Date.parse(check.expires_at ?? '')).toBe(expiresAt);
+	});
+
+	it('records the duration and expiry in the audit log', async () => {
+		await addIpBan({ip: '198.51.100.8', duration_hours: 168});
+
+		const [log] = await banIpAuditLogs();
+		const metadata = Object.fromEntries(log!.metadata);
+		expect(metadata['ip']).toBe('198.51.100.8');
+		expect(metadata['duration_hours']).toBe('168');
+		expect(Date.parse(metadata['expires_at'] ?? '')).toBeGreaterThan(Date.now() + 167 * HOUR_MS);
+	});
+
+	it('keeps a ban permanent when no duration is given', async () => {
+		await addIpBan({ip: '198.51.100.9'});
+		await addIpBan({ip: '198.51.100.10', duration_hours: 0});
+
+		const entries = await listIpBans();
+		expect(entries.map((entry) => entry.expires_at)).toEqual([null, null]);
+		expect(await checkIpBan('198.51.100.9')).toEqual({banned: true, expires_at: null});
+		const [log] = await banIpAuditLogs();
+		expect(log!.metadata.has('duration_hours')).toBe(false);
+	});
+
+	it('replaces a permanent ban with an expiring one when the address is banned again', async () => {
+		await addIpBan({ip: '198.51.100.11'});
+		await addIpBan({ip: '198.51.100.11', duration_hours: 24});
+
+		const [entry] = await listIpBans();
+		expect(entry?.expires_at).not.toBeNull();
+		const check = await checkIpBan('198.51.100.11');
+		expect(check.banned).toBe(true);
+		expect(check.expires_at).not.toBeNull();
+	});
+
+	it('rejects a duration beyond one year', async () => {
+		await addIpBan({ip: '198.51.100.12', duration_hours: 8761}, HTTP_STATUS.BAD_REQUEST);
+		await addIpBan({ip: '198.51.100.12', duration_hours: 1.5}, HTTP_STATUS.BAD_REQUEST);
+
+		expect(await listIpBans()).toEqual([]);
+	});
+
+	it('reports a check with no match as not banned with no expiry', async () => {
+		expect(await checkIpBan('198.51.100.13')).toEqual({banned: false, expires_at: null});
+	});
+
+	it('stops applying an expiring ban once its expiry has passed', async () => {
+		await new AdminRepository().banIp('198.51.100.14', 1);
+		await ipBanCache.refresh();
+		expect(ipBanCache.isBanned('198.51.100.14')).toBe(true);
+
+		await new Promise((resolve) => setTimeout(resolve, 1100));
+
+		expect(ipBanCache.isBanned('198.51.100.14')).toBe(false);
+		await ipBanCache.refresh();
+		expect(ipBanCache.isBanned('198.51.100.14')).toBe(false);
+		expect(await listIpBans()).toEqual([]);
+	});
+});

fluxer_api/src/api/middleware/IpBanMiddleware.ts

@@ -127,7 +127,7 @@ class IpBanCache {
 		const sameIpDecisionKey = getSameIpDecisionKey(parsed.canonical);
 		if (sameIpDecisionKey) {
 			const decisionCount = this.sameIpDecisionBans.get(sameIpDecisionKey);
-			if (decisionCount) {
+			if (decisionCount && this.isActive(decisionCount)) {
 				return {
 					ipAddress: parsed.canonical,
 					matchedEntry: sameIpDecisionKey,
@@ -137,7 +137,7 @@ class IpBanCache {
 		}
 		const singleMap = this.singleIpBans[parsed.family];
 		const single = singleMap.get(parsed.canonical);
-		if (single) {
+		if (single && this.isActive(single.count)) {
 			return {
 				ipAddress: parsed.canonical,
 				matchedEntry: parsed.canonical,
@@ -146,7 +146,7 @@ class IpBanCache {
 		}
 		const rangeMap = this.rangeIpBans[parsed.family];
 		for (const [canonical, range] of rangeMap.entries()) {
-			if (parsed.value >= range.start && parsed.value <= range.end) {
+			if (parsed.value >= range.start && parsed.value <= range.end && this.isActive(range.count)) {
 				return {
 					ipAddress: parsed.canonical,
 					matchedEntry: canonical,
@@ -232,6 +232,13 @@ class IpBanCache {
 		return count.permanent <= 0 && count.temporary <= 0;
 	}
 
+	private isActive(count: IpBanCount): boolean {
+		if (count.permanent > 0 || !count.temporaryExpiresAt) {
+			return true;
+		}
+		return count.temporaryExpiresAt.getTime() > Date.now();
+	}
+
 	private resolveCount(count: IpBanCount): {
 		kind: BannedIpKind;
 		expiresAt: Date | null;

fluxer_api/src/api/middleware/tests/IpBanMiddleware.test.ts

@@ -1,12 +1,16 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
 import {ipBanCache} from '@app/api/middleware/IpBanMiddleware';
-import {beforeEach, describe, expect, it} from 'vitest';
+import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
 
 beforeEach(() => {
 	ipBanCache.resetCaches();
 });
 
+afterEach(() => {
+	vi.useRealTimers();
+});
+
 describe('IpBanCache', () => {
 	it('blocks IPv4-mapped IPv6 when a single IPv4 address is banned', () => {
 		ipBanCache.ban('127.0.0.1');
@@ -44,4 +48,21 @@ describe('IpBanCache', () => {
 		expect(match?.kind).toBe('permanent');
 		expect(match?.expiresAt).toBe(null);
 	});
+	it('stops matching a temporary ban once it has expired', () => {
+		vi.useFakeTimers({toFake: ['Date']});
+		ipBanCache.banTemp('203.0.113.52', 3600);
+		ipBanCache.banTemp('203.0.113.0/24', 3600);
+		expect(ipBanCache.isBanned('203.0.113.52')).toBe(true);
+		expect(ipBanCache.isBanned('203.0.113.53')).toBe(true);
+		vi.advanceTimersByTime(3_600_001);
+		expect(ipBanCache.getMatch('203.0.113.52')).toBe(null);
+		expect(ipBanCache.getMatch('203.0.113.53')).toBe(null);
+	});
+	it('keeps matching a permanent ban that shares an address with an expired temporary one', () => {
+		vi.useFakeTimers({toFake: ['Date']});
+		ipBanCache.banTemp('203.0.113.54', 3600);
+		ipBanCache.ban('203.0.113.54');
+		vi.advanceTimersByTime(3_600_001);
+		expect(ipBanCache.getMatch('203.0.113.54')?.kind).toBe('permanent');
+	});
 });