feat: Implemented KMS, JWKS generation and JWT sign #14
Conversation
...es/openid-federation-common/src/commonMain/kotlin/com/sphereon/oid/fed/common/jwt/JoseJwt.kt
Outdated
Show resolved
Hide resolved
...s/openid-federation-common/src/jvmMain/kotlin/com/sphereon/oid/fed/common/jwt/JoseJwt.jvm.kt
Outdated
Show resolved
Hide resolved
...s/openid-federation-common/src/jvmMain/kotlin/com/sphereon/oid/fed/common/jwt/JoseJwt.jvm.kt
Outdated
Show resolved
Hide resolved
…lt value and refactored the key generation
…ation into feature/OIDF-42
|
You cannot just invent your own kid with a uuid |
| opts: Map<String, Any> | ||
| ): String { | ||
| val rsaJWK = opts["key"] as RSAKey? ?: RSAKeyGenerator(2048) | ||
| .keyID(UUID.randomUUID().toString()) |
There was a problem hiding this comment.
@nklomp made a comment about not being possible to generate a random kid with uuid
There was a problem hiding this comment.
I've made header the second parameter of the sign function, the key should be passed in for both js and jvm now.
| opts: Map<String, Any> | ||
| ): String { | ||
| val privateKey = opts["privateKey"] ?: throw IllegalArgumentException("JWK private key is required") | ||
| val header = opts["jwtHeader"] as String? ?: "{\"typ\":\"JWT\",\"alg\":\"RS256\",\"kid\":\"${Uuid.v4()}\"}" |
There was a problem hiding this comment.
You should not just create a header, let alone use a random kid value.
Although "kid" is officially not defined, normally they are the JWK thumbprint in case a JWK is used, or a DID VM in case DIDs are used.
So please make a header a first class citizen like the payload. Require it to be passed in. 2nd don't create random kids.
There was a problem hiding this comment.
I've made header the second parameter of the sign function, the key should be passed in for both js and jvm now.
…ration into feature/OIDF-42 # Conflicts: # modules/openid-federation-common/build.gradle.kts
| @@ -0,0 +1,4 @@ | |||
| package com.sphereon.oid.fed.common.jwt | |||
|
|
|||
| expect fun sign(payload: String, header: String, opts: Map<String, Any>): String | |||
There was a problem hiding this comment.
I do not like this whole signature.
First of all, opts: Map<String, Any>, with this you are putting the platform differences with the implementer. Please create a static options data class with options that work for all platforms. Do the mapping in the platform implementation. If there are options available on one platform that are not available on the other we should think hard if we really need to expose them or if we can omit them or set a default value that works for us.
Then payload: String, header: String is also not very clear. Make it at least
jwtHeader:String, jwtPayload:String or better, create a little data class
data class UnsignedJWT(val header:String, val payload,String) and use that as parameter.
There was a problem hiding this comment.
See the discussion in slack about the opts. They are they are there for external implementors. Not for platform specifics.
Regarding the header and payload. How did that ever change to string. That doesn't make any sense. These should be objects, that are translated into strings in this exact function
* fix: add missing repositories for windows * fix: update ci docker compose command
* chore: Removed redundant HTTPCache * chore: Uncommented ios targets back * refactor: refactored serializeNullable() * refactor: refactored deserialize() * refactor: refactored OutgoingEntityStatementContent.bytes() * refactor: refactored the tests to use assertEquals() * refactor: Changed the response body to jwt string * refactor: Removed unnecessary converter * feat: implement jwk persistence * fix: remove unused statement * fix: github CI * feat/OIDF-51 - Implement Persistence Module (#21) * merge oidf-7 * fix: models package * fix: openapi TrustMarkOwner property * fix: create account method return type * fix: rename file for consistency * feat: implement migration * fix: repository dependency * fix: add missing trailing new line * feat: implement services module * fix: package path * fix: remove unused file * fix: add missing entity to openapi spec * feat: persist generated keys * fix: typo * fix: missing deps * fix: ci docker command * fix: dependency * fix: remove unnecessary statement * feat: abstract jwk to its own module * feat: encrypt private keys when saving to database * feat: add note to README regarding usage of Local KMS in prod envs * fix: adapt key encryption test cases for when APP_KEY is null * fix: adjust function name * fix: add kotlin-js-store to gitignore * fix: clean common gradle file * fix: disable android build * fix: remove js implementation from services * feat: implement Subordinate repository (#29) * feat: implement federation server structure (#28) * feat: implement federation server structure * feat: implement Subordinate repository * fix: remove unused files * feat: implement federation list endpoint --------- Co-authored-by: Zoe Maas <[email protected]>
No description provided.