Skip to content

Commit 23ac471

Browse files
committed
fix for caching
1 parent 8456f1e commit 23ac471

File tree

3 files changed

+131
-41
lines changed

3 files changed

+131
-41
lines changed

server/api/teams.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import { Options, type Scope } from '@/model/Options'
22
import type { H3Event, EventHandlerRequest } from 'h3'
33

4-
const cache = new Map<string, CacheData>()
5-
6-
interface CacheData {
7-
data: Team[]
8-
valid_until: number
9-
}
104
interface Team { name: string; slug: string; description: string }
115
interface GitHubTeam { name: string; slug: string; description?: string }
126

@@ -72,16 +66,6 @@ export async function getTeams(event: H3Event<EventHandlerRequest>): Promise<Tea
7266
return teams
7367
}
7468

75-
// Use cache if valid
76-
if (cache.has(event.path)) {
77-
const cachedData = cache.get(event.path)
78-
if (cachedData && cachedData.valid_until > Math.floor(Date.now() / 1000)) {
79-
logger.info(`Returning cached data for ${event.path}`)
80-
return cachedData.data
81-
}
82-
cache.delete(event.path)
83-
}
84-
8569
if (!event.context.headers.has('Authorization')) {
8670
logger.error('No Authentication provided')
8771
throw new TeamsError('No Authentication provided', 401)
@@ -114,7 +98,5 @@ export async function getTeams(event: H3Event<EventHandlerRequest>): Promise<Tea
11498
page += 1
11599
}
116100

117-
// Cache for 60 seconds
118-
cache.set(event.path, { data: allTeams, valid_until: Math.floor(Date.now() / 1000) + 60 })
119101
return allTeams
120102
}

shared/utils/metrics-util.ts

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import { readFileSync } from 'fs';
55
import { resolve } from 'path';
66
import { getLocale } from "./getLocale";
77
import { filterHolidaysFromMetrics, isHoliday, parseUtcDate } from '@/utils/dateUtils';
8+
import { createHash } from 'crypto';
89

910
const cache = new Map<string, CacheData>();
1011

1112
interface CacheData {
1213
data: CopilotMetrics[];
1314
valid_until: number;
15+
auth_fingerprint: string; // hashed representation of Authorization header used to populate cache
1416
}
1517

1618
class MetricsError extends Error {
@@ -24,6 +26,50 @@ class MetricsError extends Error {
2426
}
2527
}
2628

29+
/**
30+
* Builds a cache key for metrics data that is bound to the caller's Authorization header (hashed) + path + query.
31+
* Exported for unit testing.
32+
*/
33+
type QueryParamValue = string | string[] | undefined;
34+
type QueryParams = Record<string, QueryParamValue>;
35+
36+
export function buildMetricsCacheKey(path: string, query: QueryParams, authHeader: string): string {
37+
// Split existing query params from provided path (if any)
38+
const [rawPath, existingQueryString] = path.split('?');
39+
const merged = new Map<string, string>();
40+
41+
// Add existing params first
42+
if (existingQueryString) {
43+
const existingParams = new URLSearchParams(existingQueryString);
44+
existingParams.forEach((value, key) => {
45+
merged.set(key, value);
46+
});
47+
}
48+
49+
// Merge in provided query object (overrides existing)
50+
Object.entries(query).forEach(([k, v]) => {
51+
if (v === undefined || v === null) return;
52+
if (Array.isArray(v)) {
53+
if (v.length === 0) return;
54+
merged.set(k, v.join(','));
55+
} else if (v !== '') {
56+
merged.set(k, v);
57+
}
58+
});
59+
60+
// Build stable, sorted query string
61+
const sortedKeys = Array.from(merged.keys()).sort();
62+
const finalParams = new URLSearchParams();
63+
sortedKeys.forEach(k => {
64+
const val = merged.get(k);
65+
if (val !== undefined) finalParams.set(k, val);
66+
});
67+
const finalQueryString = finalParams.toString();
68+
69+
const authFingerprint = createHash('sha256').update(authHeader).digest('hex').slice(0, 16); // short fingerprint
70+
return `${authFingerprint}:${rawPath}${finalQueryString ? `?${finalQueryString}` : ''}`;
71+
}
72+
2773
export async function getMetricsData(event: H3Event<EventHandlerRequest>): Promise<CopilotMetrics[]> {
2874
const logger = console;
2975
const query = getQuery(event);
@@ -52,30 +98,33 @@ export async function getMetricsData(event: H3Event<EventHandlerRequest>): Promi
5298
return usageData;
5399
}
54100

55-
// Create cache key that includes query parameters to differentiate between different date ranges
56-
const queryString = new URLSearchParams(query as Record<string, string>).toString();
57-
const cacheKey = queryString ? `${event.path}?${queryString}` : event.path;
101+
// Authorization must be validated BEFORE any cache lookup to prevent leakage of cached data
102+
const authHeader = event.context.headers.get('Authorization');
103+
if (!authHeader) {
104+
logger.error('No Authentication provided');
105+
throw new MetricsError('No Authentication provided', 401);
106+
}
107+
108+
// Build auth-bound cache key
109+
const path = event.path || '/api/metrics'; // fallback path (should always exist in practice)
110+
const cacheKey = buildMetricsCacheKey(path, query as QueryParams, authHeader);
58111

59-
if (cache.has(cacheKey)) {
60-
const cachedData = cache.get(cacheKey);
61-
if (cachedData && cachedData.valid_until > Date.now() / 1000) {
112+
// Attempt cache lookup with auth fingerprint validation
113+
const cachedData = cache.get(cacheKey);
114+
if (cachedData) {
115+
if (cachedData.valid_until > Date.now() / 1000 && cachedData.auth_fingerprint === cacheKey.split(':')[0]) {
62116
logger.info(`Returning cached data for ${cacheKey}`);
63117
return cachedData.data;
64118
} else {
65-
logger.info(`Cached data for ${cacheKey} is expired, fetching new data`);
119+
logger.info(`Cached data for ${cacheKey} is expired or fingerprint mismatch, fetching new data`);
66120
cache.delete(cacheKey);
67121
}
68122
}
69123

70-
if (!event.context.headers.has('Authorization')) {
71-
logger.error('No Authentication provided');
72-
throw new MetricsError('No Authentication provided', 401);
73-
}
74-
75124
logger.info(`Fetching metrics data from ${apiUrl}`);
76125

77126
try {
78-
const response = await $fetch(apiUrl, {
127+
const response = await $fetch(apiUrl, {
79128
headers: event.context.headers
80129
}) as unknown[];
81130

@@ -85,7 +134,8 @@ export async function getMetricsData(event: H3Event<EventHandlerRequest>): Promi
85134
const filteredUsageData = filterHolidaysFromMetrics(usageData, options.excludeHolidays || false, options.locale);
86135
// metrics is the old API format
87136
const validUntil = Math.floor(Date.now() / 1000) + 5 * 60; // Cache for 5 minutes
88-
cache.set(cacheKey, { data: filteredUsageData, valid_until: validUntil });
137+
const authFingerprint: string = cacheKey.split(':', 1)[0] || '';
138+
cache.set(cacheKey, { data: filteredUsageData, valid_until: validUntil, auth_fingerprint: authFingerprint });
89139
return filteredUsageData;
90140
} catch (error: unknown) {
91141
logger.error('Error fetching metrics data:', error);
@@ -131,17 +181,12 @@ function updateMockDataDates(originalData: CopilotMetrics[], since?: string, unt
131181
}
132182

133183
// Update dates in the dataset, copying existing entries when needed
134-
const result = dateRange.map((date, index) => {
135-
// Use existing data entries, cycling through them
184+
const result: CopilotMetrics[] = dateRange.map((date, index) => {
136185
const dataIndex = index % originalData.length;
137-
const entry = { ...originalData[dataIndex] };
138-
139-
// Update the date
140-
entry.date = date.toISOString().split('T')[0];
141-
142-
return entry;
186+
const src = originalData[dataIndex];
187+
const newDate = date.toISOString().split('T')[0];
188+
return { ...(src as CopilotMetrics), date: newDate } as CopilotMetrics;
143189
});
144-
145190
return result;
146191
}
147192

tests/metrics-auth-cache.nuxt.spec.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// @vitest-environment nuxt
2+
import { describe, it, expect } from 'vitest'
3+
import { buildMetricsCacheKey } from '../shared/utils/metrics-util'
4+
5+
/**
6+
* These tests validate that the new cache key generation logic binds cache entries
7+
* to the caller's Authorization header fingerprint and that different auth headers
8+
* produce distinct keys even when path + query are identical.
9+
*/
10+
11+
describe('Metrics Auth-Bound Cache Key', () => {
12+
const path = '/api/metrics'
13+
const query = { since: '2024-01-01', until: '2024-01-31', scope: 'organization', githubOrg: 'test-org' }
14+
15+
it('produces different keys for different auth headers', () => {
16+
const keyA1 = buildMetricsCacheKey(path, query, 'token userA-1')
17+
const keyA2 = buildMetricsCacheKey(path, query, 'token userA-2')
18+
const keyB = buildMetricsCacheKey(path, query, 'token userB-1')
19+
20+
expect(keyA1).not.toBe(keyA2)
21+
expect(keyA1.split(':')[0]).not.toBe(keyA2.split(':')[0])
22+
expect(keyA1.split(':')[0]).not.toBe(keyB.split(':')[0])
23+
})
24+
25+
it('is stable for same auth header + query', () => {
26+
const key1 = buildMetricsCacheKey(path, query, 'token stable-user')
27+
const key2 = buildMetricsCacheKey(path, query, 'token stable-user')
28+
expect(key1).toBe(key2)
29+
})
30+
31+
it('filters out undefined/empty query params', () => {
32+
const key = buildMetricsCacheKey(path, { since: '2024-01-01', empty: '', undef: undefined }, 'token x')
33+
expect(key).toContain('since=2024-01-01')
34+
expect(key).not.toContain('empty=')
35+
expect(key).not.toContain('undef=')
36+
})
37+
38+
it('joins array query params deterministically', () => {
39+
const key = buildMetricsCacheKey(path, { since: ['2024-01-01'], tag: ['a','b'] }, 'token y')
40+
expect(key).toContain('since=2024-01-01')
41+
expect(key).toMatch(/tag=a%2Cb|tag=b%2Ca/) // order preserved from input array join
42+
})
43+
44+
it('merges existing path query params with provided query object without duplication', () => {
45+
const pathWithQuery = '/api/metrics?since=2024-01-01&scope=organization'
46+
const key = buildMetricsCacheKey(pathWithQuery, { githubOrg: 'test-org', since: '2024-01-01' }, 'token z')
47+
// should not duplicate since param and should include githubOrg
48+
const pieces = key.split(':')
49+
expect(pieces.length).toBeGreaterThan(1)
50+
const qs = pieces.slice(1).join(':').split('?')[1]
51+
expect(qs?.match(/since=/g)?.length).toBe(1)
52+
expect(qs).toContain('githubOrg=test-org')
53+
})
54+
55+
it('sorts final query params for stable ordering', () => {
56+
const key1 = buildMetricsCacheKey('/api/metrics?b=2&a=1', { c: '3' }, 'token sort')
57+
const key2 = buildMetricsCacheKey('/api/metrics?a=1&b=2', { c: '3' }, 'token sort')
58+
expect(key1).toBe(key2)
59+
const qs = key1.split('?')[1]
60+
expect(qs).toBeDefined()
61+
expect(qs!.startsWith('a=1&b=2&c=3')).toBe(true)
62+
})
63+
})

0 commit comments

Comments
 (0)