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
41 changes: 25 additions & 16 deletions gateway/src/api/latest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,25 +127,34 @@ 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,
Query(PaginationDetails { limit, .. }): Query<PaginationDetails>,
) -> 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))?
.into_iter()
.take(limit as usize)
{
let name = p.resource.expect("project resource").key;
let project = service.find_project(name.as_str()).await?;
let idle_minutes = project.state.idle_minutes();

let response = project::Response {
id: project.project_id,
name,
state: project.state.into(),
idle_minutes,
};
projects.push(response);
}

Ok(AxumJson(projects))
}
Expand Down Expand Up @@ -391,7 +400,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
26 changes: 18 additions & 8 deletions gateway/src/auth.rs
Original file line number Diff line number Diff line change
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,20 @@ 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);

#[allow(clippy::blocks_in_if_conditions)]
if user.claim.is_admin()
|| user.claim.is_deployer()
|| service
.permit_client
.allowed(&user.id, &scope.to_string(), "develop") // TODO?: make this configurable per endpoint?
.await
.map_err(|_| {
error!("failed to check Permit permission");
Error::from_kind(ErrorKind::Internal)
})?
{
Ok(Self { user, scope })
} else {
Err(Error::from(ErrorKind::ProjectNotFound(scope.to_string())))
Expand Down