uc-cdis / fence

AuthN/AuthZ OIDC Service
Apache License 2.0
37 stars 48 forks source link

Support `email` scope in authentication flow #598

Open VJalili opened 5 years ago

VJalili commented 5 years ago

When I set scopes in my authentication request to ['openid', 'user'], I get an ID token whose context decodes as:

  "context": {
    "user": {
      "phone_number": null,
      "display_name": null,
      "name": "xyz@gmail.com",
      "is_admin": false,
      "policies": [],
      "email": null,
      "projects": {}
    }
  },

where name filed contains the email address, and the email field is null.

I changed the scope to ['openid', 'user', 'email'], but then I get the Unauthorized exception:

https://github.com/uc-cdis/fence/blob/31744be49ec0a56492d6a87a110c5af699f87189/fence/auth.py#L98-L104

because email is not a currently supported scope:

https://github.com/uc-cdis/fence/blob/8337488620fad4ea32160d537e73c825cd93d18d/fence/jwt/token.py#L50-L56

I was wondering if can add a support for the email scope so a future context would decode as:

  "context": {
    "user": {
      "phone_number": null,
      "display_name": null,
      "name": "xyz",
      "is_admin": false,
      "policies": [],
      "email": "xyz@gmail.com",
      "projects": {}
    }
  },
Avantol13 commented 5 years ago

Hey @VJalili ! thanks for the input, we actually don't support email as a separate field, we bundle all the user info into the context field and provide that under the user scope. The fact that you're not seeing an email in the email field has to do with how we create users for different identity providers. I'm assuming you have Google configured as the IDP, and when we create users from Google profiles, we use their email as their username. It's definitely a valid suggestion for us to also put that in the email field for Google as well, I will create a ticket on our end to support this

Avantol13 commented 5 years ago

we could additionally support the email field as the OIDC spec defines, but then the email field would end up outside the context block, per the spec. Furher conformance to the optional features in OIDC are something we are continuing to pursue as well