Surface rawToken for JWT wrapper #757

salrashid123 · 2019-08-03T18:40:01Z

Adds ability to recall the raw JWT encoded token from JsonWebSignature and JsonWebToken.

The current .toString() method for both classes returns a JSON object. Its useful to have access to the raw token directly to be used in an http client (as Authorization: Bearer <rawToken>, for example)

ref:
googleapis/google-auth-library-java#303 (review)

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebSignature.java

google-http-client/src/test/java/com/google/api/client/json/webtoken/JsonWebSignatureTest.java

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebToken.java

elharo · 2019-08-05T09:56:54Z

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebToken.java

@@ -361,6 +362,12 @@ public String toString() {
    return Objects.toStringHelper(this).add("header", header).add("payload", payload).toString();
  }

+  public String getRawToken() {


This method needs JavaDoc. Also note that the word "raw" does not appear in the RFC. Is there a term in the RFC that does describe this?

cloest term maybe getEncodedToken()

elharo

Test failing:

[ERROR] testRawToken(com.google.api.client.json.webtoken.JsonWebSignatureTest) Time elapsed: 0.008 s <<< ERROR!
java.lang.IllegalArgumentException: no JSON input found
at com.google.api.client.json.webtoken.JsonWebSignatureTest.testRawToken(JsonWebSignatureTest.java:52)

elharo · 2019-08-05T10:11:59Z

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebToken.java

@@ -361,6 +362,12 @@ public String toString() {
    return Objects.toStringHelper(this).add("header", header).add("payload", payload).toString();
  }

+  public String getRawToken() {


Reading the spec, I think what this is, is a "compact serialized form". Furthermore, I'm beginning to think it belongs in the JsonWebSignature class rather than JsonWebToken, because it isn't defined for JsonWebToken.

yeah, thats kind of what i meant that i don't know where i'd use the internal method directly (i.,e JsonWebToken.getRawToken(). i'lonly make this visible to JsonWebToken class

elharo · 2019-08-05T14:16:27Z

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebSignature.java

@@ -482,6 +482,11 @@ private static X509TrustManager getDefaultX509TrustManager() {
    return signedContentBytes;
  }

+  /** Returns the URL safe Base64 encoded JWS token. */
+  public String getRawToken() {


Since we're unlikely to get the API perfect on the first try, we should mark this @beta to warn users it might change.

elharo · 2019-08-05T14:16:35Z

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebToken.java

@@ -361,6 +362,12 @@ public String toString() {
    return Objects.toStringHelper(this).add("header", header).add("payload", payload).toString();
  }

+  public String getRawToken() {


Since we're unlikely to get the API perfect on the first try, we should mark this @beta to warn users it might change.

elharo · 2019-08-05T14:17:16Z

google-http-client/src/main/java/com/google/api/client/json/webtoken/JsonWebSignature.java

@@ -482,6 +482,11 @@ private static X509TrustManager getDefaultX509TrustManager() {
    return signedContentBytes;
  }

+  /** Returns the URL safe Base64 encoded JWS token. */
+  public String getRawToken() {


@Override

salrashid123 · 2019-08-05T15:14:20Z

ok, this became a bit more complicated. The classes are structured such that the original tokenString in JsonWebSignature parse(String tokenString) gets marshalled into a json object which itself sorts the claims. the reverse mechanism i use to unmarshall creates a different string representaton (i.,e the order is different).

so a snippet like this

String rawToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6Ikp"
+ "vaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
JsonWebSignature signature = JsonWebSignature.parse(JacksonFactory.getDefaultInstance(), rawToken);
System.out.println(signature.getEncodedToken());
System.out.println(rawToken);

gives

{
  "alg": "HS256",
  "typ": "JWT"
}.
{
  "iat": 1516239022,
  "sub": "1234567890",
  "name": "John Doe"
}

vs

{
  "alg": "HS256",
  "typ": "JWT"
}.
{
  "sub": "1234567890",
  "name": "John Doe",
  "iat": 1516239022
}

elharo · 2019-08-05T15:18:19Z

Per the JSON specification "An object is an unordered set of name/value pairs". Nothing here, tests included, should depend on this order.

salrashid123 · 2019-08-05T16:29:01Z

The ordering thing doesn't matter for JSON but in this case i'm 'attaching' a singature part of some claims that were in a specifc order to another..the net result is the JWT string representation that the output of these methods isn't valid anymore.

For example, if i take a valid rawToken that i know works on a serivce like Cloud Run

String rawToken = "a.b.c";
JsonWebSignature signature = JsonWebSignature.parse(JacksonFactory.getDefaultInstance(), rawToken);
System.out.println(signature.getEncodedToken());

the output of signatuere.getEncodedToken() isn't the same jwt and doens't work anymore against Cloud Run. Basically, i probably should figure out how to 'just save and return' the raw token thats passed into the .parse() object w/o marshalling/unmarshalling....

elharo · 2019-08-05T16:33:02Z

The signature should be independent of order as well. If it isn't, then there's a bug somewhere, maybe more than one. Are you saying that

{
  "iat": 1516239022,
  "sub": "1234567890",
  "name": "John Doe"
}

produces a different signature than

{
  "sub": "1234567890",
  "name": "John Doe"
  "iat": 1516239022,
}

salrashid123 · 2019-08-05T18:34:53Z

the order does matter...to sign, basically i'll take

{
  "iat": 1516239022,
  "sub": "1234567890",
  "name": "John Doe"
}

use it as string, covert to base64 then digitally sign that to get the signature

which isn't going to produce the same out put if i alter the claims around.

elharo · 2019-08-05T19:18:04Z

I need to check the spec, but if that's really how the JWT spec defines signing, then the spec is broken. The usual way of handling this problem is defininf a canonical and reprodubile ordering; e.g. like canonical XML does for attributes.

elharo · 2019-08-05T19:20:46Z

Also, I think we might be jumping too fast to code here. This is more complex than I realized and I'm not sure we understand the problem sufficiently well. It's probabaly worth starting with a design doc that lays out the plansd, tradeoffs, and issues involed.

elharo · 2019-08-06T17:14:52Z

So I've confirmed that the spec is indeed broken. You will need to figure out how to 'just save and return' the raw token that's passed into the .parse() object w/o marshalling/unmarshalling....

salrashid123 · 2019-08-06T17:43:55Z

yep...thats what i figured (i initially started this thinking the 'easiest' way was to 'just save and retun' the token thats passed in thorugh this

public JsonWebSignature parse(String tokenString)

...but then realized there is also a constructor that a constructor which immediately messes up the ordering

public JsonWebSignature(
      Header header, Payload payload, byte[] signatureBytes, byte[] signedContentBytes) {

what that means is while i could use the token passed into parse() anyone using the direct constructor won't be able to get .getEncodedToken() properly.

I could just throw an exception if someone tries to use that method without going through static .parse()....but that doens't seem all too clean...

elharo · 2019-08-06T17:54:36Z

Can you remove that constructor or make it private?

salrashid123 · 2019-08-06T18:21:27Z

i don't think i can remove or make it not visible (it'd break anyone using it (including our tests):
https://github.com/googleapis/google-http-java-client/blob/master/google-http-client/src/main/java/com/google/api/client/testing/json/webtoken/TestCertificates.java#L312

i could create second constructor pass through the string format (one that'd set the raw token and then call the existing constructor.

public JsonWebSignature(
      Header header, Payload payload, byte[] signatureBytes, byte[] signedContentBytes, String encodedToken)

at this point, i kinda don't know what to do since there doesn't seem to be a good way out of this

elharo · 2019-08-06T18:37:43Z

You could deprecate this constructor. Might be worth doing that if it's definitely broken like this.

elharo · 2019-08-06T18:44:33Z

I think I see a way to fix this. We need to canonicalize field order before signing, and do all our own signing. It will need some new runtime exceptions, but should be plausible. I do wonder if it might be easier to build a better library. This class has other problems too, such as exposing mutable internal state.

elharo · 2019-08-06T20:39:15Z

@BenWhitehead suggested we look at https://github.com/auth0/java-jwt to see if it might work better than rolling our own.

salrashid123 · 2019-08-06T23:18:06Z

ugh...this is a bit of a mess..I'm not sure if canonicalizing before signing is even an option anymore...
the existing surface

public JsonWebSignature(
      Header header, Payload payload, 
      byte[] signatureBytes, 
      byte[] signedContentBytes,

precludes us from doing anything since the Payload is already provided in whatever alpha(?) order it uses...and the signed portion is separate (meaning it was signed already with whatever propr string format it was...).

This means JWT Header, Payload objects used in the code is disconnected from the signature that was itself passed

better way maybe to just do a rewrite after annotating as deprecated and use the auth0 stuff

elharo · 2019-08-07T00:26:41Z

I think it's legitimate to have the public JsonWebSignature(Header header, Payload payload, byte[] signatureBytes, byte[] signedContentBytes) throw a runtime exception if the signatureBytes and signedContentBytes don't match the header and payload. It's API compatible, and a runtime exception works because this is an illegal argument problem, i.e. a programming error on the part of the caller.

If we use auth0 perhaps that could be an internal implementation detail?

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Emily Ball <emilyball@google.com>

surface rawToken for JWT wrapper

a27c1b5

salrashid123 requested a review from a team as a code owner August 3, 2019 18:40

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 3, 2019

salrashid123 mentioned this pull request Aug 3, 2019

feat: add IDTokenCredential support googleapis/google-auth-library-java#303

Merged

elharo reviewed Aug 4, 2019

View reviewed changes

fix formatting

2833b4b

elharo added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2019

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2019

elharo reviewed Aug 5, 2019

View reviewed changes

elharo suggested changes Aug 5, 2019

View reviewed changes

elharo reviewed Aug 5, 2019

View reviewed changes

elharo suggested changes Aug 5, 2019

View reviewed changes

salrashid123 mentioned this pull request Aug 14, 2019

chore: hide IdToken#getJsonWebSignature() googleapis/google-auth-library-java#329

Merged

eaball35 closed this Jan 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Surface rawToken for JWT wrapper #757

Surface rawToken for JWT wrapper #757

salrashid123 commented Aug 3, 2019

elharo Aug 5, 2019

salrashid123 Aug 5, 2019

elharo left a comment

elharo Aug 5, 2019

salrashid123 Aug 5, 2019

elharo Aug 5, 2019

elharo Aug 5, 2019

elharo Aug 5, 2019

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019 •

edited

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019

elharo commented Aug 5, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 6, 2019

elharo commented Aug 6, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 7, 2019

Surface rawToken for JWT wrapper #757

Surface rawToken for JWT wrapper #757

Conversation

salrashid123 commented Aug 3, 2019

elharo Aug 5, 2019

Choose a reason for hiding this comment

salrashid123 Aug 5, 2019

Choose a reason for hiding this comment

elharo left a comment

Choose a reason for hiding this comment

elharo Aug 5, 2019

Choose a reason for hiding this comment

salrashid123 Aug 5, 2019

Choose a reason for hiding this comment

elharo Aug 5, 2019

Choose a reason for hiding this comment

elharo Aug 5, 2019

Choose a reason for hiding this comment

elharo Aug 5, 2019

Choose a reason for hiding this comment

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019 • edited

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019

salrashid123 commented Aug 5, 2019

elharo commented Aug 5, 2019

elharo commented Aug 5, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 6, 2019

elharo commented Aug 6, 2019

elharo commented Aug 6, 2019

salrashid123 commented Aug 6, 2019

elharo commented Aug 7, 2019

elharo commented Aug 5, 2019 •

edited