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

Add recognizable prefix to Pinniped secrets (auth codes, refresh tokens, access tokens) #688

Closed
mattmoyer opened this issue Jun 30, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request estimate/M Estimated effort/complexity/risk is medium state/accepted All done! stub Stub issues that are lacking proper descriptions

Comments

@mattmoyer
Copy link
Contributor

mattmoyer commented Jun 30, 2021

Something along the lines of https://github.blog/2021-04-05-behind-githubs-new-authentication-token-formats/. The benefit is if you encounter one of these secrets out of context, it's easy to figure out what you're looking at. We could also add a short checksum as GH did. We probably can't do anything like this for JWT's, but they are somewhat self-describing anyway.

This feature would allow enterprises to reliably scan code repositories for any accidentally-committed tokens.

Notes:

  • This is referring to the downstream access & refresh tokens, and authcodes
  • Ensure the at_hash claim in the id_token is still correct after this change
  • Update the storage version
@mattmoyer mattmoyer added the stub Stub issues that are lacking proper descriptions label Jun 30, 2021
@pinniped-ci-bot pinniped-ci-bot added enhancement New feature or request priority/undecided Not yet prioritized labels Jun 30, 2021
@cfryanr
Copy link
Member

cfryanr commented Jul 2, 2021

One small detail: it would make sense to trim the prefix before using it to create the name of the Kube Secret in which the object (e.g. authcode) is stored. We tried to name these Secrets in such a way that they were not pushing up against the limits of what it allowed for the length of a Kube resource name.

@anjaltelang
Copy link
Contributor

Reviewed on 12-7-21. On Roadmap.

@pinniped-ci-bot pinniped-ci-bot added priority/backlog Prioritized for an upcoming iteration and removed priority/undecided Not yet prioritized labels Dec 12, 2021
@pinniped-ci-bot pinniped-ci-bot added the estimate/M Estimated effort/complexity/risk is medium label Apr 11, 2022
@pinniped-ci-bot pinniped-ci-bot added the state/started Someone is working on it currently label Apr 12, 2022
@pinniped-ci-bot pinniped-ci-bot added state/finished Code finished but not yet delivered state/delivered Ready for manual acceptance review and removed state/started Someone is working on it currently state/finished Code finished but not yet delivered labels Apr 13, 2022
@pinniped-ci-bot pinniped-ci-bot added state/accepted All done! and removed priority/backlog Prioritized for an upcoming iteration state/delivered Ready for manual acceptance review labels Apr 19, 2022
cfryanr added a commit that referenced this issue Dec 14, 2022
Most of the changes in this commit are because of these fosite PRs
which changed behavior and/or APIs in fosite:
- ory/fosite#667
- ory/fosite#679 (from me!)
- ory/fosite#675
- ory/fosite#688

Due to the changes in fosite PR #688, we need to bump our storage
version for anything which stores the DefaultSession struct as JSON.
cfryanr added a commit that referenced this issue Dec 14, 2022
Most of the changes in this commit are because of these fosite PRs
which changed behavior and/or APIs in fosite:
- ory/fosite#667
- ory/fosite#679 (from me!)
- ory/fosite#675
- ory/fosite#688

Due to the changes in fosite PR #688, we need to bump our storage
version for anything which stores the DefaultSession struct as JSON.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request estimate/M Estimated effort/complexity/risk is medium state/accepted All done! stub Stub issues that are lacking proper descriptions
Projects
Status: Done
Development

No branches or pull requests

4 participants