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

require a non-empty key to decode a JWT #60

Merged
merged 3 commits into from
Jul 22, 2015

Conversation

sjones608
Copy link

In testing this library for use on a project, I was surprised when JWT::decode returned a successfully decoded payload even when an empty key was supplied. I mistakenly supplied an empty key b/c I accidentally referred to it in a config file by the wrong name.

I would think that an empty key should be an error. Otherwise, it would be possible to deploy an application using this library and have all tokens be successfully decoded, regardless of what key was used to encode them. As mentioned above, this could happen through accidental mishandling of configuration files.

I could be wrong about this and an empty key might be possible by design. If so, nevermind, and I'll just be more careful. ;-)

I've made the code changes to JWT::decode and added 2 new tests to verify that empty keys generate exceptions.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@sjones608
Copy link
Author

I've signed the CLA

@robertdimarco robertdimarco self-assigned this Jul 16, 2015
robertdimarco added a commit that referenced this pull request Jul 22, 2015
require a non-empty key to decode a JWT
@robertdimarco robertdimarco merged commit 37a6b73 into firebase:master Jul 22, 2015
@jayhaase
Copy link

This code change should have also updated the function header which now incorrectly describes the parameters and thrown exceptions.

@scrobbleme
Copy link

Hi,
I'm not familiar with JWT, but I think from the documentation it reads that verification is a "should".
(I'm not sure, if this is the correct part of the doc as well)
https://tools.ietf.org/html/draft-ietf-oauth-json-web-token-13#section-5.3

@sjones608
Copy link
Author

@scrobbleme That section of the IETF draft is entitled "Replicating Claims as Header Parameters". I read it to refer to situations where some of the claims that are inside an encrypted JWT are also sent as unencrypted headers. The verification of the unencrypted headers against the actual claims inside of the JWT is what "should" be done while verification of the claims in the actual JWT is a "must".

@scrobbleme
Copy link

@sjones608 Thanks for the explanation. Quite hard to understand.

As I'm not familiar with JWT but have to use it, I try to explan, why I thought it might be optional ;)

  • I'm developing using Atlassian Connect. They use JWT, so I have to too.
  • There you have to decode the token before verification, as you need a value from the token to retrieve another value, which is needed for the verification (stupid, isn't it?).
  • Just read at "Exposing a Service"

Beside this I managed to figure out, how I still get the information I need:

$splitted_jwt = explode('.', $_GET['jwt']);
        $header = \Firebase\JWT\JWT::jsonDecode(\Firebase\JWT\JWT::urlsafeB64Decode($splitted_jwt[0]));
        $claims = \Firebase\JWT\JWT::jsonDecode(\Firebase\JWT\JWT::urlsafeB64Decode($splitted_jwt[1]));
        $signature = \Firebase\JWT\JWT::urlsafeB64Decode($splitted_jwt[2]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants