Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include misc in JWT.Claims struct. Use InMemoryKeyManager() by default in DIDJWK.create() #45

Merged
merged 8 commits into from
Apr 1, 2024

Conversation

jiyoonie9
Copy link

@jiyoonie9 jiyoonie9 commented Apr 1, 2024

closes #42
closes #40

@jiyoonie9 jiyoonie9 marked this pull request as draft April 1, 2024 02:05
Copy link
Collaborator

@kirahsapong kirahsapong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!! prob just need to pull in FlatMap and then will be g2g!

@@ -44,6 +44,10 @@ public struct JWT {
/// [Spec](https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.7)
let jwtID: String?

/// "misc" Miscellaneous claim is a map to store any additional claims
///
let miscellaneous: [String: AnyCodable]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! We can use FlatMap for this: https://github.com/TBD54566975/web5-swift/blob/main/Sources/Web5/Common/FlatMap.swift

We can rename wrappedValue to misc since that sounds better anyway

Copy link
Author

@jiyoonie9 jiyoonie9 Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i think wrappedValue is required by swift! just tried changing the name and it yelled at me ._.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try this!

    enum CodingKeys: String, CodingKey {
        case wrappedValue = "misc"
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can also rename the whole file Misc 🤔 I dont think FlatMap actually ended up being used anywhere!

// MARK: - JWT Claims Encoding

extension JWT.Claims {
public func encode(to encoder: Encoder) throws {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we add FlatMap we wont need these

@@ -30,7 +30,7 @@ public enum DIDJWK {
/// - options: Options configuring how the DIDJWK is created
/// - Returns: `BearerDID` that represents the created DIDJWK
public static func create(
keyManager: KeyManager,
keyManager: KeyManager = InMemoryKeyManager(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🚀 🚀

@jiyoonie9 jiyoonie9 changed the title jwt encoder and decoder [wip] jwt encoder and decoder Apr 1, 2024
@@ -26,33 +26,38 @@ public struct JWT {
/// or after which the JWT must not be accepted for processing.
///
/// [Spec](https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4)
@ISO8601Date private(set) var expiration: Date?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are date fields in jwt claims supposed to be a numeric value? is there any value in saving it as a date?

Copy link
Author

@jiyoonie9 jiyoonie9 Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue created: #46

@jiyoonie9 jiyoonie9 changed the title [wip] jwt encoder and decoder Include misc in JWT.Claims struct. Use InMemoryKeyManager() by default in DIDJWK.create() Apr 1, 2024
@@ -141,7 +141,7 @@ public enum PresentationExchange {
) throws -> [InputDescriptorV2: [String]] {
let vcJWTListMap: [VCDataModel] = try vcJWTList.map { vcJWT in
let parsedJWT = try JWT.parse(jwtString: vcJWT)
guard let vcJSON = parsedJWT.payload["vc"]?.value as? [String: Any] else {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#47

@jiyoonie9 jiyoonie9 marked this pull request as ready for review April 1, 2024 18:39
Copy link
Collaborator

@kirahsapong kirahsapong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amahzin

Sources/Web5/Crypto/JOSE/JWT.swift Outdated Show resolved Hide resolved
try container.encodeIfPresent(issuedAt, forKey: .issuedAt)
try container.encodeIfPresent(jwtID, forKey: .jwtID)

// Dynamically encode the miscellaneous properties at the top level

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the top level

So this means the claims body won't look like this, right?

{
  // existing well-defined claims properties
  "misc": {
    "something": "else"
  }
}

and instead will look like this, right?

{
  // existing well-defined claims properties
  "something": "else"
}

(idk Swift so apologies in advance)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thats right! in swift we don't get the affordance of [k:string: any] type like in ts, so we effectively encode/decode to/from a known property that does have a dictionary of [String: Any]

so exactly as you said:

{
  // known property with codingKey "issuer"
  "iss": "abcCorp",
  // dynamic properties with unknown codingKeys
  "something": "else",
  "and": "something",
  "else": "again"
}

while in Swift it will look like

// maps to known codingKeys
"issuer":  "abcCorp",
"miscellaneous": { 
  "something": "else", 
  "and": "something", 
  "else", "again" 
}

Copy link

@KendallWeihe KendallWeihe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk swift but nothing stands out to me as glaringly wrong

added one comment with a question around how encoding works

Jiyoon Koo and others added 2 commits April 1, 2024 14:59
notBefore: Date? = nil,
issuedAt: Date? = nil,
jwtID: String? = nil
expiration: Int? = nil,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make these Date and do the conversion for consumers

@@ -61,9 +66,11 @@ public struct JWT {
self.notBefore = notBefore
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...
            self.expiration = expiration != nil ? Int(expiration!.timeIntervalSince1970) : nil
            self.notBefore = notBefore != nil ? Int(notBefore!.timeIntervalSince1970) : nil
            self.issuedAt = issuedAt != nil ? Int(issuedAt!.timeIntervalSince1970) : nil
...

@kirahsapong
Copy link
Collaborator

cc: @EbonyLouis @acekyd

@kirahsapong kirahsapong merged commit 5208605 into main Apr 1, 2024
6 checks passed
@kirahsapong kirahsapong deleted the 40-add-nonce branch April 1, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keyManager is required when creating new DID Add nonce field in JWT.Claims()
3 participants