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

Define admin users and extend AuthCheck to handle them #6456

Closed
wants to merge 4 commits into from

Conversation

LawnGnome
Copy link
Contributor

This adds a concept of admin users, who are defined by their GitHub IDs, and allows them to be defined through an environment variable, falling back to a static list of the current crates.io team.

AuthCheck now has a builder method to require that the current cookie or token belong to an admin user.

In the future, this will be extended to use Rust's team API for the fallback.

@LawnGnome LawnGnome added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-accounts labels May 9, 2023
@LawnGnome LawnGnome requested a review from Turbo87 May 9, 2023 19:03
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

just a first glance review... I'll take a closer look tomorrow 😉

src/models/helpers/admin.rs Outdated Show resolved Hide resolved
src/models/helpers/admin.rs Outdated Show resolved Hide resolved
src/models/helpers/admin.rs Outdated Show resolved Hide resolved
src/models/user.rs Outdated Show resolved Hide resolved
src/models/user.rs Outdated Show resolved Hide resolved
@LawnGnome LawnGnome requested a review from Turbo87 May 17, 2023 22:49
@LawnGnome
Copy link
Contributor Author

Updated per review. Definitely simpler this way, so thanks!

I don't love the drilling of gh_admin_user_ids into AuthCheck, but here we are. We could sugar our way in with a helper on App just to clean up the controllers or a tuple struct wrapping the HashSet that isn't as wordy, if that's preferred, but I think this is just about OK.

@bors

This comment was marked as outdated.

src/config/server.rs Outdated Show resolved Hide resolved
src/config/server.rs Outdated Show resolved Hide resolved
LawnGnome and others added 4 commits December 28, 2023 15:12
This adds a concept of admin users, who are defined by their GitHub IDs, and allows them to be defined through an environment variable.
This will make it easier to test the logic in the future.
This function can be used to require that the current cookie or token belong to an admin user.
@LawnGnome
Copy link
Contributor Author

Closing in favour of #7852.

@LawnGnome LawnGnome closed this Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-accounts C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants