Add API Token Auth restrictions on GET APIs

Author: jakeswenson

src/controllers/crate_owner_invitation.rs

@@ -9,7 +9,7 @@ use std::collections::HashMap;
 /// Handles the `GET /me/crate_owner_invitations` route.
 pub fn list(req: &mut dyn RequestExt) -> EndpointResult {
     // Ensure that the user is authenticated
-    let user = req.authenticate()?.user();
+    let user = req.authenticate()?.forbid_api_token_auth()?.user();
 
     // Load all pending invitations for the user
     let conn = &*req.db_read_only()?;

src/controllers/krate/follow.rs

@@ -46,7 +46,7 @@ pub fn unfollow(req: &mut dyn RequestExt) -> EndpointResult {
 pub fn following(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::exists;
 
-    let user_id = req.authenticate()?.user_id();
+    let user_id = req.authenticate()?.forbid_api_token_auth()?.user_id();
     let conn = req.db_read_only()?;
     let follow = follow_target(req, &conn, user_id)?;
     let following = diesel::select(exists(follows::table.find(follow.id()))).get_result(&*conn)?;

src/controllers/token.rs

@@ -9,7 +9,7 @@ use serde_json as json;
 
 /// Handles the `GET /me/tokens` route.
 pub fn list(req: &mut dyn RequestExt) -> EndpointResult {
-    let authenticated_user = req.authenticate()?;
+    let authenticated_user = req.authenticate()?.forbid_api_token_auth()?;
     let conn = req.db_conn()?;
     let user = authenticated_user.user();
 

src/controllers/user/me.rs

@@ -54,7 +54,7 @@ pub fn me(req: &mut dyn RequestExt) -> EndpointResult {
 pub fn updates(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::any;
 
-    let authenticated_user = req.authenticate()?;
+    let authenticated_user = req.authenticate()?.forbid_api_token_auth()?;
     let user = authenticated_user.user();
 
     let followed_crates = Follow::belonging_to(&user).select(follows::crate_id);

src/controllers/util.rs

@@ -28,6 +28,18 @@ impl AuthenticatedUser {
     pub fn user(self) -> User {
         self.user
     }
+
+    /// Disallows token authenticated users
+    pub fn forbid_api_token_auth(self) -> AppResult<Self> {
+        if self.token_id.is_none() {
+            Ok(self)
+        } else {
+            Err(
+                internal("API Token authentication was explicitly disallowed for this API")
+                    .chain(forbidden()),
+            )
+        }
+    }
 }
 
 /// The Origin header (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin)

src/tests/krate/following.rs

@@ -57,3 +57,63 @@ fn following() {
     assert!(!is_following());
     assert_eq!(user.search("following=1").crates.len(), 0);
 }
+
+#[test]
+fn disallow_api_token_auth_for_get_crate_following_status() {
+    let (app, _, _, token) = TestApp::init().with_token();
+    let api_token = token.as_model();
+
+    let a_crate = "a_crate";
+
+    app.db(|conn| {
+        CrateBuilder::new(a_crate, api_token.user_id).expect_build(conn);
+    });
+
+    // Token auth on GET for get following status is disallowed
+    token
+        .get(&format!("/api/v1/crates/{}/following", a_crate))
+        .assert_forbidden();
+}
+
+#[test]
+fn getting_followed_crates_allows_api_token_auth() {
+    let (app, _, user, token) = TestApp::init().with_token();
+    let api_token = token.as_model();
+
+    let crate_to_follow = "some_crate_to_follow";
+    let crate_not_followed = "another_crate";
+
+    app.db(|conn| {
+        CrateBuilder::new(crate_to_follow, api_token.user_id).expect_build(conn);
+        CrateBuilder::new(crate_not_followed, api_token.user_id).expect_build(conn);
+    });
+
+    let is_following = |crate_name: &str| -> bool {
+        #[derive(Deserialize)]
+        struct F {
+            following: bool,
+        }
+
+        // Token auth on GET for get following status is disallowed
+        user.get::<F>(&format!("/api/v1/crates/{}/following", crate_name))
+            .good()
+            .following
+    };
+
+    let follow = |crate_name: &str| {
+        assert!(
+            token
+                .put::<OkBool>(&format!("/api/v1/crates/{}/follow", crate_name), b"")
+                .good()
+                .ok
+        );
+    };
+
+    follow(crate_to_follow);
+
+    assert!(is_following(crate_to_follow));
+    assert!(!is_following(crate_not_followed));
+
+    let json = token.search("following=1");
+    assert_eq!(json.crates.len(), 1);
+}

src/tests/owners.rs

@@ -320,6 +320,15 @@ fn invitations_are_empty_by_default() {
     assert_eq!(json.crate_owner_invitations.len(), 0);
 }
 
+#[test]
+fn api_token_cannot_list_invitations() {
+    let (_, _, _, token) = TestApp::init().with_token();
+
+    token
+        .get("/api/v1/me/crate_owner_invitations")
+        .assert_forbidden();
+}
+
 #[test]
 fn invitations_list() {
     let (app, _, owner, token) = TestApp::init().with_token();

src/tests/token.rs

@@ -36,6 +36,12 @@ fn list_logged_out() {
     anon.get(URL).assert_forbidden();
 }
 
+#[test]
+fn list_with_api_token_is_forbidden() {
+    let (_, _, _, token) = TestApp::init().with_token();
+    token.get(URL).assert_forbidden();
+}
+
 #[test]
 fn list_empty() {
     let (_, _, user) = TestApp::init().with_user();

src/tests/user.rs

@@ -206,6 +206,12 @@ fn crates_by_user_id_not_including_deleted_owners() {
     assert_eq!(response.crates.len(), 0);
 }
 
+#[test]
+fn api_token_cannot_get_user_updates() {
+    let (_, _, _, token) = TestApp::init().with_token();
+    token.get("/api/v1/me/updates").assert_forbidden();
+}
+
 #[test]
 fn following() {
     use cargo_registry::schema::versions;