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

Remove redundant sudo check #21968

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Conversation

maxb
Copy link
Contributor

@maxb maxb commented Jul 20, 2023

This is a follow-up to #21772.

Historically, for some reason, auth/token/revoke-orphan was
sudo-protected by writing custom code in its handler function, instead
of via the usual declarative PathsSpecial.Root mechanism.

In fact, there was a declaration mentioning revoke-orphan in the token
backend's PathsSpecial.Root, but it was incorrect! That was corrected
in #21772, making the custom code in the handler function redundant.
However, removal of the now-redundant code was deferred to this
follow-up PR, out of an abundance of caution, and wanting extra eyes on
a change deleting a security check.

This is a follow-up to hashicorp#21772.

Historically, for some reason, `auth/token/revoke-orphan` was
sudo-protected by writing custom code in its handler function, instead
of via the usual declarative PathsSpecial.Root mechanism.

In fact, there was a declaration mentioning revoke-orphan in the token
backend's PathsSpecial.Root, but it was incorrect! That was corrected
in hashicorp#21772, making the custom code in the handler function redundant.
However, removal of the now-redundant code was deferred to this
follow-up PR, out of an abundance of caution, and wanting extra eyes on
a change deleting a security check.
@maxb
Copy link
Contributor Author

maxb commented Jul 20, 2023

I request pr/no-changelog on the basis of this being a follow-up cleanup to what was already changelogged in the previous PR.

@maxb
Copy link
Contributor Author

maxb commented Jul 20, 2023

The corrected PathsSpecial.Root for the token backend, now matching this endpoint may be found here:

vault/vault/token_store.go

Lines 784 to 788 in e969e4a

PathsSpecial: &logical.Paths{
Root: []string{
"revoke-orphan",
"accessors/",
},

@averche averche requested a review from a team July 21, 2023 17:50
@averche averche added the core Issues and Pull-Requests specific to Vault Core label Jul 21, 2023
@maxb
Copy link
Contributor Author

maxb commented Jul 24, 2023

Thanks for the approval. Is there anything blocking merging this now?

The red PR statuses are that there is no milestone, and the ones that sadly fail on every community PR due to denied access.

@averche averche added this to the 11.15 milestone Jul 24, 2023
Copy link
Contributor

@averche averche left a comment

Choose a reason for hiding this comment

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

I believe we should be good to merge, thanks again @maxb!

@averche averche merged commit b2e110e into hashicorp:main Jul 24, 2023
46 of 47 checks passed
@maxb maxb deleted the clean-redundant-sudo-code branch July 25, 2023 03:38
@peteski22 peteski22 modified the milestones: 11.15, 1.15 Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues and Pull-Requests specific to Vault Core pr/no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants