feat: Add OIDC Salt service#64
Conversation
calvogenerico
left a comment
calvogenerico
left a comment
There was a problem hiding this comment.
I left several comments, but the most important one it's the combination of the values using JSON. I believe we have to go a little bit stronger there.
packages/auth-server/package.json
Outdated
| "@vueuse/nuxt": "^11.1.0", | ||
| "@wagmi/core": "^2.13.3", | ||
| "@wagmi/vue": "^0.0.49", | ||
| "axios": "^1.7.9", |
There was a problem hiding this comment.
What do you think about just using fetch? I believe we are not doing anything fancy with the requests, It's basically a GET request. I believe we can avoid adding a dependency here.
packages/auth-server/package.json
Outdated
| "@wagmi/vue": "^0.0.49", | ||
| "axios": "^1.7.9", | ||
| "jsonwebtoken": "^9.0.2", | ||
| "jwk-to-pem": "^2.0.7", |
There was a problem hiding this comment.
Have you checked jose? It's a library created by auth0 that I believe it's kind of the standard for jwt manipulation.
With jose you can use directly the JWK instead of having to convert to PEM:
https://github.com/panva/jose/blob/HEAD/docs/jwt/verify/functions/jwtVerify.md
https://github.com/panva/jose/blob/HEAD/docs/key/import/functions/importJWK.md
| 'https://accounts.google.com', | ||
| 'accounts.google.com', | ||
| ]; | ||
| const SALT_ENTROPY = process.env.SALT_ENTROPY || 'entropy'; |
There was a problem hiding this comment.
For this kind of services I like to have a "secure" default. Defaulting to an usecure string means that if we have a typo when we set this variable we are going to deploy with an unsecure value. I believe it's better to just throw if the value is missing.
| const aud = verifiedToken.aud; | ||
| const sub = verifiedToken.sub; | ||
|
|
||
| const data = { iss, aud, sub , entropy: SALT_ENTROPY }; |
There was a problem hiding this comment.
JSON it's kind of not deterministic enough for this. Different implementations might serialize keys in different order.
I believe a better approach is to treat the values as buffers and then concatenate them together. But if you do that you need to be careful about uniqueness of the keys generated. The easiest way to achieve this is by prefixing each chunk with the size, or using some separator that are sure that it's not present in the chunks that you are merging.
| return jwkToPem(jwk); | ||
| } | ||
|
|
||
| export default defineEventHandler(async (event) => { |
There was a problem hiding this comment.
Have thought how we can test this?
…ksync-sso into oidc/salt-service
* Add salt endpoint * Verify JWT * Add hash salt * Add hash salt * Update packages/auth-server/server/api/salt.ts * Lint * lint * Remove axios * Remove axios * Make SALT_ENTROPY required * Use jose * Encode data in buffer * Crop salt
* Add salt endpoint * Verify JWT * Add hash salt * Add hash salt * Update packages/auth-server/server/api/salt.ts * Lint * lint * Remove axios * Remove axios * Make SALT_ENTROPY required * Use jose * Encode data in buffer * Crop salt
* Add salt endpoint * Verify JWT * Add hash salt * Add hash salt * Update packages/auth-server/server/api/salt.ts * Lint * lint * Remove axios * Remove axios * Make SALT_ENTROPY required * Use jose * Encode data in buffer * Crop salt
2 participants
Description
Additional context