Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove 404 on revoke consent #2397

Merged
merged 2 commits into from
Mar 22, 2021

Conversation

NikolaySl
Copy link
Contributor

Related issue

No

Proposed changes

Do not throw 404 on revoke consent sessions of subject - https://www.ory.sh/hydra/docs/reference/api/#revokes-consent-sessions-of-a-subject-for-a-specific-oauth-20-client

The same changes and as in #1168 (#1173)

Checklist

  • [x ] I have read the contributing guidelines.
  • [ x] I have read the security policy.
  • [x ] I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • [x ] I have added tests that prove my fix is effective or that my feature
    works.
  • [ x] I have added or changed the documentation.

Further comments

@CLAassistant
Copy link

CLAassistant commented Mar 10, 2021

CLA assistant check
All committers have signed the CLA.

@NikolaySl NikolaySl changed the title Remove 404 on revoke consent fix: Remove 404 on revoke consent Mar 10, 2021
@aeneasr
Copy link
Member

aeneasr commented Mar 10, 2021

Thank you, this looks great! Could you please sign the CLA (just two clicks), then we can merge it right away!

@NikolaySl NikolaySl force-pushed the Remove_404_on_revoke_consent branch from a5f83a6 to eb37506 Compare March 10, 2021 17:12
@NikolaySl
Copy link
Contributor Author

@aeneasr Thanks for quick response! I've signed SLA.

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Looking good! To complete this, we should also remove 404 from the list of swagger responses here and since I forgot that in my original PR probably also here

@NikolaySl
Copy link
Contributor Author

Just to clarify - I'm under impression that 404 is still possible in both cases: here (one more) and here
If I'm wrong I'll update right away.

persistence/sql/persister_consent.go Outdated Show resolved Hide resolved
@NikolaySl NikolaySl force-pushed the Remove_404_on_revoke_consent branch from cdcd20a to 26444d5 Compare March 19, 2021 10:01
@aeneasr aeneasr merged commit 854b9ee into ory:master Mar 22, 2021
@aeneasr
Copy link
Member

aeneasr commented Mar 22, 2021

Awesome, thank you! 🎉 Your contribution makes Ory better :)

@NikolaySl NikolaySl deleted the Remove_404_on_revoke_consent branch March 23, 2021 09:34
@NikolaySl
Copy link
Contributor Author

@aeneasr Thank you for quick turn around. When new release with this fix will be available?

@aeneasr
Copy link
Member

aeneasr commented Mar 23, 2021

There have been a couple of changes since 1.9.2 v1.9.2...master and I need to review them but this week it's pretty hectic, so probably next week

@grantzvolsky grantzvolsky mentioned this pull request Mar 30, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants