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

Conversation

ctriant
Copy link
Contributor

@ctriant ctriant commented Sep 22, 2021

Introduce parameter revoke_refresh_on_issue in order to revoke the Refresh Token used to issue a new one.

@ctriant ctriant force-pushed the revoke_refresh_token_on_issue branch from 72c3b74 to 3ceea51 Compare September 23, 2021 12:56
@ctriant ctriant force-pushed the revoke_refresh_token_on_issue branch from 3ceea51 to 1a0b5b5 Compare September 29, 2021 08:21
Comment on lines 84 to 87
issue_refresh = kwargs.get("issue_refresh", False)
if "offline_access" in grant.scope:
issue_refresh = True
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic is reversed here as well. issue_refresh used to override offline_access, which IMHO is the correct behavior. Maybe this is better:

        issue_refresh = kwargs.get("refresh_token", None)
        # The existence of offline_access scope overwrites issue_refresh
        if issue_refresh is None and "offline_access" in scope:
            issue_refresh = True

Copy link
Member

Choose a reason for hiding this comment

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

ok @nsklikas I wait for your revision before merge

@ctriant ctriant force-pushed the revoke_refresh_token_on_issue branch from 1a0b5b5 to d96cddc Compare October 1, 2021 09:27
@ctriant ctriant force-pushed the revoke_refresh_token_on_issue branch from d96cddc to 16e99e6 Compare October 1, 2021 09:31
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.

I like this feature, thank you guys!
@ctriant let us know when this PR would be ready to be merged, we're watching many integrations, take your time and give us a know when ready for merge

@ctriant
Copy link
Contributor Author

ctriant commented Oct 1, 2021

I like this feature, thank you guys! @ctriant let us know when this PR would be ready to be merged, we're watching many integrations, take your time and give us a know when ready for merge

@peppelinux i think we are ready, i integrated the suggestions of @nsklikas

@peppelinux peppelinux requested a review from nsklikas October 2, 2021 16:43
@peppelinux peppelinux merged commit 47120f8 into IdentityPython:develop Oct 6, 2021
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