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

[portal] [google] GoogleAuthenticationResource userinfo endpoint url needs configuration and userInfo id issue #1323

Closed
yinghe1 opened this Issue Jun 19, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@yinghe1

yinghe1 commented Jun 19, 2018

Expected Behavior

Once user obtained google oauth2 webapp client clientId and clientSecret and setup in gravitee.yml for security:
providers:
- type: google
The portal should be able to use "Sign in with Google" to authenticate user

Current Behavior

Possible Solution

  • Allow GOOGLE_ACCESS_TOKEN_URL and GOOGLE_USER_INFO_URL to be configurable in gravitee.yml
  • Null check userInfo.get("sub") or userInfo.get("id") to newUser.setSourceId (can this be null?)

Steps to Reproduce (for bugs)

  1. Get google oauth2 clientId and clientSecret
  2. Setup in gravitee-management-rest-api gravitee.yml
    security:
    providers:
    • type: google
  3. Try authentication on portal using Sign in with Google

Context

With the mentioned issue fixed, Sign in with Google on the portal is working

Environment

This is 1.17 code

@brasseld

This comment has been minimized.

Member

brasseld commented Jun 19, 2018

IMHO, we should move to v3 endpoints and not allowing to override them from gravitee.yml.

WDYT ?

@yinghe1

This comment has been minimized.

yinghe1 commented Jun 19, 2018

If you change private static final String GOOGLE_USER_INFO_URL = "https://www.googleapis.com/oauth2/v3/userinfo"; that should fix the issue i saw. it will still be good practice to null check sub field.

Allowing override will bring flexibility if google changes things. But I can live with it if it is into backlog.

We care more about the #1324 which is standard OAuth2. If you guys can try and confirm whether you run into the same issue. That will be great. I do have patch for 1324 that is the preferred config solution I suggested. If need to contribute, i will need to go through a company approval process. Or if you have a better solution for 1324, that will be great.

@brasseld

This comment has been minimized.

Member

brasseld commented Jun 19, 2018

Ok I will take care of this one.

IMHO, there is no reason to provide flexibility for this endpoint. And if there is a v4 in the future, we will have to change reference accordingly.

Agree for null check sub field

@yinghe1

This comment has been minimized.

yinghe1 commented Jun 19, 2018

Thank you Brassely

brasseld added a commit to gravitee-io/gravitee-management-rest-api that referenced this issue Jun 21, 2018

@brasseld brasseld changed the title from GoogleAuthenticationResource userinfo endpoint url needs configuration and userInfo id issue to [portal] [google] GoogleAuthenticationResource userinfo endpoint url needs configuration and userInfo id issue Jun 21, 2018

@brasseld brasseld self-assigned this Jun 21, 2018

@brasseld brasseld added the type: bug label Jun 21, 2018

@brasseld brasseld added this to the 1.18.0 milestone Jun 21, 2018

NicolasGeraud added a commit to gravitee-io/gravitee-management-rest-api that referenced this issue Jul 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment