fix(resource-recorder): check project permission by project id (#1839)

* nit

* feat: allow ulid in project endpoint paths

* refa

* feat: check by proj id

* fix: test

backends/src/client/gateway.rs

@@ -52,10 +52,14 @@ impl ProjectsDal for ServicesApiClient {
     }
 
     #[instrument(skip_all)]
-    async fn head_user_project(&self, user_token: &str, project_name: &str) -> Result<bool, Error> {
+    async fn head_user_project(
+        &self,
+        user_token: &str,
+        project_ident: &str,
+    ) -> Result<bool, Error> {
         self.request_raw(
             Method::HEAD,
-            format!("projects/{}", project_name).as_str(),
+            format!("projects/{}", project_ident).as_str(),
             None::<()>,
             Some(header_map_with_bearer(user_token)),
         )

backends/src/lib.rs

@@ -36,13 +36,7 @@ pub trait ClaimExt {
     async fn owns_project<G: ProjectsDal>(
         &self,
         projects_dal: &G,
-        project_name: &str,
-    ) -> Result<bool, client::Error>;
-    /// Verify if the claim subject has ownership of a project.
-    async fn owns_project_id<G: ProjectsDal>(
-        &self,
-        projects_dal: &G,
-        project_id: &str,
+        project_ident: &str,
     ) -> Result<bool, client::Error>;
 }
 
@@ -84,20 +78,9 @@ impl ClaimExt for Claim {
     async fn owns_project<G: ProjectsDal>(
         &self,
         projects_dal: &G,
-        project_name: &str,
-    ) -> Result<bool, client::Error> {
-        let token = self.token.as_ref().expect("token to be set");
-        projects_dal.head_user_project(token, project_name).await
-    }
-
-    #[instrument(skip_all)]
-    async fn owns_project_id<G: ProjectsDal>(
-        &self,
-        projects_dal: &G,
-        project_id: &str,
+        project_ident: &str,
     ) -> Result<bool, client::Error> {
         let token = self.token.as_ref().expect("token to be set");
-        let projects = projects_dal.get_user_project_ids(token).await?;
-        Ok(projects.iter().any(|id| id == project_id))
+        projects_dal.head_user_project(token, project_ident).await
     }
 }

backends/src/test_utils/gateway.rs

@@ -68,7 +68,7 @@ pub async fn get_mocked_gateway_server() -> MockServer {
 
     let p = projects.clone();
     Mock::given(method(http::Method::HEAD))
-        .and(path_regex("/projects/[a-z0-9-]+"))
+        .and(path_regex("/projects/[a-zA-Z0-9-]+"))
         .respond_with(move |req: &Request| {
             let Some(bearer) = req.headers.get("AUTHORIZATION") else {
                 return ResponseTemplate::new(401);
@@ -77,7 +77,9 @@ pub async fn get_mocked_gateway_server() -> MockServer {
 
             let user = bearer.to_str().unwrap().split_whitespace().nth(1).unwrap();
 
-            if p.iter().any(|p| p.owner_id == user && p.name == project) {
+            if p.iter()
+                .any(|p| p.owner_id == user && (p.name == project || p.id == project))
+            {
                 ResponseTemplate::new(200)
             } else {
                 ResponseTemplate::new(401)

cargo-shuttle/src/lib.rs

@@ -2275,8 +2275,8 @@ impl Shuttle {
                 state => {
                     debug!("deployment logs stream received state: {state} when it expected to receive running state");
                     println!(
-                    "Deployment entered an unexpected state - Please create a ticket to report this."
-                );
+                        "Deployment entered an unexpected state - Please create a ticket to report this."
+                    );
                 }
             }
 

gateway/src/auth.rs

@@ -9,6 +9,7 @@ use shuttle_common::claims::Claim;
 use shuttle_common::models::error::{ApiError, InvalidProjectName, ProjectNotFound};
 use thiserror::Error;
 use tracing::error;
+use ulid::Ulid;
 
 use crate::api::latest::RouterState;
 use crate::service;
@@ -78,17 +79,27 @@ where
             .await
             .map_err(Error::StatusCode)?;
 
-        let scope = match Path::<ProjectName>::from_request_parts(parts, state).await {
-            Ok(Path(p)) => p,
-            Err(_) => Path::<(ProjectName, String)>::from_request_parts(parts, state)
-                .await
-                .map(|Path((p, _))| p)
-                .map_err(|_| InvalidProjectName)?,
-        };
-
         let RouterState { service, .. } = RouterState::from_ref(state);
 
-        let allowed = claim.is_admin()
+        // Enables checking HEAD at /projects/{ulid}, used by res-rec to check permission for a proj id
+        let scope = match Path::<Ulid>::from_request_parts(parts, state).await {
+            Ok(Path(ulid)) => {
+                let p = service.find_project_by_id(&ulid.to_string()).await?;
+                ProjectName::new(&p.name).expect("valid project name")
+            }
+            Err(_) => {
+                // Normal check for project name in path
+                match Path::<ProjectName>::from_request_parts(parts, state).await {
+                    Ok(Path(p)) => p,
+                    Err(_) => Path::<(ProjectName, String)>::from_request_parts(parts, state)
+                        .await
+                        .map(|Path((p, _))| p)
+                        .map_err(|_| InvalidProjectName)?,
+                }
+            }
+        };
+
+        if claim.is_admin()
             || claim.is_deployer()
             || service
                 .permit_client
@@ -97,9 +108,8 @@ where
                     &service.find_project_by_name(&scope).await?.id,
                     "develop", // TODO: make this configurable per endpoint?
                 )
-                .await?;
-
-        if allowed {
+                .await?
+        {
             Ok(Self { claim, scope })
         } else {
             Err(ProjectNotFound(scope.to_string()).into())

resource-recorder/src/lib.rs

@@ -117,7 +117,7 @@ where
         if !claim.is_admin()
             && !claim.is_deployer()
             && !claim
-                .owns_project_id(&self.gateway_client, project_id)
+                .owns_project(&self.gateway_client, project_id)
                 .await
                 .map_err(|_| Status::internal("could not verify project ownership"))?
         {