-
Notifications
You must be signed in to change notification settings - Fork 4
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 support for batched token requests in privacypass-issuer #83
base: main
Are you sure you want to change the base?
Add support for batched token requests in privacypass-issuer #83
Conversation
Looks good to me. Just not sure about the indentation (tabs or spaces). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the change looks great and functional.
main comment is on the RSA-RAW change
src/index.ts
Outdated
// name: ctx.isTest() ? 'RSA-PSS' : 'RSA-RAW', | ||
// RSA-RAW is handled directlry by blindrsa-tsst() | ||
// https://github.com/cloudflare/blindrsa-ts/blob/64dc207f079ca680d4db6cd5a22f5bce6f1b28e8/src/blindrsa.ts#L168 | ||
name: 'RSA-PSS', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not change
you should consider adding the importKey mock for the test, as is done on cloudflare/privacypass-ts
321d396
to
2a7d82a
Compare
2a7d82a
to
c632a40
Compare
This upgrades privacypass-ts to the latest version, and updates tests accordingly
…rbitrary_batched_toknes
cb6889f
to
3912840
Compare
3912840
to
18c7e8f
Compare
const client = new Client(BlindRSAMode.PSS); | ||
export async function testE2E(issuerName: string, nTokens: number, mTLS?: MTLSConfiguration): Promise<boolean> { | ||
return nTokens > 1 | ||
? testArbitraryBatchedRequest(issuerName, nTokens, mTLS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test must be split in two cases based on the type of token, not on the number of tokens requested.
Note that it's valid to perform a batchedTokenRequest with 0, 1, or more tokens.
'verify', | ||
]); | ||
} | ||
const MODE = BlindRSAMode.PSS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is used only once, better to remove MODE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's include a e2e test case for batchedtokens with 0,1, or more tokens
This PR introduces several updates and improvements to the
privacypass-issuer
project. Key changes include:InvalidContentTypeError
andMismatchedTokenKeyIDError
to handle specific error scenarios.handleTokenRequest
to handle different content types (PRIVATE_TOKEN_REQUEST
andARBITRARY_BATCHED_TOKEN_REQUEST
).handleSingleTokenRequest
for single token issuance.handleBatchedTokenRequest
for batched token issuance, including validation and key-pair retrieval.getBlindRSAKeyPair
to handle key retrieval and error logging.index.test.ts
to verify the handling of batched token requests.e2e/issuer.ts
to verify handling of batched token requests in a deployed worker.- Updated
testCommand
ine2e/index.ts
to accept the number of tokens for testing.--tokens
option to specify the number of tokens to use in tests.