[86] Ignore [String] JWK parameters
#116
Conversation
ghost
requested review from
There was a problem hiding this comment.
Hi @xaverlohmueller, 👋
thanks so much for implementing this workaround. Sorry for the late reply we had a quite busy start of the week.
It already looks really good! Please have a look at my comments and tell me what you think. 🙂 Afterward we'll merge and release a new version with this workaround.
| @@ -62,7 +62,7 @@ extension RSAPublicKey: Decodable { | |||
|
|
|||
| // Other common parameters are optional. | |||
| var parameters: [String: String] = [:] | |||
| for key in commonParameters.allKeys { | |||
| for key in commonParameters.allKeys where key != .X509CertificateChain { | |||
There was a problem hiding this comment.
Please also ignore the keyOperations parameter, it is an array of strings as well.
| for key in commonParameters.allKeys where key != .X509CertificateChain { | |
| for key in commonParameters.allKeys where key != .X509CertificateChain && key != .keyOperations { |
There was a problem hiding this comment.
It would be awesome if you could also add the workaround to the decoding of RSAPrivateKey here.
Tests/JWKx5cTests.swift
Outdated
| @testable import JOSESwift | ||
|
|
||
| class JWKx5cTests: XCTestCase { | ||
| func testBuildingJWKSetShouldNotFailIfCertificatesArePresent() { |
There was a problem hiding this comment.
This test would probably fit nicely into JWKRSADecodingTests.swift.
ghost
mentioned this pull request
|
Hey, thanks for your review and suggestions for improvement! I implemented the requested changes. |
ghost
mentioned this pull request
| @@ -119,7 +119,7 @@ extension RSAPrivateKey: Decodable { | |||
|
|
|||
| // Other common parameters are optional. | |||
| var parameters: [String: String] = [:] | |||
| for key in commonParameters.allKeys { | |||
| for key in commonParameters.allKeys where key != .X509CertificateChain { | |||
There was a problem hiding this comment.
It would be awesome if you could also ignore the keyOperations parameter here and in the public key decoding.
| for key in commonParameters.allKeys where key != .X509CertificateChain { | |
| for key in commonParameters.allKeys where key != .X509CertificateChain && key != .keyOperations { |
ghost
changed the title
[String] JWK parameters
|
Awesome! |
|
Thanks for your contribution @xaverlohmueller from my side as well! 👏 Just FYI: A new version with the merged changes will be released during the day. |
|
The 1.3.1 release is now live on CocoaPods! 🎉 |
Description
(Fixes #86)
This pull request provides a workaround for the
x5cparameter, which isn't aStringbut an Array of Strings ([String]).The current behavior is to throw an error when serializing.
Please note that this is just a workaround for #86 and does not do any certificate validation (as planned in #57).