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

Admin action plan #8049

Open
LawnGnome opened this issue Feb 1, 2024 · 9 comments
Open

Admin action plan #8049

LawnGnome opened this issue Feb 1, 2024 · 9 comments
Assignees
Labels
A-backend ⚙️ A-frontend 🐹 C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear C-tracking-issue Category: A tracking issue for an RFC, an unstable feature, or an issue made of many parts

Comments

@LawnGnome
Copy link
Contributor

Previously, on crates.io gaining support for admin actions without needing to run crates-admin...

Adding support for admin actions in UI has been identified as a priority for a while. Last year, I opened #6811 (among other PRs) to start fleshing that out as a separate admin console using server side rendering. I didn't particularly like the code, it was warty, and (as it turned out) other things came up that demanded a bit more of my attention like artifact signing and malware detection, so it hasn't progressed since then.

More recently, @Turbo87 merged #7852, which provides minimal support for the concept of crates.io admins, and uses it to allow them to yank and unyank crates.

This is a good first step, and I definitely prefer the idea of incorporating admin functionality more directly into crates.io rather than building a new frontend, but is incomplete — there are other actions that we also need to be able to perform, it's probably a little too easy for us to accidentally perform an action right now1, and the logging is arguably too ephemeral at present.

Here's what I'd like to do:

  1. Only allow admin actions when an admin explicitly opts in. Practically, this would involve something like GitHub's sudo mode (so you only get privileges for a limited period in one session), and we may want to consider requiring crates.io-specific 2FA as part of flipping that switch on.
  2. Migrate logging of admin actions into the database, rather than logging them into the normal logging machinery, so that we can retain them indefinitely, and
  3. Require an admin action to include an explanation for why it's being taken (in most cases, presumably just a link to a Zulip thread or Zendesk ticket).
  4. Finally, expand the list of admin actions in the UI to include:
    1. User locking and unlocking
    2. Mass crate yanking and unyanking
    3. Crate and crate version deletion2
    4. your idea here

Feedback encouraged, @rust-lang/crates-io!

Footnotes

  1. Of course, yank and unyank actions are trivially reversible, so it's not really that bad in practice when that's all we support. A misclick can be fixed with another click.

  2. This will probably require further thought on implementation, since it's not trivially reversible. @walterhpearce suggested putting it on a timer of a few minutes to allow easy cancellation before the action is taken.

@LawnGnome LawnGnome added A-frontend 🐹 C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ C-tracking-issue Category: A tracking issue for an RFC, an unstable feature, or an issue made of many parts labels Feb 1, 2024
@LawnGnome LawnGnome self-assigned this Feb 1, 2024
@Rustin170506
Copy link
Member

4. UI to include

Do you mean we need to implement it in our current frontend crates.io UI instead of an admin console UI?

2. Mass crate yanking and unyanking

I think we should create a separate page on crates.io if we want to add a base to our current frontend. Is that correct?

@Rustin170506
Copy link
Member

Rustin170506 commented Feb 2, 2024

BTW, thank you for writing it out! I am also highly interested in assisting with the implementation of the admin feature. Please let me know if there is anything I can do to help. I am also open to exploring new frontend frameworks. I have some experience with Vue and React. However, I understand that maybe we do not require an admin-console UI at the moment :(

@Turbo87
Copy link
Member

Turbo87 commented Feb 6, 2024

looks like a good plan! 👍

2. Migrate logging of admin actions into the database, rather than logging them into the normal logging machinery, so that we can retain them indefinitely,

I'm not so sure about this part. The log files can easily be pushed to S3 for long-term storage, while the database would have to carry them around forever. I'm currently not seeing the big advantage of logging to the database instead of using our regular logging system.

3. Require an admin action to include an explanation for why it's being taken (in most cases, presumably just a link to a Zulip thread or Zendesk ticket).

just to brain dump, IMHO yanking should generally require an explanation, also from regular owners. if a crate/version is yanked, as a user I would like to know if that is due to a security issue in a particular version, or whatever else the reason for it was.

Do you mean we need to implement it in our current frontend crates.io UI instead of an admin console UI?

yeah. at least for things that are already exposed in the regular user interface for crate owners this would make things a bit easier. for things like crate deletions we might need custom admin-only UI though.

I am also open to exploring new frontend frameworks

I guess eventually we'll have to move away from Ember, but it's somewhat hard to decide in which direction we should move forward if we did so. Whatever we go with, we would probably want to keep our frontend test suite, but that is currently coupled to the Ember code base. It might make sense to look into porting at least the higher-level tests to something like https://playwright.dev/.

@eth3lbert
Copy link
Contributor

but it's somewhat hard to decide in which direction we should move forward if we did so

I highly recommend checking out svelte/svelteKit. Its simplicity and easy learning curve make it suitable for developers of all levels. Furthermore, its performance and bundle size excel compared to other frameworks. Additionally, component-based development and a template syntax similar to Handlebars could ease the transition for crates.io developers.

@LawnGnome
Copy link
Contributor Author

Do you mean we need to implement it in our current frontend crates.io UI instead of an admin console UI?

As much as possible, yes.

I'm not so sure about this part. The log files can easily be pushed to S3 for long-term storage, while the database would have to carry them around forever. I'm currently not seeing the big advantage of logging to the database instead of using our regular logging system.

My main concern with using the logging system is that — if anything — we want to be retaining our general logs for less time, rather than more. I don't doubt that we could set up some plumbing to filter out only the admin action logs and send them to S3, but that's additional complexity.

My secondary concern is accessibility: if the logs are in the database, we could (if necessary) eventually build a dashboard for them, and we can search out of the read-only replica with SQL. Having them in S3 may restrict access further (I certainly don't have access to our S3 setup, and probably shouldn't), and makes it slower to access them if we need to.

I don't feel strongly enough about this to call it a blocker — my primary concern is that the logs are persisted somewhere indefinitely, not so much exactly where — but I did have reasons for suggesting the database.

just to brain dump, IMHO yanking should generally require an explanation, also from regular owners. if a crate/version is yanked, as a user I would like to know if that is due to a security issue in a particular version, or whatever else the reason for it was.

Yeah, I like that, actually. Might hack something together.

I am also open to exploring new frontend frameworks

On the frontend discussion: I'm on record in the past as being supportive of moving away from Ember.

I'm most familiar with React, but I also don't particularly like React (OK, mostly hooks), so I'm open to exploring other options as well. I would advocate pretty strongly for TypeScript in whatever we migrate to, though. #bikeshed

@Turbo87
Copy link
Member

Turbo87 commented Feb 9, 2024

we want to be retaining our general logs for less time, rather than more. I don't doubt that we could set up some plumbing to filter out only the admin action logs and send them to S3, but that's additional complexity.

idk about S3 exporting, but DataDog makes it quite easy to have different retention periods depending on the data contained within the log record.

if the logs are in the database, we could (if necessary) eventually build a dashboard for them

same for usage within DataDog, and it doesn't need as much custom code :)

but yeah, it's a tradeoff...

@Turbo87
Copy link
Member

Turbo87 commented Feb 12, 2024

cross-linking #3119 here, so that we don't forget about it :)

@Rustin170506
Copy link
Member

Rustin170506 commented Mar 1, 2024

During today's meeting, we discussed introducing the yank reason for crates.io UI for regular and admin users. We should also consider supporting it from Cargo's interface using the yank command. Users of Cargo have requested this feature for a long time. See more at rust-lang/cargo#2608

@LawnGnome Please let me know if I can help land it on both crates.io and the Cargo side. I guess we need a RFC or something for it? Or we are going to implement it on the crates.io side first?

@mdtro
Copy link
Contributor

mdtro commented Mar 1, 2024

An additional note on the admin action logs --

If we place them only in the database, they're generally going to be mutable. If we use a logging service of sorts or a S3 bucket with suitable restrictions configured, we can make sure they are in an immutable store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ A-frontend 🐹 C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear C-tracking-issue Category: A tracking issue for an RFC, an unstable feature, or an issue made of many parts
Projects
None yet
Development

No branches or pull requests

5 participants