Implement token exchange #59

angelakis · 2020-06-16T15:36:36Z

Update:
We decided to refactor token coop based on the by_grant_type branch. Check #59 (comment) for more info.

Original Message:
This is a pull request to implement token exchange a.k.a. RFC 8693.

The implementation will not be as straightforward as I first hoped it would be, as token exchange is an extension grant type used in the token endpoint.

Currently implemented token and tokencoop endpoints, do not have an easy way to add/configure new grant_types so I think we should refactor one of those two.

TokenCoop was selected to be refactored but we could in the future
just keep the new version instead of having both Token and the new
configurable/extendable TokenCoop.

I think it would be a good idea to use grant_types_supported as a configuration option to figure out what grant types the endpoint should support.

As I previously mentioned, this is still a work in progress and the actual token exchange code has not yet been implemented. RFC 8693 is broad so its implementation will need a discussion too.

Any feedback welcome.

nsklikas · 2020-06-18T11:26:13Z

src/oidcendpoint/oidc/token_coop.py

+            grant_type not in self.token_grant_types
+            or grant_type not in grant_types_supported
+        ):
+            raise ProcessError("Invalid grant type")


Add the grant_type value to the exception?

nsklikas · 2020-06-18T11:26:36Z

src/oidcendpoint/oidc/token_coop.py


        return request

+    def _token_exchange_post_parse_request(self, request, client_id="", **kwargs):


We could override the parse_request method, add a request_cls param to override self.request_cls and provide the proper one depending on the grant_type. This way we can get rid of *_post_parse_request

I'd rather add a get_request_class() method.

nsklikas · 2020-06-18T11:26:58Z

src/oidcendpoint/oidc/token_coop.py

+        elif grant_type == "urn:ietf:params:oauth:grant-type:token-exchange":
+            return self._token_exchange_post_parse_request(request, client_id, **kwargs)
+        else:
+            raise ProcessError("Invalid grant type")


Add the grant_type value to the exception?

nsklikas · 2020-06-18T11:27:10Z

src/oidcendpoint/oidc/token_coop.py

                response_args = self._refresh_access_token(request, **kwargs)
+            elif grant_type == "urn:ietf:params:oauth:grant-type:token-exchange":
+                logger.debug("Token Exchange Request")
+                response_args = self._token_exchange(request, **kwargs)


Move this if/elif/elif to separate function?

rohe · 2020-06-24T09:50:46Z

Have a look at https://github.com/IdentityPython/oidcendpoint/blob/by_grant_type/src/oidcendpoint/oidc/token_coop.py
I'd rather do it that way.
Now, what you see there is just a first iteration. It should of course be configurable.
But you can see the layout.

rohe · 2020-06-25T09:41:34Z

I've even added a test based on an example in the RFC.
It's extremely crude but it works :-)

rohe · 2020-06-25T18:17:40Z

And yes, I think we should get rid of Token and only keep TokenCoop.
Once we've done that we should probably rename TokenCoop to Token.

angelakis · 2020-09-09T15:51:17Z

We decided to use @rohe's by_grant_type branch and work on that to implement a simple Token Exchange functionality.

I believe that our first goal should be to support exchanging access tokens for new access tokens with (the same or) altered scopes.

In that regard I have just pushed the current implementation which seems to work.

There's one simple change concerning by_grant_type and its configuration:

The user can now set the usual grant types to have a default                  
configuration and class, without the need to set the oidcendpoint             
class path. This can be done by setting a value of "default" or               
None (no value in yaml dict).

There are multiple issues right now and the main one is probably how session works, and its dependency on state. We should discuss a way forward concerning that.

Tests are also missing currently.

Any other feedback welcome.

nsklikas · 2020-09-15T11:14:43Z

Can you clean up the commits a little bit? Also it seems that there are extra changes in this PR that are not related to token exchange.

nsklikas · 2020-09-15T11:15:20Z

src/oidcendpoint/client_authn.py

-        _cinfo = endpoint_context.cdb[client_id]
-        if isinstance(_cinfo, str):
-            if not _cinfo in endpoint_context.cdb:
+        if isinstance(_cinfo, str): # This is if _cinfo is a reference to another client_id


Why is this needed?

angelakis · 2020-09-16T08:20:23Z

Pushed multiple tests both for the by-grant-type refactor and token exchange. They try to highlight the functionality we introduce so they will need changes once we decide on the token exchange features (or whether we allow specifying "default" on a grant_type in grant_types_supported).

angelakis · 2020-09-16T08:33:52Z

Can you clean up the commits a little bit? Also it seems that there are extra changes in this PR that are not related to token exchange.

I could remove the isort commits and keep only the parts containing the actual changes in TokenCoop, if that's ok with @rohe. And if we want to do this even better I could split up the grant_types_supported changes and the token exchange ones.

rohe · 2020-09-18T07:30:38Z

That would be OK with me.

angelakis · 2020-09-24T15:43:56Z

I cleaned up unrelated changes and then split up the changes in two different branches, creating #88 with the configurable-grant-types parts. So only token exchange related changes should remain here.

This branch is now based on #88 and should be merged after that.

angelakis · 2021-03-11T13:07:22Z

Accidentally closed this while trying to push the rebased version to the new session handling. Seeing that this PR has changed a lot since its creation, it may be better to create a new one.

angelakis force-pushed the feature-token-exchange branch from 3919d57 to 2f85f83 Compare June 17, 2020 13:19

nsklikas reviewed Jun 18, 2020

View reviewed changes

angelakis force-pushed the feature-token-exchange branch from 2f85f83 to a1f2199 Compare September 9, 2020 15:42

angelakis changed the base branch from master to develop September 10, 2020 10:06

angelakis force-pushed the feature-token-exchange branch 3 times, most recently from 2e4a8d1 to 612dec9 Compare September 10, 2020 15:20

nsklikas reviewed Sep 15, 2020

View reviewed changes

angelakis force-pushed the feature-token-exchange branch from 72c95b4 to aa8e983 Compare September 15, 2020 15:30

angelakis force-pushed the feature-token-exchange branch from aa8e983 to e914eb8 Compare September 17, 2020 11:55

angelakis force-pushed the feature-token-exchange branch 2 times, most recently from cee5f8a to 3ecb084 Compare September 24, 2020 13:53

angelakis mentioned this pull request Sep 24, 2020

Feature configurable grant types #88

Merged

angelakis force-pushed the feature-token-exchange branch from 3ecb084 to f13e22d Compare September 24, 2020 15:41

angelakis changed the base branch from develop to feature-configurable-grant-types September 24, 2020 15:46

angelakis force-pushed the feature-token-exchange branch from f13e22d to 76d1dc6 Compare September 24, 2020 15:53

angelakis force-pushed the feature-configurable-grant-types branch from ffee2a9 to ec006ad Compare November 11, 2020 12:52

angelakis force-pushed the feature-token-exchange branch from 76d1dc6 to a9a78cf Compare March 11, 2021 12:56

angelakis changed the base branch from feature-configurable-grant-types to new_session_handling March 11, 2021 12:58

angelakis closed this Mar 11, 2021

angelakis force-pushed the feature-token-exchange branch from a9a78cf to a4afb20 Compare March 11, 2021 13:00

angelakis mentioned this pull request Mar 18, 2021

Feature token exchange #105

Open


		return request

		def _token_exchange_post_parse_request(self, request, client_id="", **kwargs):

Implement token exchange #59

Implement token exchange #59

Uh oh!

Conversation

angelakis commented Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohe commented Jun 24, 2020

Uh oh!

rohe commented Jun 25, 2020

Uh oh!

rohe commented Jun 25, 2020

Uh oh!

angelakis commented Sep 9, 2020

Uh oh!

nsklikas commented Sep 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angelakis commented Sep 16, 2020

Uh oh!

angelakis commented Sep 16, 2020

Uh oh!

rohe commented Sep 18, 2020

Uh oh!

angelakis commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angelakis commented Mar 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

angelakis commented Jun 16, 2020 •

edited

Loading

angelakis commented Sep 24, 2020 •

edited

Loading