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

Prototype admin console #6353

Closed
wants to merge 11 commits into from
Closed

Conversation

LawnGnome
Copy link
Contributor

@LawnGnome LawnGnome commented Apr 20, 2023

Context

This is an early exploration of implementing a HTML-based admin console. This is definitely not in a ready-to-merge state at present, and literally nothing in here is set in stone. There are many open technical questions below.

This PR only implements an admin console that can list crate versions, search through them, and yank them.1

It's likely that this would turn into several PRs if and when we decide to merge this. It's definite that I have to write more tests before that could happen. 🙈

Background, goals, et cetera

There's a decent amount of prior art here, starting with #3119. @carols10cents drafted #5376 a while back (and, indeed, I stole a couple of bits out of it in the first commit on this branch), and @mdtro has been exploring this space as well. I started a Zulip topic a couple of weeks ago to discuss some of the initial planning, so I'll be referencing that here and there below.

Fundamentally, the issue right now is that too much routine administration of crates.io requires direct access to Heroku. This makes it difficult to automate things that could otherwise be automated, and also means that there aren't a lot of guard rails for common operations. crates-admin helps with at least some of the guard rails, but not with the access issues. Implementing an admin console will allow team members to be able to help support crates.io without necessarily having to log directly into infrastructure.

Adjacent to this is future security work that the Rust Foundation and others would like to do which would share much of the same authorisation and API plumbing: surfacing audits and security scans of crates, showing provenance information for crate versions, and crate signing.

Future work

Should this be merged, likely fast follows would include:

  • Deletion of crates and crate versions
  • Crate quarantine
  • Maybe viewing audit actions?
  • You tell me!

Open technical questions and concerns

These are the foundational questions and concerns that have to be addressed before this can be considered for merging. In most cases I've made a choice just to allow this to be prototyped, but I'm not wedded to any of them.

Deployment location

The admin console could live within the existing crates.io site, or on a separate admin.crates.io site. For the time being, I've implemented it within the existing site, but opinions have been split on this in Zulip.

If we do decide to go to a separate site, then another consideration would be whether to make this a wholly separate application within Heroku2, or just route based on the Host header and point a subdomain at the same deployment. I don't have enough experience with crates.io ops — or, honestly, Heroku — to have an opinion here.

Authorisation

In Zulip, I framed this as a choice between "static list of users" and "flag in database". @pietroalbini quickly educated me on the third, probably better option: rely on the team API, once the current work3 to harden it further is complete.

In the meantime, I've gone with the static list of users based on an environment variable with a fallback to a hard coded list just for straight up ease-of-development, but on the assumption that it would be replaced fairly quickly with something from the team API.

From a controller perspective, this is implemented via a new AdminUser extractor that gets the user from their cookie and verifies that they are an admin.

Existing and forthcoming API endpoints

One thing I haven't really addressed in this PR is authorising API endpoints. There's a one line hack in the yank controller to allow an admin to yank a crate version, but ideally, this check should be made more generic, and be able to handle admin tokens as well with the appropriate token scope4.

Audit logging

This is the one thing I haven't put any effort into as yet. Putting stuff into version_owner_actions would presumably be a good first step, but I'm open to input here on what we should do, and how that should be surfaced.

Development experience

Here be dragons. 🐉

The Ember development server configures Express to handle any request with an Accept header including text/html internally, without ever handing it to any proxied site. This makes implementing a static HTML area within the backend challenging.5

Temporarily, I've addressed this by moving the development server to port 4201 and adding a Caddy reverse proxy in front of the server that can route requests to the backend as needed. This probably isn't a viable solution for anything that's going to be merged, since it adds another runtime dependency and complicates things.

I would greatly appreciate suggestions here (although we need to decide where and how the admin console will be deployed first, since that may affect how this is achieved technically).

Frontend tech stack

The preference expressed on Zulip was basically static HTML, so static HTML it is. 1998 is back.6

I chose to use Handlebars for templating to allow some knowledge crossover with the Ember frontend. Of course, the way Handlebars is used in Ember is wildly different to how it's used in this kind of scenario (as a quick glance at the new templates will reveal), and it's not clear to me that it's actually worth it. Still, we'll need something. 🤷

The actual frontend is styled and built using Bootstrap out of sheer, unadulterated laziness.

Trying this out

OK, finally, let's say you actually want to spin this up. Here's what that looks like right now, assuming you already have a crates.io development environment ready to go:

  1. Install caddy. Your package manager probably has it. Any v2 version should be fine.
  2. Run caddy with caddy run.
  3. Add an environment variable GH_ADMIN_USERS with a space or comma separated list of GitHub user names that you want to be admins in your local deployment. (Probably yours, but I'm not the boss of you.)
  4. Start everything else up normally.
  5. Go to http://localhost:4200/admin/ and behold. 🪄

Footnotes

  1. If you're thinking "wow, Adam, that took a long time for you to literally reimplement one API endpoint", you're… not wrong, honestly. But there's a bunch of foundational (small "f") stuff that has to happen for us to build bigger things later.

  2. We'd presumably have to share the database between applications, and do some refactoring so that common parts of API controllers like yank aren't duplicated. Not terrible, but it's a bit more up-front work.

  3. I had a quick look for a GitHub issue that describes what's going on, but I couldn't find it easily. Sorry!

  4. Paging @Turbo87 to the token scope phone, since I need to be more up to date on what's going on there.

  5. OK, I swore a lot while I was figuring that out. 🤬

  6. 1998 Adam has a lot of hot takes on last year's new Radiohead album. 🔥


# GitHub users that are admins on this instance, separated by commas. Whitespace
# will be ignored.
export GH_ADMIN_USERS=
Copy link
Member

Choose a reason for hiding this comment

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

Haven't taken a look at the whole PR, but please use GitHub IDs rather than usernames. Usernames can change and the old username can be taken over by another user.

@LawnGnome
Copy link
Contributor Author

Deployment location

Based on today's crates.io meeting, it sounds like we have a mild consensus forming for "subdirectory within crates.io" for deployment/infra reasons, with the understanding that we may choose to migrate to a separate domain later. (And we may, at least, want to add a redirect on admin.crates.io.)

@mdtro is going to research options for using a path-limited cookie with stronger security policies for authenticating in the admin console.

@Turbo87
Copy link
Member

Turbo87 commented Apr 22, 2023

RE dev experience: I'm not sure how your setup looks like, but assuming you use pnpm start:local you will have the Ember.js dev server running on port 4200 and the Rust API server on port 8888. If you want to access the admin panel you could presumably access http://localhost:8888/admin/ directly instead of going through port 4200? you may have to adjust https://github.com/rust-lang/crates.io/blob/master/src/middleware/ember_html.rs#L29 to also include /admin though.

I chose to use Handlebars for templating to allow some knowledge crossover with the Ember frontend.

one potential alternative is minijinja, which we already use for the database dump generation. from what I remember, it has fewer dependencies and was compiling a little faster.

another alternative that I would like to eventually try out is https://github.com/leptos-rs/leptos. I don't think we'd necessarily want their client-side rendering or hydration, but it might be nice for server-side rendering since it basically has a component model. but as I said, I don't have any real experience with it, so no idea if this would work well or not.

@LawnGnome
Copy link
Contributor Author

RE dev experience: I'm not sure how your setup looks like, but assuming you use pnpm start:local you will have the Ember.js dev server running on port 4200 and the Rust API server on port 8888. If you want to access the admin panel you could presumably access http://localhost:8888/admin/ directly instead of going through port 4200? you may have to adjust https://github.com/rust-lang/crates.io/blob/master/src/middleware/ember_html.rs#L29 to also include /admin though.

The problem here is that the auth callback is (currently) hardcoded to port 4200, and cookies on 4200 won't be shared with 8888, so you can't trivially access /admin/ on port 8888 as a result.

another alternative that I would like to eventually try out is https://github.com/leptos-rs/leptos. I don't think we'd necessarily want their client-side rendering or hydration, but it might be nice for server-side rendering since it basically has a component model. but as I said, I don't have any real experience with it, so no idea if this would work well or not.

Thanks! Now you mention it I think I've heard of it, but I've never tried it. Will investigate at least a base level of feasibility.

@bors
Copy link
Contributor

bors commented Apr 25, 2023

☔ The latest upstream changes (presumably 81bcaff) made this pull request unmergeable. Please resolve the merge conflicts.

@LawnGnome
Copy link
Contributor Author

another alternative that I would like to eventually try out is https://github.com/leptos-rs/leptos. I don't think we'd necessarily want their client-side rendering or hydration, but it might be nice for server-side rendering since it basically has a component model. but as I said, I don't have any real experience with it, so no idea if this would work well or not.

I looked into this: it's probably more interesting as an eventual candidate should we rewrite the public UI, rather than for this project. Its server side rendering seems to be oriented more towards a hybrid "initial SSR + hydrated client side from there" type of approach rather than pure SSR, which isn't really what we need here.

one potential alternative is minijinja, which we already use for the database dump generation. from what I remember, it has fewer dependencies and was compiling a little faster.

My initial reaction to this is positive, but I can't tell if that's just because I used to write a lot of Django and therefore it feels familiar. 😃

Will make sure it fits into the ad hoc component model we're sort of shrugging towards in the prototype, but I think it might end up being what I prefer.

@LawnGnome
Copy link
Contributor Author

should we rewrite the public UI

Oh, one thing I meant to add in there is that I do have some concerns over the accessibility story with Leptos should we go that way. It doesn't (presently) matter for the admin console, but I would expect a good a11y story is going to be a pretty hard requirement for the public UI.

@Turbo87 Turbo87 added the C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear label May 8, 2023
@LawnGnome
Copy link
Contributor Author

Closed in favour of the heavily reworked and rebased #6811.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants