Merge pull request #5571 from Turbo87/auth-check-scopes

AuthCheck: Add `with_endpoint_scope()` and `for_crate()` options

Author: Turbo87

src/auth.rs

@@ -1,6 +1,7 @@
 use crate::controllers;
 use crate::db::RequestTransaction;
 use crate::middleware::log_request;
+use crate::models::token::{CrateScope, EndpointScope};
 use crate::models::{ApiToken, User};
 use crate::util::errors::{
     account_locked, forbidden, internal, AppError, AppResult, InsecurelyGeneratedTokenRevoked,
@@ -13,17 +14,43 @@ use http::header;
 #[derive(Debug, Clone)]
 pub struct AuthCheck {
     allow_token: bool,
+    endpoint_scope: Option<EndpointScope>,
+    crate_name: Option<String>,
 }
 
 impl AuthCheck {
     #[must_use]
     pub fn default() -> Self {
-        Self { allow_token: true }
+        Self {
+            allow_token: true,
+            endpoint_scope: None,
+            crate_name: None,
+        }
     }
 
     #[must_use]
     pub fn only_cookie() -> Self {
-        Self { allow_token: false }
+        Self {
+            allow_token: false,
+            endpoint_scope: None,
+            crate_name: None,
+        }
+    }
+
+    pub fn with_endpoint_scope(&self, endpoint_scope: EndpointScope) -> Self {
+        Self {
+            allow_token: self.allow_token,
+            endpoint_scope: Some(endpoint_scope),
+            crate_name: self.crate_name.clone(),
+        }
+    }
+
+    pub fn for_crate(&self, crate_name: &str) -> Self {
+        Self {
+            allow_token: self.allow_token,
+            endpoint_scope: self.endpoint_scope,
+            crate_name: Some(crate_name.to_string()),
+        }
     }
 
     pub fn check(&self, request: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
@@ -47,19 +74,63 @@ impl AuthCheck {
             log_request::add_custom_metadata("tokenid", id);
         }
 
-        if !self.allow_token && auth.token_id.is_some() {
-            let error_message = "API Token authentication was explicitly disallowed for this API";
-            return Err(internal(error_message).chain(forbidden()));
+        if let Some(ref token) = auth.token {
+            if !self.allow_token {
+                let error_message =
+                    "API Token authentication was explicitly disallowed for this API";
+                return Err(internal(error_message).chain(forbidden()));
+            }
+
+            if !self.endpoint_scope_matches(token.endpoint_scopes.as_ref()) {
+                let error_message = "Endpoint scope mismatch";
+                return Err(internal(error_message).chain(forbidden()));
+            }
+
+            if !self.crate_scope_matches(token.crate_scopes.as_ref()) {
+                let error_message = "Crate scope mismatch";
+                return Err(internal(error_message).chain(forbidden()));
+            }
         }
 
         Ok(auth)
     }
+
+    fn endpoint_scope_matches(&self, token_scopes: Option<&Vec<EndpointScope>>) -> bool {
+        match (&token_scopes, &self.endpoint_scope) {
+            // The token is a legacy token.
+            (None, _) => true,
+
+            // The token is NOT a legacy token, and the endpoint only allows legacy tokens.
+            (Some(_), None) => false,
+
+            // The token is NOT a legacy token, and the endpoint allows a certain endpoint scope or a legacy token.
+            (Some(token_scopes), Some(endpoint_scope)) => token_scopes.contains(endpoint_scope),
+        }
+    }
+
+    fn crate_scope_matches(&self, token_scopes: Option<&Vec<CrateScope>>) -> bool {
+        match (&token_scopes, &self.crate_name) {
+            // The token is a legacy token.
+            (None, _) => true,
+
+            // The token does not have any crate scopes.
+            (Some(token_scopes), _) if token_scopes.is_empty() => true,
+
+            // The token has crate scopes, but the endpoint does not deal with crates.
+            (Some(_), None) => false,
+
+            // The token is NOT a legacy token, and the endpoint allows a certain endpoint scope or a legacy token.
+            (Some(token_scopes), Some(crate_name)) => token_scopes
+                .iter()
+                .any(|token_scope| token_scope.matches(crate_name)),
+        }
+    }
 }
 
 #[derive(Debug)]
 pub struct AuthenticatedUser {
     user: User,
-    token_id: Option<i32>,
+    token: Option<ApiToken>,
 }
 
 impl AuthenticatedUser {
@@ -68,7 +139,11 @@ impl AuthenticatedUser {
     }
 
     pub fn api_token_id(&self) -> Option<i32> {
-        self.token_id
+        self.api_token().map(|token| token.id)
+    }
+
+    pub fn api_token(&self) -> Option<&ApiToken> {
+        self.token.as_ref()
     }
 
     pub fn user(self) -> User {
@@ -86,10 +161,7 @@ fn authenticate_user(req: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
         let user = User::find(&conn, id)
             .map_err(|err| err.chain(internal("user_id from cookie not found in database")))?;
 
-        return Ok(AuthenticatedUser {
-            user,
-            token_id: None,
-        });
+        return Ok(AuthenticatedUser { user, token: None });
     }
 
     // Otherwise, look for an `Authorization` header on the request
@@ -112,10 +184,110 @@ fn authenticate_user(req: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
 
         return Ok(AuthenticatedUser {
             user,
-            token_id: Some(token.id),
+            token: Some(token),
         });
     }
 
     // Unable to authenticate the user
     return Err(internal("no cookie session or auth header found").chain(forbidden()));
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn cs(scope: &str) -> CrateScope {
+        CrateScope::try_from(scope).unwrap()
+    }
+
+    #[test]
+    fn regular_endpoint() {
+        let auth_check = AuthCheck::default();
+
+        assert!(auth_check.endpoint_scope_matches(None));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
+
+        assert!(auth_check.crate_scope_matches(None));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
+    }
+
+    #[test]
+    fn publish_new_endpoint() {
+        let auth_check = AuthCheck::default()
+            .with_endpoint_scope(EndpointScope::PublishNew)
+            .for_crate("tokio-console");
+
+        assert!(auth_check.endpoint_scope_matches(None));
+        assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
+
+        assert!(auth_check.crate_scope_matches(None));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
+    }
+
+    #[test]
+    fn publish_update_endpoint() {
+        let auth_check = AuthCheck::default()
+            .with_endpoint_scope(EndpointScope::PublishUpdate)
+            .for_crate("tokio-console");
+
+        assert!(auth_check.endpoint_scope_matches(None));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
+        assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
+
+        assert!(auth_check.crate_scope_matches(None));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
+    }
+
+    #[test]
+    fn yank_endpoint() {
+        let auth_check = AuthCheck::default()
+            .with_endpoint_scope(EndpointScope::Yank)
+            .for_crate("tokio-console");
+
+        assert!(auth_check.endpoint_scope_matches(None));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
+        assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
+
+        assert!(auth_check.crate_scope_matches(None));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
+    }
+
+    #[test]
+    fn owner_change_endpoint() {
+        let auth_check = AuthCheck::default()
+            .with_endpoint_scope(EndpointScope::ChangeOwners)
+            .for_crate("tokio-console");
+
+        assert!(auth_check.endpoint_scope_matches(None));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
+        assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
+        assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
+
+        assert!(auth_check.crate_scope_matches(None));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
+        assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
+        assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
+    }
+}

src/tests/owners.rs

@@ -321,11 +321,17 @@ fn owner_change_via_change_owner_token() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, &body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
-        json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+        json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
     );
+    // TODO swap these assertions once token scopes are activated for this endpoint
+    // assert_eq!(response.status(), StatusCode::OK);
+    // assert_eq!(
+    //     response.into_json(),
+    //     json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+    // );
 }
 
 #[test]
@@ -344,11 +350,17 @@ fn owner_change_via_change_owner_token_with_matching_crate_scope() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, &body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
-        json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+        json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
     );
+    // TODO swap these assertions once token scopes are activated for this endpoint
+    // assert_eq!(response.status(), StatusCode::OK);
+    // assert_eq!(
+    //     response.into_json(),
+    //     json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+    // );
 }
 
 #[test]
@@ -367,17 +379,11 @@ fn owner_change_via_change_owner_token_with_wrong_crate_scope() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, &body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
-        json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+        json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
     );
-    // TODO this should return "403 Forbidden" once token scopes are implemented for this endpoint
-    // assert_eq!(response.status(), StatusCode::FORBIDDEN);
-    // assert_eq!(
-    //     response.into_json(),
-    //     json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
-    // );
 }
 
 #[test]
@@ -395,17 +401,11 @@ fn owner_change_via_publish_token() {
     let body = json!({ "owners": [user2.gh_login] });
     let body = serde_json::to_vec(&body).unwrap();
     let response = token.put::<()>(&url, &body);
-    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(response.status(), StatusCode::FORBIDDEN);
     assert_eq!(
         response.into_json(),
-        json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
+        json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
     );
-    // TODO this should return "403 Forbidden" once token scopes are implemented for this endpoint
-    // assert_eq!(response.status(), StatusCode::FORBIDDEN);
-    // assert_eq!(
-    //     response.into_json(),
-    //     json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
-    // );
 }
 
 #[test]