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

Add cache issuer directory #15

Merged
merged 6 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
274 changes: 119 additions & 155 deletions package-lock.json

Large diffs are not rendered by default.

18 changes: 16 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,28 @@
"eslintConfig": {
"root": true,
"extends": [
"typescript",
"plugin:@typescript-eslint/recommended",
"prettier"
]
],
"parser": "@typescript-eslint/parser",
"plugins": [
"@typescript-eslint"
],
"rules": {
"@typescript-eslint/no-unused-vars": [
"warn",
{
"argsIgnorePattern": "^_"
}
]
}
},
"devDependencies": {
"@cloudflare/blindrsa-ts": "0.3.2",
"@cloudflare/workers-types": "4.20240117.0",
"@types/jest": "29.5.11",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@typescript-eslint/parser": "^6.21.0",
"dotenv": "16.4.0",
"esbuild": "0.19.12",
"eslint": "8.56.0",
Expand Down
1 change: 1 addition & 0 deletions src/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { R2Bucket } from '@cloudflare/workers-types/2023-07-01';

export interface Bindings {
// variables and secrets
DIRECTORY_CACHE_MAX_AGE_SECONDS: string;
ENVIRONMENT: string;
LOGGING_SHIM_TOKEN: string;
SENTRY_ACCESS_CLIENT_ID: string;
Expand Down
22 changes: 14 additions & 8 deletions src/context/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,33 @@ export class MetricsRegistry {
registry: RegistryType;
options: RegistryOptions;

requestsTotal: CounterType;
directoryCacheMissTotal: CounterType;
erroredRequestsTotal: CounterType;
issuanceRequestTotal: CounterType;
keyRotationTotal: CounterType;
keyClearTotal: CounterType;
issuanceRequestTotal: CounterType;
requestsTotal: CounterType;
signedTokenTotal: CounterType;

constructor(options: RegistryOptions) {
this.options = options;
this.registry = new Registry();

this.requestsTotal = this.registry.create('counter', 'requests_total', 'total requests');
this.directoryCacheMissTotal = this.registry.create(
'counter',
'directory_cache_miss_total',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the metric counts cache miss. It is also possible to devise a metric directory_cache_request_total{status="hit|miss"}. However, I feel this would add uneccessary weight.

'Number of requests for private token issuer directory which are not served by the cache.'
);
this.erroredRequestsTotal = this.registry.create(
'counter',
'errored_requests_total',
'Errored requests served to eyeball'
);
this.issuanceRequestTotal = this.registry.create(
'counter',
'issuance_request_total',
'Number of requests for private token issuance.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

successful?

Suggested change
'Number of requests for private token issuance.'
'Number of successful requests for private token issuance.'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary successful. the metric is increased at the start of handleTokenRequest, no matter the response.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side: this is also not a new metrics. The metrics have been sorted by alphabetical orders, and it's part of this change.

);
this.keyRotationTotal = this.registry.create(
'counter',
'key_rotation_total',
Expand All @@ -40,11 +50,7 @@ export class MetricsRegistry {
'key_clear_total',
'Number of key clear performed.'
);
this.issuanceRequestTotal = this.registry.create(
'counter',
'issuance_request_total',
'Number of requests for private token issuance.'
);
this.requestsTotal = this.registry.create('counter', 'requests_total', 'total requests');
this.signedTokenTotal = this.registry.create(
'counter',
'signed_token_total',
Expand Down
64 changes: 59 additions & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from '@cloudflare/privacypass-ts';
import { ConsoleLogger } from './context/logging';
import { MetricsRegistry } from './context/metrics';
import { hexEncode } from './utils/hex';
const { BlindRSAMode, Issuer, TokenRequest } = publicVerif;

const keyToTokenKeyID = async (key: Uint8Array): Promise<number> => {
Expand Down Expand Up @@ -79,7 +80,38 @@ export const handleTokenRequest = async (ctx: Context, request: Request) => {
});
};

export const handleTokenDirectory = async (ctx: Context, request?: Request) => {
const getDirectoryCache = async (): Promise<Cache> => {
return caches.open('response/issuer-directory');
};

const FAKE_DOMAIN_CACHE = 'cache.local';
const DIRECTORY_CACHE_REQUEST = new Request(
`https://${FAKE_DOMAIN_CACHE}${PRIVATE_TOKEN_ISSUER_DIRECTORY}`
);

export const handleHeadTokenDirectory = async (ctx: Context, request: Request) => {
const getResponse = await handleTokenDirectory(ctx, request);

return new Response(undefined, {
status: getResponse.status,
headers: getResponse.headers,
});
};

export const handleTokenDirectory = async (ctx: Context, request: Request) => {
const cache = await getDirectoryCache();
const cachedResponse = await cache.match(DIRECTORY_CACHE_REQUEST);
if (cachedResponse) {
if (request.headers.get('if-none-match') === cachedResponse.headers.get('etag')) {
return new Response(undefined, {
status: 304,
headers: cachedResponse.headers,
});
}
return cachedResponse;
}
ctx.metrics.directoryCacheMissTotal.inc({ env: ctx.env.ENVIRONMENT });

const keys = await ctx.env.ISSUANCE_KEYS.list({ include: ['customMetadata'] });

if (keys.objects.length === 0) {
Expand All @@ -95,15 +127,32 @@ export const handleTokenDirectory = async (ctx: Context, request?: Request) => {
})),
};

return new Response(JSON.stringify(directory), {
const body = JSON.stringify(directory);
const digest = new Uint8Array(
await crypto.subtle.digest('SHA-256', new TextEncoder().encode(body))
);
const etag = `"${hexEncode(digest)}"`;

const response = new Response(body, {
headers: {
'content-type': MediaType.PRIVATE_TOKEN_ISSUER_DIRECTORY,
'cache-control': 'public, max-age=86400',
'cache-control': `public, max-age=${ctx.env.DIRECTORY_CACHE_MAX_AGE_SECONDS}`,
'content-length': body.length.toString(),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Isn't content-length automatically set? Or is it missing due to chunked encoding being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I first tested, it did not seem to be set automatically. therefore adding it here. The question I have is if the request is compressed, I'm not sure if it's modified automatically.

'date': new Date().toUTCString(),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Date is not that important for caching, has this been added for debugging purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's been added to help debugging if there are caching issues in the future.

etag,
},
});
ctx.waitUntil(cache.put(DIRECTORY_CACHE_REQUEST, response.clone()));

return response;
};

export const handleRotateKey = async (ctx: Context, request?: Request) => {
const clearDirectoryCache = async (): Promise<boolean> => {
const cache = await getDirectoryCache();
return cache.delete(DIRECTORY_CACHE_REQUEST);
};

export const handleRotateKey = async (ctx: Context, _request?: Request) => {
ctx.metrics.keyRotationTotal.inc({ env: ctx.env.ENVIRONMENT });

// Generate a new type 2 Issuer key
Expand Down Expand Up @@ -148,10 +197,12 @@ export const handleRotateKey = async (ctx: Context, request?: Request) => {
customMetadata: metadata,
});

ctx.waitUntil(clearDirectoryCache());

return new Response(`New key ${publicKeyEnc}`, { status: 201 });
};

const handleClearKey = async (ctx: Context, request?: Request) => {
const handleClearKey = async (ctx: Context, _request?: Request) => {
ctx.metrics.keyClearTotal.inc({ env: ctx.env.ENVIRONMENT });
const keys = await ctx.env.ISSUANCE_KEYS.list();

Expand All @@ -169,6 +220,9 @@ const handleClearKey = async (ctx: Context, request?: Request) => {
}
const toDeleteArray = [...toDelete];
await ctx.env.ISSUANCE_KEYS.delete(toDeleteArray);

ctx.waitUntil(clearDirectoryCache());

return new Response(`Keys cleared: ${toDeleteArray.join('\n')}`, { status: 201 });
};

Expand Down
13 changes: 13 additions & 0 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ export class Router {
// normalise path, so that they never end with a trailing '/'
path = this.normalisePath(path);
this.handlers[method][path] = handler;
if (method === HttpMethod.GET) {
this.handlers[HttpMethod.HEAD] ??= {};
this.handlers[HttpMethod.HEAD][path] = async (
ctx: Context,
request: Request
): Promise<Response> => {
const response = await handler(ctx, request);
if (response.ok) {
return new Response(null, response);
}
return response;
};
}
return this;
}

Expand Down
12 changes: 12 additions & 0 deletions src/utils/hex.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const hexDecode = (s: string) => {
const bytes = s.match(/.{1,2}/g);
if (!bytes) {
return new Uint8Array(0);
}
return Uint8Array.from(bytes.map(b => parseInt(b, 16)));
};

export const hexEncode = (u: Uint8Array) =>
Array.from(u)
.map(b => b.toString(16).padStart(2, '0'))
.join('');
61 changes: 60 additions & 1 deletion test/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { jest } from '@jest/globals';

import { Context } from '../src/context';
import { handleTokenRequest, default as workerObject } from '../src/index';
import { IssuerConfigurationResponse } from '../src/types';
import { b64ToB64URL, u8ToB64 } from '../src/utils/base64';
import { ExecutionContextMock, getContext, getEnv } from './mocks';
import { ExecutionContextMock, MockCache, getContext, getEnv } from './mocks';
import { RSABSSA } from '@cloudflare/blindrsa-ts';
import {
MediaType,
Expand Down Expand Up @@ -147,3 +149,60 @@ describe('rotate and clear key', () => {
expect(directory['token-keys']).toHaveLength(1);
});
});

describe('cache directory response', () => {
const rotateURL = `${sampleURL}/admin/rotate`;
const directoryURL = `${sampleURL}${PRIVATE_TOKEN_ISSUER_DIRECTORY}`;

const initializeKeys = async (numberOfKeys = 1): Promise<void> => {
const rotateRequest = new Request(rotateURL, { method: 'POST' });

for (let i = 0; i < numberOfKeys; i += 1) {
await workerObject.fetch(rotateRequest, getEnv(), new ExecutionContextMock());
}
};

it('should cache the directory response', async () => {
await initializeKeys();

const mockCache = new MockCache();
const spy = jest.spyOn(caches, 'open').mockResolvedValue(mockCache);
const directoryRequest = new Request(directoryURL);

let response = await workerObject.fetch(directoryRequest, getEnv(), new ExecutionContextMock());
expect(response.ok).toBe(true);
expect(Object.entries(mockCache.cache)).toHaveLength(1);

const [cachedURL, _] = Object.entries(mockCache.cache)[0];
const sampleEtag = '"sampleEtag"';
const cachedResponse = new Response('cached response', { headers: { etag: sampleEtag } });
mockCache.cache[cachedURL] = cachedResponse;

response = await workerObject.fetch(directoryRequest, getEnv(), new ExecutionContextMock());
expect(response.ok).toBe(true);
expect(response).toBe(cachedResponse);

const cachedDirectoryRequest = new Request(directoryURL, {
headers: { 'if-none-match': sampleEtag },
});
response = await workerObject.fetch(
cachedDirectoryRequest,
getEnv(),
new ExecutionContextMock()
);
expect(response.status).toBe(304);

const headCachedDirectoryRequest = new Request(directoryURL, {
method: 'HEAD',
headers: { 'if-none-match': sampleEtag },
});
response = await workerObject.fetch(
headCachedDirectoryRequest,
getEnv(),
new ExecutionContextMock()
);
expect(response.status).toBe(304);

spy.mockClear();
});
});
25 changes: 25 additions & 0 deletions test/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,31 @@ import { Context, WaitUntilFunc } from '../src/context';
import { ConsoleLogger, Logger } from '../src/context/logging';
import { MetricsRegistry } from '../src/context/metrics';

export class MockCache implements Cache {
public cache: Record<string, Response> = {};

async match(info: RequestInfo, options?: CacheQueryOptions): Promise<Response | undefined> {
if (options) {
throw new Error('CacheQueryOptions not supported');
}
const url = new URL(info instanceof Request ? info.url : info).href;
return this.cache[url];
}

async delete(info: RequestInfo, options?: CacheQueryOptions): Promise<boolean> {
if (options) {
throw new Error('CacheQueryOptions not supported');
}
const url = new URL(info instanceof Request ? info.url : info).href;
return delete this.cache[url];
}

async put(info: RequestInfo, response: Response): Promise<void> {
const url = new URL(info instanceof Request ? info.url : info).href;
this.cache[url] = response;
}
}

export class ExecutionContextMock implements ExecutionContext {
waitUntils: Promise<any>[] = [];
passThrough = false;
Expand Down
1 change: 1 addition & 0 deletions wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ route = { pattern = "pp-issuer.example.test", custom_domain=true }
crons = ["0 0 * * *"]

[env.production.vars]
DIRECTORY_CACHE_MAX_AGE_SECONDS = "86400"
ENVIRONMENT = "production"
SENTRY_SAMPLE_RATE = "0" # Between 0-1 if you log errors on Sentry. 0 disables Sentry logging. Configuration is done through Workers Secrets

Expand Down
Loading