feat(gateway): compare permission checks to permit (#1706)

* feat: permit logic in project list * feat: update scopeduser * fix: rebase * fix: allow deployer bypass * fix: project list name lookup * fix: check the project id * test: set high pdp timeout * refactor: compare permission checks & log diffs * feat: sort project list desc, remove pagination * fix: adjust pdp timeout * nits
shuttle-hq · Apr 3, 2024 · 9130d5b · 9130d5b
1 parent 7c48569
commit 9130d5b
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 81 deletions.
diff --git a/backends/tests/integration/permit_tests.rs b/backends/tests/integration/permit_tests.rs
@@ -90,7 +90,6 @@ mod needs_docker {
             let client = Client::new(
                 api_url.to_owned(),
                 PDP.get().unwrap().uri.clone(),
-                // "http://localhost:19716".to_owned(),
                 "default".to_owned(),
                 std::env::var("PERMIT_ENV").unwrap_or_else(|_| "testing".to_owned()),
                 api_key,
@@ -100,8 +99,6 @@ mod needs_docker {
 
             Wrap(client)
         }
-
-        async fn teardown(self) {}
     }
 
     #[test_context(Wrap)]

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -283,6 +283,8 @@ services:
     environment:
       - PDP_CONTROL_PLANE=https://api.eu-central-1.permit.io
       - PDP_API_KEY=${PERMIT_API_KEY}
+      # Querying users with lots of resource instances takes more than the default 1s
+      - PDP_OPA_CLIENT_QUERY_TIMEOUT=10
     ports:
       - 7000:7000
     networks:

diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs
@@ -106,11 +106,11 @@ async fn get_project(
     State(RouterState { service, .. }): State<RouterState>,
     ScopedUser { scope, .. }: ScopedUser,
 ) -> Result<AxumJson<project::Response>, Error> {
-    let project = service.find_project(&scope).await?;
+    let project = service.find_project_by_name(&scope).await?;
     let idle_minutes = project.state.idle_minutes();
 
     let response = project::Response {
-        id: project.project_id.to_uppercase(),
+        id: project.id.to_uppercase(),
         name: scope.to_string(),
         state: project.state.into(),
         idle_minutes,
@@ -129,25 +129,31 @@ async fn check_project_name(
         .await
         .map(AxumJson)
 }
-
 async fn get_projects_list(
     State(RouterState { service, .. }): State<RouterState>,
-    User { id: name, .. }: User,
-    Query(PaginationDetails { page, limit }): Query<PaginationDetails>,
+    User { id, .. }: User,
 ) -> Result<AxumJson<Vec<project::Response>>, Error> {
-    let limit = limit.unwrap_or(u32::MAX);
-    let page = page.unwrap_or(0);
-    let projects = service
-        // The `offset` is page size * amount of pages
-        .iter_user_projects_detailed(&name, limit * page, limit)
-        .await?
-        .map(|project| project::Response {
-            id: project.0.to_uppercase(),
-            name: project.1.to_string(),
-            idle_minutes: project.2.idle_minutes(),
-            state: project.2.into(),
-        })
-        .collect();
+    let mut projects = vec![];
+    for p in service
+        .permit_client
+        .get_user_projects(&id)
+        .await
+        .map_err(|_| Error::from(ErrorKind::Internal))?
+    {
+        let proj_id = p.resource.expect("project resource").key;
+        let project = service.find_project_by_id(&proj_id).await?;
+        let idle_minutes = project.state.idle_minutes();
+
+        let response = project::Response {
+            id: project.id,
+            name: project.name,
+            state: project.state.into(),
+            idle_minutes,
+        };
+        projects.push(response);
+    }
+    // sort by descending id
+    projects.sort_by(|p1, p2| p2.id.cmp(&p1.id));
 
     Ok(AxumJson(projects))
 }
@@ -199,7 +205,7 @@ async fn create_project(
         .await?;
 
     let response = project::Response {
-        id: project.project_id.to_string().to_uppercase(),
+        id: project.id.to_string().to_uppercase(),
         name: project_name.to_string(),
         state: project.state.into(),
         idle_minutes,
@@ -218,11 +224,11 @@ async fn destroy_project(
         ..
     }: ScopedUser,
 ) -> Result<AxumJson<project::Response>, Error> {
-    let project = service.find_project(&project_name).await?;
+    let project = service.find_project_by_name(&project_name).await?;
     let idle_minutes = project.state.idle_minutes();
 
     let mut response = project::Response {
-        id: project.project_id.to_uppercase(),
+        id: project.id.to_uppercase(),
         name: project_name.to_string(),
         state: project.state.into(),
         idle_minutes,
@@ -266,10 +272,9 @@ async fn delete_project(
     }
 
     let project_name = scoped_user.scope.clone();
-    let project = state.service.find_project(&project_name).await?;
+    let project = state.service.find_project_by_name(&project_name).await?;
 
-    let project_id =
-        Ulid::from_string(&project.project_id).expect("stored project id to be a valid ULID");
+    let project_id = Ulid::from_string(&project.id).expect("stored project id to be a valid ULID");
 
     // Try to startup destroyed, errored or outdated projects
     let project_deletable = project.state.is_ready() || project.state.is_stopped();
@@ -309,7 +314,7 @@ async fn delete_project(
         // Wait for the project to be ready
         handle.await;
 
-        let new_state = state.service.find_project(&project_name).await?;
+        let new_state = state.service.find_project_by_name(&project_name).await?;
 
         if !new_state.state.is_ready() {
             return Err(Error::from_kind(ErrorKind::ProjectCorrupted));
@@ -393,7 +398,7 @@ async fn override_create_service(
     scoped_user: ScopedUser,
     req: Request<Body>,
 ) -> Result<Response<Body>, Error> {
-    let user_id = scoped_user.user.claim.sub.clone();
+    let user_id = scoped_user.user.id.clone();
     let posthog_client = state.posthog_client.clone();
     tokio::spawn(async move {
         let event = async_posthog::Event::new("shuttle_api_start_deployment", &user_id);

diff --git a/gateway/src/auth.rs b/gateway/src/auth.rs
@@ -4,10 +4,11 @@ use axum::extract::{FromRef, FromRequestParts, Path};
 use axum::http::request::Parts;
 use serde::{Deserialize, Serialize};
 use shuttle_backends::project_name::ProjectName;
-use shuttle_common::claims::{Claim, Scope};
+use shuttle_backends::ClaimExt;
+use shuttle_common::claims::Claim;
 use shuttle_common::models::error::InvalidProjectName;
 use shuttle_common::models::user::UserId;
-use tracing::{trace, Span};
+use tracing::{error, trace, Span};
 
 use crate::api::latest::RouterState;
 use crate::{Error, ErrorKind};
@@ -19,7 +20,6 @@ use crate::{Error, ErrorKind};
 /// is valid against the user's owned resources.
 #[derive(Clone, Deserialize, PartialEq, Eq, Serialize, Debug)]
 pub struct User {
-    pub projects: Vec<ProjectName>,
     pub claim: Claim,
     pub id: UserId,
 }
@@ -32,18 +32,15 @@ where
 {
     type Rejection = Error;
 
-    async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> {
+    async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> {
         let claim = parts.extensions.get::<Claim>().ok_or(ErrorKind::Internal)?;
         let user_id = claim.sub.clone();
 
         // Record current account name for tracing purposes
         Span::current().record("account.user_id", &user_id);
 
-        let RouterState { service, .. } = RouterState::from_ref(state);
-
         let user = User {
             claim: claim.clone(),
-            projects: service.iter_user_projects(&user_id).await?.collect(),
             id: user_id,
         };
 
@@ -83,7 +80,43 @@ where
                 .map_err(|_| Error::from(ErrorKind::InvalidProjectName(InvalidProjectName)))?,
         };
 
-        if user.projects.contains(&scope) || user.claim.scopes.contains(&Scope::Admin) {
+        let RouterState { service, .. } = RouterState::from_ref(state);
+
+        let has_bypass = user.claim.is_admin() || user.claim.is_deployer();
+
+        let allowed = has_bypass
+            || {
+                let projects: Vec<_> = service.iter_user_projects(&user.id).await?.collect();
+                let internal_allowed = projects.contains(&scope);
+
+                let permit_allowed = service
+                    .permit_client
+                    .allowed(
+                        &user.id,
+                        &service.find_project_by_name(&scope).await?.id,
+                        "develop", // TODO?: make this configurable per endpoint?
+                    )
+                    .await
+                    .map_err(|_| {
+                        error!("failed to check Permit permission");
+                        // Error::from_kind(ErrorKind::Internal)
+                    })
+                    .unwrap_or_default();
+
+                if internal_allowed != permit_allowed {
+                    error!(
+                        "PERMIT: Permissions for user {} project {} did not match internal permissions. Internal: {}, Permit: {}",
+                        user.id,
+                        scope,
+                        internal_allowed,
+                        permit_allowed
+                    );
+                }
+
+                internal_allowed
+            };
+
+        if allowed {
             Ok(Self { user, scope })
         } else {
             Err(Error::from(ErrorKind::ProjectNotFound(scope.to_string())))

diff --git a/gateway/src/project.rs b/gateway/src/project.rs
@@ -1845,7 +1845,12 @@ pub mod exec {
             .await
             .expect("could not list projects")
         {
-            match gateway.find_project(&project_name).await.unwrap().state {
+            match gateway
+                .find_project_by_name(&project_name)
+                .await
+                .unwrap()
+                .state
+            {
                 Project::Errored(ProjectError { ctx: Some(ctx), .. }) => {
                     if let Some(container) = ctx.container() {
                         if let Ok(container) = gateway
@@ -1938,8 +1943,11 @@ pub mod exec {
             .await
             .expect("could not list cch projects")
         {
-            if let Project::Ready(ProjectReady { container, .. }) =
-                gateway.find_project(&project_name).await.unwrap().state
+            if let Project::Ready(ProjectReady { container, .. }) = gateway
+                .find_project_by_name(&project_name)
+                .await
+                .unwrap()
+                .state
             {
                 if let Ok(container) = gateway
                     .context()