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

docs: oauth2/revoke api fewer parameter as required in doc #3261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arjprd
Copy link
Contributor

@arjprd arjprd commented Sep 16, 2022

client id and client secret are mandatory in oauth2/revoke api

Related issue(s)

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • 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.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

client_id and client_secret are mandatory in token revoke api
@arjprd arjprd requested a review from aeneasr as a code owner September 16, 2022 11:34
@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #3261 (89f9a1e) into master (954693f) will not change coverage.
Report is 321 commits behind head on master.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3261   +/-   ##
=======================================
  Coverage   76.80%   76.80%           
=======================================
  Files         123      123           
  Lines        8845     8845           
=======================================
  Hits         6793     6793           
  Misses       1627     1627           
  Partials      425      425           

@arjprd arjprd changed the title fix: oauth2/revoke api fewer parameter as required in doc docs: oauth2/revoke api fewer parameter as required in doc Sep 19, 2022
@aeneasr aeneasr requested a review from jonas-jonas October 4, 2022 09:32
Copy link
Member

@jonas-jonas jonas-jonas left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for your PR. :)

Unfortunately, these files are generated by a generator, so manually changing them does not work, as they will be overridden the next time someone runs the generation.

But, fixing that is easy:

  1. Go to https://github.com/ory/hydra/blob/master/oauth2/handler.go#L578
  2. Add ClientID and ClientSecret to the revokeOAuth2Token struct with the appropriate tags and comments (these are what defines what to generate!)
  3. Run make sdk in the root of the repository

The last step will update/generate a few other files as well - these are necessary and should be committed, too.

And another note: since the time you opened the PR we updated the Go version to 1.19 in this repository. That introduced a few formatting changes, so please update the state on your branch (rebase or merge) and make sure to run make sdk from a shell that has Go 1.19 active (check via go version) to not introduce formatting issues.

Thanks again for providing this PR! :)

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.

2 participants