JWT condition #604
JWT condition #604
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## epic-v0.6.x #604 +/- ##
==============================================
Coverage ? 89.30%
==============================================
Files ? 77
Lines ? 6676
Branches ? 363
==============================================
Hits ? 5962
Misses ? 679
Partials ? 35 ☔ View full report in Codecov by Sentry. |
|
|
||
| export const jwtConditionSchema = baseConditionSchema.extend({ | ||
| conditionType: z.literal(JWTConditionType).default(JWTConditionType), | ||
| public_key: z.string().optional(), |
There was a problem hiding this comment.
are these parameters really optional?
There was a problem hiding this comment.
I need @cygnusv help with that
| subject: contextParamSchema.optional(), | ||
| expiration_window: z.number().int().nonnegative().optional(), | ||
| issued_window: z.number().int().nonnegative().optional(), | ||
| jwtToken: contextParamSchema.default(JWT_PARAM_DEFAULT), |
There was a problem hiding this comment.
isn't the jwtToken what the user will provide at decryption time? rather than being defined in the condition
There was a problem hiding this comment.
yep, you are correct, here is defined that some user param must be provided or used default :jwtToken
There was a problem hiding this comment.
@theref see discussion in nucypher/nucypher#3567 (comment)
|
Converted to draft since this work shouldn't be merged before nucypher/nucypher#3570 (currently in draft) is merged |
| @@ -19,3 +19,8 @@ export const TEST_SIWE_PARAMS = { | |||
| domain: 'localhost', | |||
| uri: 'http://localhost:3000', | |||
| }; | |||
|
|
|||
| export const TEST_ECDSA_PUBLIC_KEY = | |||
There was a problem hiding this comment.
Is it possible to provide the public key without the PEM header and footer i.e. just use the base64, or hex if that is more familiar/acceptable (although less efficient in terms of space)? On the server-side we can do some adjustment if needed: convert it to PEM, OR not bother if the library allows base64/hex format, or ...
(cc @cygnusv )
| subject: contextParamSchema.optional(), | ||
| expirationWindow: z.number().int().nonnegative().optional(), | ||
| issuedWindow: z.number().int().nonnegative().optional(), |
There was a problem hiding this comment.
@vzotova let's comment/drop these lines for the moment. The corresponding PR on the nucypher side will only validate the expected issuer and public key. We can easily add more restrictions like these once we have more clarity on what's actually needed.
|
|
||
| export const jwtConditionSchema = baseConditionSchema.extend({ | ||
| conditionType: z.literal(JWTConditionType).default(JWTConditionType), | ||
| publicKey: z.string().optional(), |
There was a problem hiding this comment.
Based on nucypher/nucypher#3570, publicKey is a required property.
Type of PR:
Required reviews:
What this does:
Implements JWT condition
Issues fixed/closed:
See #3567
Why it's needed:
Notes for reviewers: