Refactor request token to be a JWT #125

jiyoontbd · 2024-01-08T18:43:59Z

closes #121

codecov · 2024-01-08T18:47:24Z

Codecov Report

Merging #125 (0de27d2) into main (fe212cb) will decrease coverage by 1.71%.
The diff coverage is 68.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
- Coverage   78.39%   76.68%   -1.71%     
==========================================
  Files          30       32       +2     
  Lines         671      725      +54     
  Branches       65       71       +6     
==========================================
+ Hits          526      556      +30     
- Misses        111      129      +18     
- Partials       34       40       +6

Components	Coverage Δ
protocol	`85.53% <ø> (ø)`
httpclient	`77.40% <72.41%> (-5.34%)`	⬇️

jiyoontbd · 2024-01-11T18:53:32Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

+ val jwt: SignedJWT
+ try {
+ jwt = SignedJWT.parse(token)
+ // todo: resolving header.kid against a didresolver


put these todos in a new issue: #128

mistermoe · 2024-01-11T20:15:24Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/ExceptionDeclarations.kt

@jiyoontbd thoughts on doing the same as this here?

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt

mistermoe · 2024-01-11T20:23:31Z

httpclient/build.gradle.kts

@@ -28,6 +28,7 @@ dependencies {
 implementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.9.8")
 implementation("com.fasterxml.jackson.core:jackson-databind:2.15.2")
 implementation("decentralized-identity:did-common-java:1.9.0") // would like to grab this via web5 dids
+ implementation("com.github.f4b6a3:uuid-creator:5.3.3")


can we use import java.util.UUID or naw?

i believe java uuid does not natively support uuidv7 - but we can resort to just regular java uuid for now and punt on whether we want to require uuidv7 for all token generation. i will remove this dep and use java uuid for now

…e review comments

…st-token-jwt

diehuxx · 2024-01-13T05:24:30Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

+ * @param pfiDid DID of the PFI
+ * @return DID of the requester/JWT token issuer
+ */
+fun verifyRequestToken(token: String, pfiDid: String): String {


We should use this in http-server in protected endpoints.

httpclient/src/main/kotlin/tbdex/sdk/httpclient/ExceptionDeclarations.kt

phoebe-lew · 2024-01-15T02:19:26Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

I would separate out the request token related methods into a RequestTokenUtils file

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

phoebe-lew · 2024-01-15T02:27:43Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

+ }
+ }
+
+ if (expirationTime.before(Date.from(Instant.now()))) {


Could also convert to require if we invert, i.e

require(Instant.now().isBefore(expirationTime.toInstant()))

phoebe-lew · 2024-01-15T02:31:41Z

httpclient/src/main/kotlin/tbdex/sdk/httpclient/Utils.kt

+ val expirationTime = claimsSet.expirationTime
+
+ val requiredKeys = listOf("aud", "iss", "exp", "jti", "iat")
+ requiredKeys.forEach { key ->


Since this is checking every key, we can use all. i.e

require(requiredKeys.all { key -> claimsSet.claims.containsKey(key) }) { throw RequestTokenMissingClaimsException("Missing required claims.") }

or similar. If we want to log out which specific claim is missing, can keep as forEach and require inside the loop

oh yea. in js, we do log out specifically which claim is missing. i adjusted the exception message to log which one's missing.

phoebe-lew · 2024-01-15T02:35:52Z

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt

+ val token = generateRequestToken(did, pfiDid)
+ val claimsSet = SignedJWT.parse(token).jwtClaimsSet
+
+ val requiredKeys = listOf("aud", "iss", "exp", "jti", "iat")


this list is duplicated between test and impl, should extract out to a const in the utils file

phoebe-lew · 2024-01-15T02:37:10Z

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt

+ requiredKeys.forEach {
+ assertTrue(claimsSet.claims.containsKey(it))
+ }


Using assertk, can do:

assertThat(claimsSet.claims.keys).containsExactlyInAnyOrder(requiredKeys)

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt

…rations.kt Co-authored-by: phoebe-lew <plew@squareup.com>

Co-authored-by: phoebe-lew <plew@squareup.com>

…to httpserver getexchange handler with a todo

jiyoontbd · 2024-01-16T21:41:01Z

httpserver/src/main/kotlin/tbdex/sdk/httpserver/handlers/GetExchanges.kt

- // verifying JWT token: https://github.com/TBD54566975/tbdex/issues/210
-
+ val token = arr[1]
+ // TODO: how to access pfiDid here?


@phoebe-lew so i want to actually call RequestToken.verify() in this protected endpoint, but not sure how to access pfiDid from here?

punting it to a separate issue #135

mistermoe

woooo

… for this

…ble by getExchanges()

using jwt to create bearer token

58e5654

jiyoontbd added 2 commits January 10, 2024 12:28

adding verifyrequesttoken() method with some todos

30567b6

adding a test

61849ce

jiyoontbd commented Jan 11, 2024

View reviewed changes

jiyoontbd marked this pull request as ready for review January 11, 2024 18:53

jiyoontbd requested review from mistermoe, phoebe-lew, michaelneale, KendallWeihe, ethan-tbd and tomdaffurn as code owners January 11, 2024 18:53

adding ktdoc

b6c266a

jiyoontbd linked an issue Jan 11, 2024 that may be closed by this pull request

[http-api] request tokens should be JWTs TBD54566975/tbdex#210

Closed

3 tasks

mistermoe reviewed Jan 11, 2024

View reviewed changes

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt Outdated Show resolved Hide resolved

mistermoe reviewed Jan 11, 2024

View reviewed changes

httpclient/src/test/kotlin/tbdex/sdk/httpclient/UtilsTest.kt Outdated Show resolved Hide resolved

mistermoe reviewed Jan 11, 2024

View reviewed changes

jiyoontbd added 3 commits January 12, 2024 16:18

adding tests, aligning exception naming with tbdex-js, addressing cod…

74104ad

…e review comments

Merge branch 'main' of github.com:TBD54566975/tbdex-kt into 121-reque…

bacf4ef

…st-token-jwt

adding ktdocs for exception constructors

e0cc85b

jiyoontbd requested a review from mistermoe January 12, 2024 22:11

diehuxx reviewed Jan 13, 2024

View reviewed changes

phoebe-lew approved these changes Jan 15, 2024

View reviewed changes

jiyoontbd and others added 4 commits January 16, 2024 15:40

Update httpclient/src/main/kotlin/tbdex/sdk/httpclient/ExceptionDecla…

8ea4ada

…rations.kt Co-authored-by: phoebe-lew <plew@squareup.com>

Apply suggestions from code review

4aadd18

Co-authored-by: phoebe-lew <plew@squareup.com>

pulled out request token to its own object. added verifyToken() call …

e9246c5

…to httpserver getexchange handler with a todo

changing method name

d791c33

jiyoontbd commented Jan 16, 2024

View reviewed changes

mistermoe approved these changes Jan 16, 2024

View reviewed changes

jiyoontbd added 4 commits January 16, 2024 18:09

commenting out use of requesttoken.verify() for now, created an issue…

1dda322

… for this

fixing tbdexhttpclient calling requesttoken methods. added a test

cb99777

attempting to add pfiDid to tbdexhttpserverconfig and have it accessi…

6882e20

…ble by getExchanges()

pfiDid is not nullable

0de27d2

jiyoontbd linked an issue Jan 17, 2024 that may be closed by this pull request

Use RequestToken.verify() in http-server #135

Closed

jiyoontbd merged commit 6a83857 into main Jan 18, 2024
1 of 5 checks passed

jiyoontbd deleted the 121-request-token-jwt branch January 18, 2024 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor request token to be a JWT #125

Refactor request token to be a JWT #125

jiyoontbd commented Jan 8, 2024 •

edited by phoebe-lew

Loading

codecov bot commented Jan 8, 2024 •

edited

Loading

jiyoontbd Jan 11, 2024

mistermoe Jan 11, 2024

mistermoe Jan 11, 2024

jiyoontbd Jan 12, 2024

diehuxx Jan 13, 2024

phoebe-lew Jan 15, 2024

jiyoontbd Jan 16, 2024

phoebe-lew Jan 15, 2024

phoebe-lew Jan 15, 2024

jiyoontbd Jan 16, 2024

phoebe-lew Jan 15, 2024

phoebe-lew Jan 15, 2024

jiyoontbd Jan 16, 2024

jiyoontbd Jan 16, 2024

mistermoe left a comment

Refactor request token to be a JWT #125

Refactor request token to be a JWT #125

Conversation

jiyoontbd commented Jan 8, 2024 • edited by phoebe-lew Loading

codecov bot commented Jan 8, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistermoe left a comment

Choose a reason for hiding this comment

jiyoontbd commented Jan 8, 2024 •

edited by phoebe-lew

Loading

codecov bot commented Jan 8, 2024 •

edited

Loading