Auto merge of #3518 - jtgeibel:forbid-token-auth-on-me, r=pietroalbini

Forbid token auth on `/me` endpoint

This prohibits access to the `/api/v1/me` endpoint via an API token. I'd like to merge and deploy this before moving forward on rust-lang/simpleinfra#43, which would expose this endpoint. (Previous lockdown work was done in #3470.)

The second commit ensure that the user is only authenticated on the search endpoint when querying for followed crates.

r? `@pietroalbini`

Author: bors

src/controllers/krate/search.rs

@@ -5,7 +5,6 @@ use diesel_full_text_search::*;
 
 use crate::controllers::cargo_prelude::*;
 use crate::controllers::helpers::Paginate;
-use crate::controllers::util::AuthenticatedUser;
 use crate::models::{
     Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, TopVersions, Version,
 };
@@ -40,9 +39,6 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS};
 pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::sql_types::{Bool, Text};
 
-    // Don't require that authentication succeed, because it's only necessary
-    // if the "following" param is set.
-    let authenticated_user: AppResult<AuthenticatedUser> = req.authenticate();
     let params = req.query();
     let sort = params.get("sort").map(|s| &**s);
     let include_yanked = params
@@ -160,7 +156,7 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
             ),
         );
     } else if params.get("following").is_some() {
-        let user_id = authenticated_user?.user_id();
+        let user_id = req.authenticate()?.user_id();
         query = query.filter(
             crates::id.eq_any(
                 follows::table

src/controllers/user/me.rs

@@ -13,7 +13,7 @@ use crate::views::{EncodableMe, EncodablePrivateUser, EncodableVersion, OwnedCra
 
 /// Handles the `GET /me` route.
 pub fn me(req: &mut dyn RequestExt) -> EndpointResult {
-    let user_id = req.authenticate()?.user_id();
+    let user_id = req.authenticate()?.forbid_api_token_auth()?.user_id();
     let conn = req.db_conn()?;
 
     let (user, verified, email, verification_sent): (User, Option<bool>, Option<String>, bool) =

src/tests/token.rs

@@ -1,4 +1,4 @@
-use crate::{user::UserShowPrivateResponse, RequestHelper, TestApp};
+use crate::{RequestHelper, TestApp};
 use cargo_registry::{
     models::ApiToken,
     schema::api_tokens,
@@ -272,17 +272,6 @@ fn revoke_token_success() {
     });
 }
 
-#[test]
-fn token_gives_access_to_me() {
-    let url = "/api/v1/me";
-    let (_, anon, user, token) = TestApp::init().with_token();
-
-    anon.get(url).assert_forbidden();
-
-    let json: UserShowPrivateResponse = token.get(url).good();
-    assert_eq!(json.user.name, user.as_model().name);
-}
-
 #[test]
 fn using_token_updates_last_used_at() {
     let url = "/api/v1/me";
@@ -293,7 +282,7 @@ fn using_token_updates_last_used_at() {
     assert_none!(token.as_model().last_used_at);
 
     // Use the token once
-    token.get::<EncodableMe>("/api/v1/me").good();
+    token.search("following=1");
 
     let token: ApiToken =
         app.db(|conn| assert_ok!(ApiToken::belonging_to(user.as_model()).first(conn)));