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

Adding optional headers to send in JWT #53

Merged
merged 6 commits into from
Jun 22, 2015

Conversation

mcocaro
Copy link

@mcocaro mcocaro commented Jun 17, 2015

Some services require additional info to be sent in the JOSE header, adding the option for encoding.

@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.

@mcocaro
Copy link
Author

mcocaro commented Jun 17, 2015

I signed it!

Martin

On Wed, Jun 17, 2015 at 4:18 PM, googlebot [email protected] wrote:

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).

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

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

verify. Thanks.


Reply to this email directly or view it on GitHub
#53 (comment).

@tuupola
Copy link

tuupola commented Jun 18, 2015

Do you have any example how and when this is used?

@mcocaro
Copy link
Author

mcocaro commented Jun 18, 2015

yea - check this imp out:

https://github.com/layerhq/support/blob/master/identity-services-samples/php/jwt.php

they require the addition of: 'cty' => 'layer-eit;v=1' as a header

Martin

On Thu, Jun 18, 2015 at 1:35 AM, Mika Tuupola [email protected]
wrote:

Do you have any example how and when this is used?


Reply to this email directly or view it on GitHub
#53 (comment).

{
$header = array('typ' => 'JWT', 'alg' => $alg);
if ($keyId !== null) {
$header['kid'] = $keyId;
}
if ( isset($head) && is_array($head) ) {
$header = array_merge($header, $head);

Choose a reason for hiding this comment

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

If $head also contains one of the required fields (typ, alg, or - potentially - kid), this approach would overwrite it, correct? This strikes me as undesirable, but I'm open to other opinions on the matter.

Copy link
Author

Choose a reason for hiding this comment

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

it does - thought about that and leaned towards allowing it but to be safe
we can do it the other way around to ensure the other params are not
overwritten, will send updated pr shortly

Martin

On Thu, Jun 18, 2015 at 11:22 AM, Rob DiMarco [email protected]
wrote:

In Authentication/JWT.php
#53 (comment):

 {
     $header = array('typ' => 'JWT', 'alg' => $alg);
     if ($keyId !== null) {
         $header['kid'] = $keyId;
     }
  •    if ( isset($head) && is_array($head) ) {
    
  •        $header = array_merge($header, $head);
    

If $head also contains one of the required fields (typ,alg, or potentiallykid`),
this approach would overwrite it, correct? This strikes me as undesirable,
but I'm open to other opinions on the matter.


Reply to this email directly or view it on GitHub
https://github.com/firebase/php-jwt/pull/53/files#r32761604.

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

robertdimarco added a commit that referenced this pull request Jun 22, 2015
Adding optional headers to send in JWT
@robertdimarco robertdimarco merged commit dcaa08e into firebase:master Jun 22, 2015
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.

4 participants