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

Conversation

@angelakis
Copy link
Contributor

As discussed in #59 (comment), I split up the configurable grant types feature (originally by-grant-types) from the token exchange specific changes to make review easier.

This branch refactors token_coop module to allow configuring different handlers for each grant type configured.

@rohe
Copy link
Contributor

rohe commented Oct 5, 2020

EndpointHelper is not a general class for all endpoints so I think we should name it TokenEndpointHelper .

@angelakis
Copy link
Contributor Author

angelakis commented Oct 7, 2020

EndpointHelper is not a general class for all endpoints so I think we should name it TokenEndpointHelper .

Correct, I changed it.

@angelakis
Copy link
Contributor Author

Should I also change token_coop module and class name to Token and replace the existing one? Or leave it for the future?

@rohe
Copy link
Contributor

rohe commented Oct 7, 2020

I think you should.

@angelakis
Copy link
Contributor Author

I think you should.

Previously the token endpoint was named AccessToken instead of Token, which is the correct name now. I guess we should try to keep AccessToken (maybe as an alias) for backwards compatibility?

@rohe
Copy link
Contributor

rohe commented Oct 8, 2020

I don't know. It is the token endpoint not the access token endpoint.
But sure keep AccessToken as an alias.

Copy link
Member

@peppelinux peppelinux left a comment

Choose a reason for hiding this comment

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

LGTM
@angelakis is it time to merge it?

@angelakis
Copy link
Contributor Author

angelakis commented Nov 4, 2020

LGTM
@angelakis is it time to merge it?

I wanted to rename to Token and create an alias for AccessToken but haven't found the time yet.

We can merge once I make the changes @nsklikas pointed today, the renaming can be done on a separate pr.

rohe and others added 7 commits November 11, 2020 13:20
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).
Removed "default" value for grant types and added True instead.

So now None or True enables the grant_type with the default class,
while False disables it and of course a dict with a custom class
uses that instead.
@angelakis angelakis force-pushed the feature-configurable-grant-types branch from c90e418 to ec006ad Compare November 11, 2020 12:50
@angelakis
Copy link
Contributor Author

I believe this can be merged now.

@peppelinux
Copy link
Member

@rohe do we have to merge the new storage model before merge this or we can procede as It Is?

@peppelinux
Copy link
Member

This looks ready for merge. My fingers are on the merge Button, Is there any doubts? Can we proceed?

@rohe
Copy link
Contributor

rohe commented Nov 14, 2020

Go ahead!

@peppelinux peppelinux merged commit e923f21 into IdentityPython:develop Nov 14, 2020
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