Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from Apr 3, 2024
3 changes: 0 additions & 3 deletions backends/tests/integration/permit_tests.rs
Expand Up @@ -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,
Expand All @@ -100,8 +99,6 @@ mod needs_docker {

Wrap(client)
}

async fn teardown(self) {}
}

#[test_context(Wrap)]
Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yml
Expand Up @@ -282,6 +282,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:
Expand Down
57 changes: 31 additions & 26 deletions gateway/src/api/latest.rs
Expand Up @@ -104,11 +104,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,
Expand All @@ -127,25 +127,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))
}
Expand Down Expand Up @@ -197,7 +203,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,
Expand All @@ -216,11 +222,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,
Expand Down Expand Up @@ -264,10 +270,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();
Expand Down Expand Up @@ -307,7 +312,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));
Expand Down Expand Up @@ -391,7 +396,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);
Expand Down
49 changes: 41 additions & 8 deletions gateway/src/auth.rs
Expand Up @@ -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};
Expand All @@ -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,
}
Expand All @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to still have a User struct? We can just implement FromRequestParts for the claim right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only traits defined in the current crate can be implemented for a type parameter
Claim can technically be moved to backends (it seems), but I'd skip it for now.


// 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,
};

Expand Down Expand Up @@ -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())))
Expand Down
14 changes: 11 additions & 3 deletions gateway/src/project.rs
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down