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

feat: add front/backchannel logout params to client cli #2387

Merged

Conversation

mattbonnell
Copy link
Contributor

@mattbonnell mattbonnell commented Mar 6, 2021

Related issue

#1487

Proposed changes

Adds client params related to OpenID Connect front/backchannel logout to the hydra clients create and hydra clients update CLI commands.

Checklist

  • I have read the contributing 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
    security@ory.sh) 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

make docs/cli generated a lot of changes in the docs, perhaps it hadn't been run in awhile.

@mattbonnell
Copy link
Contributor Author

test failing on a go.sum diff, yet the PR doesn't contain any changes to go.sum 🤔

@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

If you merge master into this branch, the CI should work!

@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

Ah no sorry, I still have to fix something

@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

Thank you, this looks great! The CI is failing because some files are formatted incorrectly. To format them, run:

$ make format
$ git commit -a -m "styles: format code"
$ git push

Thank you!

@mattbonnell
Copy link
Contributor Author

Thank you, this looks great! The CI is failing because some files are formatted incorrectly. To format them, run:

$ make format
$ git commit -a -m "styles: format code"
$ git push

Thank you!

Thanks!

It seems like the Makefile is missing a docs/node_modules rule:

❯ make format
make: *** No rule to make target `docs/node_modules', needed by `format'.  Stop.

I ran the three commands in the format rule manually:

goimports -w --local github.com/ory .                                                                                                 
npm run format
cd docs; npm run format

Seems CI is still not happy 😢

@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

Try

make node_modules
make docs/node_modules
make format

For me it formats:

        geändert:       docs/docs/concepts/consent.mdx
        geändert:       docs/docs/concepts/login.mdx
        geändert:       docs/docs/guides/consent.mdx
        geändert:       docs/docs/guides/login.mdx
        geändert:       docs/docs/guides/logout.mdx
        geändert:       docs/docs/reference/api.mdx
        geändert:       docs/scripts/rerelease.js

@mattbonnell
Copy link
Contributor Author

mattbonnell commented Mar 7, 2021

Try

make node_modules
make docs/node_modules
make format

For me it formats:

        geändert:       docs/docs/concepts/consent.mdx
        geändert:       docs/docs/concepts/login.mdx
        geändert:       docs/docs/guides/consent.mdx
        geändert:       docs/docs/guides/login.mdx
        geändert:       docs/docs/guides/logout.mdx
        geändert:       docs/docs/reference/api.mdx
        geändert:       docs/scripts/rerelease.js
❯ make node_modules
make: `node_modules' is up to date.
❯ make docs/node_modules
make: *** No rule to make target `docs/node_modules'.  Stop.
❯ make format
make: *** No rule to make target `docs/node_modules', needed by `format'.  Stop.

The Makefile is indeed missing a docs/node_modules rule https://github.com/ory/hydra/blob/master/Makefile

@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

Looks like there's another makefile problem. Try:

npm i
npm run format
cd docs
npm i
npm run format

@mattbonnell mattbonnell force-pushed the mattbonnell/cli-client-openid-logout branch from 88bd548 to ab491fe Compare March 7, 2021 17:37
@mattbonnell
Copy link
Contributor Author

Thanks @aeneasr, that did the trick!

@aeneasr aeneasr merged commit 055f801 into ory:master Mar 7, 2021
@aeneasr
Copy link
Member

aeneasr commented Mar 7, 2021

Awesome, thank you! (:

@mattbonnell mattbonnell deleted the mattbonnell/cli-client-openid-logout branch March 7, 2021 18:33
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