Openid connect support (fixes #939) #1425

leplatrem · 2017-12-17T00:54:05Z

Fixes #939

We could make this piece of code work with both Auth0 and Firefox Accounts \o/

@rfk f?

rfk · 2017-12-20T02:40:13Z

We could make this piece of code work with both Auth0 and Firefox Accounts \o/

FWIW I'm a strong 👍 to this goal :-)

rfk · 2017-12-20T02:41:56Z

kinto/core/authentication.py

+            return None
+
+        try:
+            # Bearer+OIDC id_token=jwt, access_token=bearer


Out of curiosity, is this approach based on some prior art, or your own scheme? I'm not sure how well it will interact with refresh tokens, which would allow you to generate a fresh access_token that's not bound to the id_token.

It is our own scheme. We asked ourselves a lot of question about that.

Looking at the Auth0 documentation is seems that the only way to verify an access_token is to use the id_token. (at_hash value) but it doesn't seems to be the way of doing it with Firefox Account where we would most likely use the /verify endpoint.

My understanding of OAuth was that sending the access_token should be enough but then how do you validate it with Auth0? How does services usually work with id_token and access_token ? Do they send the id_token?

My understanding of OAuth was that sending the access_token should be enough

That's my understanding as well. The id_token is a point-in-time confirmation that we successfully authenticated a particular user, and the access_token is an ongoing grant of authorization to do a particular thing. I don't know much about Auth0, but having to send both on every request doesn't mesh with my expectations of OIDC.

Looking at the Auth0 documentation is seems that the only way to verify an access_token is to use the id_token

Can you please drop a link to this for my reference?

This was it: https://auth0.com/docs/api-auth/tutorials/verify-access-token

But this is what I also found: https://auth0.com/docs/api-auth/intro#calling-apis-with-access-tokens

So maybe the access_token is also a JWT that self validates?

The problem is that if you look at this table: https://auth0.com/docs/api-auth/intro#how-to-use-the-new-flows

You see that tokeninfo is disabled...

Should we rely on userinfo call instead?

leplatrem · 2018-01-24T10:13:04Z

@cfguimaraes in order to try this branch with Auth0, you can use these settings:

multiauth.policies = oidc
multiauth.policy.oidc.use = kinto.core.authentication.OpenIDConnectPolicy
oidc.issuer_url = https://{yours}.auth0.com
oidc.audience = {api-id}

cfguimaraes · 2018-01-24T16:00:24Z

@leplatrem thank you for the mention, I will try this on Codenvy

almet · 2018-01-26T18:15:24Z

This seems great, thanks for working on it! Being able to use Openid connect will be a lot useful, so we can ditch basic auth default mechanism :-)

cfguimaraes · 2018-01-26T19:54:15Z

kinto/core/authentication.py

+            parts = payload.split(',')
+            credentials = {k: v for k, v in [p.strip().split("=") for p in parts]}
+            id_token = credentials["id_token"]
+            access_token = credentials["access_token"]


Are keys ["id_token", "acess_token"] mandatory in JWT from an OIDC provider?
I've created a non-interactive client on Auth0 dashboard, from the APIs gotten the JWT token using curl, however, I always got HTTP 401 Unauthorized from Kinto.

{ "iss": "https://tenant.auth0.com/", "sub": "key@clients", "aud": "https://tenant.auth0.com/api/v2/", "iat": 1516993206, "exp": 1517079606, "azp": "gaJ9QUArBJTPpL76SYV8jDhqu9Cztq93", "gty": "client-credentials" }

The code above is the Payload extracted from JWT.io site

Accordingly to section 2 from Open ID Core the keys required are all right.

My bad, I'm testing all day with a non-interactive client from Auth0, the purpose of that is to grant authorization to scripts, in a server-server communication channel.
Tomorrow I will plug Auth0 lock (login) and update this topic with discovers, I'm really happy with how simples this code are at the same time how great are thanks to OIDC discovery.
Thanks to @leplatrem too for spend resources on this code.

Status update:

First of all, I become testing this in an Ionic project, already configured with Auth0, but when trying to integrate with kinto.js I don't know why, but sadly doesn't work

Download quick start from Auth0's GitHub samples, inspecting the code on this branch I've seen that this is relying on python-jose for authentication, specifically to decode the JWT, also, @leplatrem fixed an issue and make a PR to address an error on at_hashclaim being required, but accordingly to OIDC that is not required at all

I've forked the @leplatrem repo from python-jose and build the branch with her contributions to generate a wheel (whl) and installed it with pip3, however, I'm always getting an error from Authentication.py
Incorrect claims, please check the audience and issuer: **at_hash claim missing from token**.

I think that the local installation of python-jose (whl) is not being reflected on kinto source, can someone give me a help, I don't know what I have to do to make the decode function of python-jose work with enhanced code from @leplatrem in order to advance in these tests.

The good news is that after all, we already have a sample demonstrating how to integrate OIDC compliant like Auth0 to kinto.

cfguimaraes · 2018-02-01T03:16:26Z

kinto/core/authentication.py

+            if access_token is None:
+                options = {"verify_at_hash": False}
+
+            payload = jwt.decode(id_token, rsa_key, algorithms=["RS256"],


I've logged the code and verified that the error at_hash claim missing from token is being raised from this function.
I cloned (https://github.com/leplatrem/python-jose/tree/75-at_hash-optional)[leplatrem python-jose] implementation that is supposed to address the issue in order to test, but I don't know if the changes on code is being reflected on kinto source

Sorry @cfguimaraes for the delay...

Basically you should be able to apply the fork with:

$ cd kinto/ $ source .venv/bin/activate $ pip install -e <path-to-python-jose-fork>

But according to the code above if you provide a JWT but no access token, then this hash is not verified.

The status of this branch is still very rough, I want to land it ASAP, we'll need it in our production stacks at Mozilla :)

@Natim

paired with @Natim

cfguimaraes · 2018-02-03T17:56:03Z

Good news, working today in this feature I could connect with Auth0 OIDC Conformant JWT Token.

However, the code looks for a Bearer+OIDC Authorization Header, I couldn't find a place where this Header is described, only Bearer token.

The code in the branch doesn't seem to work properly, I've needed to fork python-jose implementation of the at_hash issue of leplatrem, after that all works fine.

I will clear the local code, and make new tests to ensure the python-jose code has a bug or not in the current version of Kinto.

In 4 hours I'll make a report.

cfguimaraes · 2018-02-03T19:06:46Z

I've cleaned local code, make a pull and update the code to reflect the locations for the plugin and all work fine, without the code on python-jose.
Sadly I don't have many use cases to test, but Authentication is working fine, with a valid acess_token and Forbidden with an invalid JWT.

If there is something you'll want I do, I will be happy to help @leplatrem.

leplatrem · 2018-02-05T15:08:30Z

Thanks @cfguimaraes for your detailed feedback!

Next step on my side is to write a little single page app demo (with Github auth or something) and update the docs / tutorial...

However, the code looks for a Bearer+OIDC Authorization Header, I couldn't find a place where this Header is described, only Bearer token.

We invented it with @Natim :) Since Kinto can have several authentication policies enabled, with several ones that use bearer tokens, we wanted to have a special one. Maybe we could default it to just Bearer and make it optional in settings to customize it. If you thought that was confusing, it's a sign :)

Notes:
https://developer.github.com/apps/building-oauth-apps/

leplatrem · 2018-02-05T18:25:03Z

I made some changes to this branch. The tests will fail. The audience setting was renamed to client_id.

I made it work with Google Accounts, this is the frontend demo that I wrote:
https://github.com/leplatrem/kinto-oidc-demo/

It's still Work-In-Progress though.

cfguimaraes · 2018-02-05T21:55:20Z

I really think that a Bearer token as proposed is the best as default.
If one wants a different behavior that could configure that, I think that a tool has to be standard compatible, those that are exceptions have to write code for exception, IMO.

Natim · 2018-02-06T09:36:26Z

I really think that a Bearer token as proposed is the best as default.

To be honest I am not quite sure about that postulate. When using the bearer token with Kinto you still have a to build the Authorization header by hand.

If you know that Kinto expect a Bearer+OIDC realm then it doesn't change anything to write Bearer+OIDC rather than Bearer.

Plus this let you configure Kinto with multiple auth providers without messing around with configuration.

leplatrem · 2018-02-06T10:19:36Z

Plus this let you configure Kinto with multiple auth providers without messing around with configuration.

Honestly, having multiple authentication policies configured is not the most common use-case

Natim · 2018-02-06T10:55:35Z

kinto/plugins/openid/__init__.py

+            return None
+
+        try:
+            # Bearer+OIDC id_token=jwt, access_token=bearer


Remove support for ID_Token (which is used by the frontend), on the backend we want to use access/bearer token

The thing is that we can have access tokens in JWT format. I haven't looked at those yet.

Also, using id tokens saves a round-trip to the Identity Provider. With the inconvenient that their validity remains unquestioned until they expire.

cfguimaraes · 2018-02-06T15:19:18Z

@Natim, I like standard things, don't you think that being standard this tool will help much more developers at all?
In OIDC Core, token response section says

The OAuth 2.0 token_type response parameter value MUST be Bearer, as specified in OAuth 2.0 Bearer Token Usage [RFC6750], unless another Token Type has been negotiated with the Client

I think, having a non-standard Header like Bearer+OIDC will force 2 implementations, one for tools that accept content negotiation, and another for standard tools only. In the first case we will force developers to write and manage more code, to accept negotiation with Kinto endpoint, and to access other API's that are standard compatible only sending a Bearer jwt.
I as a developer don't like how this sounds, if I need a non-standard solution just implementing an interface and settings in kinto.ini (configuration file) will make me much happier.

Natim · 2018-02-06T15:22:44Z

unless another Token Type has been negotiated with the Client

That's my point.

I think, having a non-standard Header like Bearer+OIDC will force 2 implementations

I don't think so.

I as a developer don't like how this sounds, if I need a non-standard solution just implementing an interface and settings in kinto.ini (configuration file) will make me much happier.

I agree with you here.

cfguimaraes · 2018-02-06T15:38:36Z

I don't think so.

Could you say more about how you see this?
I still see 2 implementations, in this case, one for Bearer (standard), and another for any non-standard header (as any developer can extend kinto) like Bearer+OIDC as invented for @leplatrem in her code.

I've made a login in an Auth0 OIDC compliant client, sending a compliant acess_token Authorization heared as a Bearer jwt I've been Unauthorized, so I've to send a Kinto compliant Bearer+OIDC token or change the code to expect a Bearer.
I've made the first because I'm writing request in a REST plugin, but when I've made an app to make tests with Angular the tools I used sends standard Bearer token. I don't investigate how to extend the js lib I used to send a custom Header, but I really, don't wish to do that.
And is this I'm saying, if we'll deliver a built-in plugin with Kinto, this SHOULD be OIDC standard compliant, this way we can bring a more convenient tool for developers.
If one needs a different solution than that, this already will have to write custom code, so is acceptable.

What you think about this?

Natim · 2018-02-06T15:41:21Z

Everything is the same (Bearer token, JWT). Only the Realm that has got a suffix when you call Kinto.
You can use the same lib, you can use everything that already exists around OIDC, grab the access_token and then when the clients call Kinto it will use the proper OIDC plugin realm.
That's why I don't think it needs another custom implementation.

I've made the first because I'm writing request in a REST plugin, but when I've made an app to make tests with Angular the tools I used sends standard Bearer token.

Then maybe I am wrong. Can you show me how you use your plugin?

leplatrem · 2018-02-08T17:58:25Z

I reworked the settings part, it now supports multiple OpenID policies, and I updated the demo.
I drafted some docs as well.

At this point I would take as much feedback as possible :)

Natim · 2018-02-08T18:00:03Z

docs/configuration/settings.rst

+.. code-block:: ini
+
+    # User ID field name (Default: `sub`)
+    multiauth.policy.oidc.userid_field = email


Should we replace oidc by google here?

leplatrem · 2018-02-09T16:55:09Z

The code is ready to be thoroughly reviewed/merged.

Also, I would also really appreciate if someone could test this: https://github.com/leplatrem/kinto-oidc-demo

Now the question is: should we do the scope filtering feature in this PR as well as revamp the whole authentication documentation? My take is «no» :)

leplatrem · 2018-02-13T14:12:09Z

@Natim final r?

almet · 2018-02-13T14:50:00Z

Now the question is: should we do the scope filtering feature in this PR as well as revamp the whole authentication documentation? My take is «no» :)

That's the easy way, for sure ! But the risk is that it will stay as it is. I remember that this auth documentation have had to be revamped for some time now and it's still not the case.

My take would be to have documentation before merging this ;) (but, it's easy to say when you're not the one doing it!)

Natim · 2018-02-13T15:02:19Z

I think the authentication documentation already have been revamp to use account rather than the dummy Basic Auth, I don't think it is related to openid anyway but more to the account plugin so another pull-request will be fine.

I think it is good enough as it is to be merge right away, we should create the issue regarding documentation and scopes management once we know how we are supposed to handle it with Auth0.

leplatrem · 2018-02-13T16:37:14Z

I think it is good enough as it is to be merge right away

I'll create the issue once approved and merged :)

Natim · 2018-02-14T08:12:48Z

kinto/plugins/openid/__init__.py

+            return None
+
+        try:
+            # Bearer+OIDC id_token=jwt, access_token=bearer


Is this on purpose? I though we said that we would only send Bearer token and not id_tokens anymore?

Natim · 2018-02-14T08:25:42Z

Since OpenID is about Authentication and Oauth about Authorization, I believe it is fine not validating scopes here and using the userinfo_endpoint to validate bearer tokens.

In the meantime I believe we should remove the custom authentication flow using id_token.

leplatrem · 2018-02-14T09:52:42Z

Since OpenID is about Authentication and Oauth about Authorization, I believe it is fine not validating scopes here and using the userinfo_endpoint to validate bearer tokens.

Our original use case is to obtain different userids depending on scope (to isolate data between apps basically).
What if we just define two clients IDs and define two policies?

    multiauth.policy.notes.issuer = https://accounts.google.com
    multiauth.policy.notes.client_id = YYYYYYYY.apps.googleusercontent.com
    multiauth.policy.notes.client_secret = UAlL-YYYYYYYY

    multiauth.policy.lockbox.issuer = https://accounts.google.com
    multiauth.policy.lockbox.client_id = XXXXXX.apps.googleusercontent.com
    multiauth.policy.lockbox.client_secret = UAlL-XXXXXX

The user ID will be notes:abc on one side, and lockbox:abc on the other side... The IdP could restrict the access to one or another with the callback/redirect URIs settings.

I believe we should remove the custom authentication flow using id_token.

I don't know, using ID tokens saves a round trip to the IdP. But yes, it would make everything simpler indeed. I wish I had an example of flow where we obtain JWT access tokens, but I don't. Yeah, you're right, let's make it super stupid and simple: just access tokens.

Natim · 2018-02-14T10:10:09Z

What if we just define two clients IDs and define two policies?

The issue is that in that case the bearer_token will be validated in both case right?

leplatrem · 2018-02-14T10:28:03Z

The issue is that in that case the bearer_token will be validated in both case right?

If it's for the second policy, yes. Two round trips to the IdP

Natim

Thanks

leplatrem force-pushed the 939-openid-connect branch 2 times, most recently from 0ce4850 to d9db8ba Compare December 17, 2017 00:59

rfk reviewed Dec 20, 2017

View reviewed changes

leplatrem mentioned this pull request Dec 21, 2017

OpenID: fetch infos from user profile if access token is provided mozilla/doorman#80

Closed

3 tasks

leplatrem mentioned this pull request Jan 24, 2018

Add support for Auth0 #1270

Closed

cfguimaraes reviewed Jan 26, 2018

View reviewed changes

cfguimaraes reviewed Feb 1, 2018

View reviewed changes

leplatrem mentioned this pull request Feb 2, 2018

Add OpenID connect support Kinto/kinto-admin#460

Closed

leplatrem added 2 commits February 2, 2018 11:49

Add support for OpenID Connect (fixes #939)

b4acd8f

paired with @Natim

Move to plugins and add tests

ce32ca4

leplatrem force-pushed the 939-openid-connect branch from d9db8ba to ce32ca4 Compare February 2, 2018 15:36

leplatrem added 2 commits February 5, 2018 12:46

Add unit tests

bb2300d

Fix tests coverage

f37fc6a

Add state/tokens views

5242f36

Natim reviewed Feb 6, 2018

View reviewed changes

leplatrem added 4 commits February 8, 2018 11:33

Improve views

63943a6

Improve coverage

94834cd

Fix settings and allow multiple openid policies

7fb2ae6

Add documentation

0e30768

leplatrem added a commit to leplatrem/kinto-oidc-demo that referenced this pull request Feb 8, 2018

Fix demo with latest Kinto/kinto#1425

7d47eba

Natim reviewed Feb 8, 2018

View reviewed changes

leplatrem added 4 commits February 8, 2018 19:00

@Natim review

565fc68

Fix docs

b8c9172

Raise 400 for scope if userid field is email

98f3408

Polish OpenAPI output

2a221c0

leplatrem changed the title ~~[WIP] Openid connect support (fixes #939)~~ Openid connect support (fixes #939) Feb 9, 2018

Update changelog

4c06649

leplatrem mentioned this pull request Feb 13, 2018

Revamp authentication documentation #1494

Closed

Natim reviewed Feb 14, 2018

View reviewed changes

Natim approved these changes Feb 14, 2018

View reviewed changes

Remove support JWT ID Tokens

8b4cc02

Natim approved these changes Feb 14, 2018

View reviewed changes

Merge branch 'master' into 939-openid-connect

9df130e

leplatrem merged commit 3b6556a into master Feb 14, 2018

leplatrem deleted the 939-openid-connect branch February 14, 2018 14:51

Openid connect support (fixes #939) #1425

Openid connect support (fixes #939) #1425

Conversation

leplatrem commented Dec 17, 2017 • edited Loading

rfk commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leplatrem commented Jan 24, 2018

cfguimaraes commented Jan 24, 2018

almet commented Jan 26, 2018

cfguimaraes Jan 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfguimaraes commented Feb 3, 2018

cfguimaraes commented Feb 3, 2018 • edited Loading

leplatrem commented Feb 5, 2018

leplatrem commented Feb 5, 2018

cfguimaraes commented Feb 5, 2018

Natim commented Feb 6, 2018

leplatrem commented Feb 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfguimaraes commented Feb 6, 2018 • edited Loading

Natim commented Feb 6, 2018

cfguimaraes commented Feb 6, 2018

Natim commented Feb 6, 2018 • edited Loading

leplatrem commented Feb 8, 2018

Choose a reason for hiding this comment

leplatrem commented Feb 9, 2018 • edited Loading

leplatrem commented Feb 13, 2018

almet commented Feb 13, 2018

Natim commented Feb 13, 2018

leplatrem commented Feb 13, 2018

Choose a reason for hiding this comment

Natim commented Feb 14, 2018

leplatrem commented Feb 14, 2018

Natim commented Feb 14, 2018

leplatrem commented Feb 14, 2018 • edited Loading

Natim left a comment

Choose a reason for hiding this comment

leplatrem commented Dec 17, 2017 •

edited

Loading

cfguimaraes Jan 26, 2018 •

edited

Loading

cfguimaraes commented Feb 3, 2018 •

edited

Loading

cfguimaraes commented Feb 6, 2018 •

edited

Loading

Natim commented Feb 6, 2018 •

edited

Loading

leplatrem commented Feb 9, 2018 •

edited

Loading

leplatrem commented Feb 14, 2018 •

edited

Loading