-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
deps: update go-jose/go-jose
to v4
#5596
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the link here 🫶
Curious; how did the previous version handle this? Or would it always accept anything it knew? Do we know if this list could be updated by auth0 (and how we would get notified of this?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading those docs;
I wonder if this is something controlled by Docker Hub (and in which case possibly we could limit the list to what Docker Hub uses)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, Hub only uses a subset of these but having this larger list (which includes higher key-length ones and all the ones supported by auth0) means that if some change were to happen on auth0, we'd continue working. As of right now, I don't think we lose anything by accepting that longer list. I'd expect that if a vulnerability was found for one of these we'd find out/be notified about it.
Likewise, I'd hope we can figure out/agree on being notified if other teams internally are changing the algorithms used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also in
/cli/internal/
, so I think we don't need to be as worried about future support/being able to drop some from the list in the future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. Previously the lib would accept any algorithm (that its code was aware of) and this just adds some forward security in case one of those becomes insecure or able to be abused for a DOS.
Technically just RS256 is used today in hub and our own token signings (/v2/users/login). No need to worry about M2M here, but it too changed to RS256 about a week ago 😅.
Would agree that including all supported auth0 algs is best here. In theory, we should be in control of any changes to that on our tenant but auth0 has changed things on us before without telling...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @twelsh-aw ! Sounds like we're good to go then ❤️