feat: fetch an entity configuration #5
Conversation
berendsliedrecht
commented
Jul 9, 2024
•
berendsliedrecht
commented
- Still needs testing
berendsliedrecht
force-pushed
the
fetch-entity-configuration
branch
2 times, most recently
from
03080bb
to
06a468a
Compare
|
|
||
| /** | ||
| * | ||
| * @todo define extact scheme |
There was a problem hiding this comment.
| * @todo define extact scheme | |
| * @todo define exact scheme |
| const { claims, header } = jsonWebTokenSchema({ | ||
| claimsSchema: entityConfigurationClaimsSchema, | ||
| headerSchema: entityConfigurationHeaderSchema, | ||
| }).parse(entityConfigurationJwt) |
There was a problem hiding this comment.
Wouldn't a entityConfigurationJwtSchema variable make sense here? Then it can be reused as well
There was a problem hiding this comment.
Yeah they can be combined.
| // TODO: create byte array of the JWT that has to be verified | ||
| const toBeVerified = new Uint8Array() | ||
|
|
||
| const isValid = await verifyJwtCallback(toBeVerified, key) |
There was a problem hiding this comment.
Should probably add a try catch as often in callbacks errors are thrown and it would be good ti handle that.
Also -- should we make the callback props an object? It's easier to extend over time without breaking changes.
There was a problem hiding this comment.
Not too opposed to making it an object, but if we extend the object vs function params we will have the same amount of breaking changes.
Add a required prop to an object vs function param == breaking change
add an optional prop to an object vs function param == no breaking change
I do prefer it as there are two inputs of the same type so the ordering can be messed up there.
| @@ -0,0 +1,134 @@ | |||
| import assert from 'node:assert' | |||
| import {} from 'node:crypto' | |||
There was a problem hiding this comment.
| import {} from 'node:crypto' |
| * @todo make get/post use the same method private internally as changes with apply to both | ||
| * | ||
| */ | ||
| const post = async ({ |
There was a problem hiding this comment.
being able to optionally provide a custom fetch instance may be nice?
E.g. with Paradym we limit the domains/IPs that can be called with our fetch implementation, to prevent untrusted urls calling internal services.
There was a problem hiding this comment.
Yes that is the idea. But I have not thought of a way, yet, to do this properly. We can go the x509/crypto way (fetchEntityConfiguration(...normalParams, fetch? = global.fetch)` but I am not sure, yet.
| import type { SignCallback, VerifyCallback } from '../src/utils' | ||
|
|
||
| import assert from 'node:assert/strict' | ||
| import nock from 'nock' |
There was a problem hiding this comment.
I've had issues with nock in Paradym when using native node fetch, so I moved to msw which works quite well. If you're not experiencing these issues, nock is fine with me, as the API is nice 👍
There was a problem hiding this comment.
nock beta supports the native fetch! If I encounter errors with it I'll swap it.
Yes, I would like to revisit that when the first alpha version is finished. |
Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>
berendsliedrecht
force-pushed
the
fetch-entity-configuration
branch
from
06a468a
to
a86e90b
Compare
