Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

Use "https://accounts.google.com" as issuer to validate JWT tokens#17

Merged
tangiel merged 3 commits intocloudendpoints:masterfrom
AODocs:jwt_issuer_fix
Oct 14, 2016
Merged

Use "https://accounts.google.com" as issuer to validate JWT tokens#17
tangiel merged 3 commits intocloudendpoints:masterfrom
AODocs:jwt_issuer_fix

Conversation

@clementdenis
Copy link
Contributor

All the JWT tokens issued by Google currently have https://accounts.google.com as their issuer.
Below is an example of a JWT token generated today using the OAuth2 playground.

{
  "iss": "https://accounts.google.com",
  "at_hash": "***",
  "aud": "***",
  "sub": "***",
  "email_verified": true,
  "azp": "***",
  "hd": "***",
  "email": "***",
  "iat": 1476203989,
  "exp": 1476207589
}

It seems like the issuer has been changing back and forth from "accounts.google.com" to "https://accounts.google.com" recently (see my comment on this post)

This change switches back to https://accounts.google.com as the issuer, it fixes the endpoints v2 backend sample.

@codecov-io
Copy link

codecov-io commented Oct 11, 2016

Current coverage is 80.44% (diff: 75.00%)

Merging #17 into master will decrease coverage by <.01%

@@             master        #17   diff @@
==========================================
  Files           155        155          
  Lines          5009       5012     +3   
  Methods           0          0          
  Messages          0          0          
  Branches        842        842          
==========================================
+ Hits           4030       4032     +2   
  Misses          716        716          
- Partials        263        264     +1   

Powered by Codecov. Last update 9ca0a94...9363f04

@lookuptable
Copy link

I believe JWTs with "accounts.google.com" are still being issued and this change will break them.

Possible solutions would be:

  1. Add both "accounts.google.com" and "https://accounts.google.com" as default issuers;
  2. No default issuers at all. Users need to explicitly annotate their code to allow Google ID token (they can list both URLs as issuers if they choose to do so).

Do you have a preference? @tangiel

@clementdenis
Copy link
Contributor Author

Indeed, according to the documentation, both issuers can be used:
https://developers.google.com/identity/protocols/OpenIDConnect#obtainuserinfo

I submitted a new change that should support both issuers, but I'm not sure it would be enough.

Copy link

@lookuptable lookuptable left a comment

Choose a reason for hiding this comment

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

thank you for contributing. Just one minor comment.

Constant.GOOGLE_ID_TOKEN_NAME, "accounts.google.com",
"https://www.googleapis.com/oauth2/v1/certs");
public static final IssuerConfig GOOGLE_ID_TOKEN_ISSUER_ALT = new IssuerConfig(
Constant.GOOGLE_ID_TOKEN_NAME + "_alt", "https://accounts.google.com",

Choose a reason for hiding this comment

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

Can we define another constant in Constant.java instead of doing string concatenation everywhere?

Also I'd prefer to suffixing it with "_https" instead of "_alt" to be more explicit.

@tangiel tangiel merged commit e305cce into cloudendpoints:master Oct 14, 2016
@clementdenis clementdenis deleted the jwt_issuer_fix branch July 31, 2018 12:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants