Hirevo / alexandrie

An alternative crate registry, implemented in Rust.
https://hirevo.github.io/alexandrie/
Apache License 2.0
493 stars 55 forks source link

Option to enable authentication for all cargo API routes #176

Open fsavy-tehtris opened 11 months ago

fsavy-tehtris commented 11 months ago

With the stabilization of credential-process in cargo, more routes can be protected with a token that cargo can provide. (this is kinda related to #93)

Are there any plans to add authentication to these other routes (I'm mostly interested in download)?

From a quick glance, I believe this could be achieved by adding an Auth parameter like it's done in publish: https://github.com/Hirevo/alexandrie/blob/4813442d0bc4ff419725c1684ffff94960d2cce7/crates/alexandrie/src/api/crates/publish.rs#L195-L197

We also probably need an additional configuration value to let users enable or disable this feature. Would you welcome a PR with such modifications?

Hirevo commented 11 months ago

Hi, thank you for notifying me about this.

The credential-process RFC is rather specific to Cargo (on the user's machine) and doesn't need any changes on the side of the registries.

But the registry-auth RFC (which has been stabilised alongside credential-process in rust-lang/cargo#12649) does specify how registries can communicate to Cargo to send the Authorization header for every endpoints (like crate search and crate download).

They seem to have done this about two weeks ago and I completely missed it, thanks again for the notification.

As a first approach, I think you are right to propose a new configuration option, similar to the login_required one in [frontend] (maybe a new auth_required option in [general] ?).

Implementation-wise, this would mean to make the previously-unauthenticated endpoints take in an Option<Auth> argument and make the relevant checks in the body, in the same manner that the frontend does it with login_required:

https://github.com/Hirevo/alexandrie/blob/4813442d0bc4ff419725c1684ffff94960d2cce7/crates/alexandrie/src/frontend/krate.rs#L38-L40

Hirevo commented 11 months ago

Actually, now that there is a new auth-required option in the crate index's config.json file, one way to implement this could be to have the registry read out this value when starting up, instead of having a new configuration option of our own which could get out of sync with that config.json file.

We could even make the frontend.login_required option be optional and make it inherit from that auth-required option from the crate index if it is omitted.