Skip to content

Commit 0b97911

Browse files
authored
fix(resource-recorder)!: disable service id endpoint (#1644)
1 parent b637bef commit 0b97911

File tree

13 files changed

+217
-148
lines changed

13 files changed

+217
-148
lines changed

common/src/backends/client/gateway.rs

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ impl Client {
3636
/// Interact with all the data relating to projects
3737
#[allow(async_fn_in_trait)]
3838
pub trait ProjectsDal {
39+
/// Get a user project
40+
async fn get_user_project(
41+
&self,
42+
user_token: &str,
43+
project_name: &str,
44+
) -> Result<models::project::Response, Error>;
45+
46+
/// Check the HEAD of a user project
47+
async fn head_user_project(&self, user_token: &str, project_name: &str) -> Result<bool, Error>;
48+
3949
/// Get the projects that belong to a user
4050
async fn get_user_projects(
4151
&self,
@@ -56,22 +66,49 @@ pub trait ProjectsDal {
5666
}
5767

5868
impl ProjectsDal for Client {
69+
#[instrument(skip_all)]
70+
async fn get_user_project(
71+
&self,
72+
user_token: &str,
73+
project_name: &str,
74+
) -> Result<models::project::Response, Error> {
75+
self.public_client
76+
.request(
77+
Method::GET,
78+
format!("projects/{}", project_name).as_str(),
79+
None::<()>,
80+
Some(Authorization::bearer(user_token).expect("to build an authorization bearer")),
81+
)
82+
.await
83+
}
84+
85+
#[instrument(skip_all)]
86+
async fn head_user_project(&self, user_token: &str, project_name: &str) -> Result<bool, Error> {
87+
self.public_client
88+
.request_raw(
89+
Method::HEAD,
90+
format!("projects/{}", project_name).as_str(),
91+
None::<()>,
92+
Some(Authorization::bearer(user_token).expect("to build an authorization bearer")),
93+
)
94+
.await?;
95+
96+
Ok(true)
97+
}
98+
5999
#[instrument(skip_all)]
60100
async fn get_user_projects(
61101
&self,
62102
user_token: &str,
63103
) -> Result<Vec<models::project::Response>, Error> {
64-
let projects = self
65-
.public_client
104+
self.public_client
66105
.request(
67106
Method::GET,
68107
"projects",
69108
None::<()>,
70109
Some(Authorization::bearer(user_token).expect("to build an authorization bearer")),
71110
)
72-
.await?;
73-
74-
Ok(projects)
111+
.await
75112
}
76113
}
77114

common/src/backends/client/mod.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use bytes::Bytes;
12
use headers::{ContentType, Header, HeaderMapExt};
23
use http::{Method, Request, StatusCode, Uri};
34
use hyper::{body, client::HttpConnector, Body, Client};
@@ -36,38 +37,48 @@ pub struct ServicesApiClient {
3637
}
3738

3839
impl ServicesApiClient {
39-
/// Make a new client that connects to the given endpoint
4040
fn new(base: Uri) -> Self {
4141
Self {
4242
client: Client::new(),
4343
base,
4444
}
4545
}
4646

47-
/// Make a get request to a path on the service
4847
pub async fn request<B: Serialize, T: DeserializeOwned, H: Header>(
4948
&self,
5049
method: Method,
5150
path: &str,
5251
body: Option<B>,
5352
extra_header: Option<H>,
5453
) -> Result<T, Error> {
54+
let bytes = self.request_raw(method, path, body, extra_header).await?;
55+
let json = serde_json::from_slice(&bytes)?;
56+
57+
Ok(json)
58+
}
59+
60+
pub async fn request_raw<B: Serialize, H: Header>(
61+
&self,
62+
method: Method,
63+
path: &str,
64+
body: Option<B>,
65+
extra_header: Option<H>,
66+
) -> Result<Bytes, Error> {
5567
let uri = format!("{}{path}", self.base);
5668
trace!(uri, "calling inner service");
5769

5870
let mut req = Request::builder().method(method).uri(uri);
5971
let headers = req
6072
.headers_mut()
6173
.expect("new request to have mutable headers");
62-
63-
headers.typed_insert(ContentType::json());
64-
6574
if let Some(extra_header) = extra_header {
6675
headers.typed_insert(extra_header);
6776
}
77+
if body.is_some() {
78+
headers.typed_insert(ContentType::json());
79+
}
6880

6981
let cx = Span::current().context();
70-
7182
global::get_text_map_propagator(|propagator| {
7283
propagator.inject_context(&cx, &mut HeaderInjector(req.headers_mut().unwrap()))
7384
});
@@ -79,18 +90,15 @@ impl ServicesApiClient {
7990
};
8091

8192
let resp = self.client.request(req?).await?;
82-
8393
trace!(response = ?resp, "Load response");
8494

8595
if resp.status() != StatusCode::OK {
8696
return Err(Error::RequestError(resp.status()));
8797
}
8898

89-
let body = resp.into_body();
90-
let bytes = body::to_bytes(body).await?;
91-
let json = serde_json::from_slice(&bytes)?;
99+
let bytes = body::to_bytes(resp.into_body()).await?;
92100

93-
Ok(json)
101+
Ok(bytes)
94102
}
95103
}
96104

common/src/backends/mod.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use tracing::instrument;
22

3-
use crate::claims::{Claim, Scope};
3+
use crate::claims::{AccountTier, Claim, Scope};
44

55
use self::client::{ProjectsDal, ResourceDal};
66

@@ -17,6 +17,7 @@ pub mod trace;
1717
pub trait ClaimExt {
1818
/// Verify that the [Claim] has the [Scope::Admin] scope.
1919
fn is_admin(&self) -> bool;
20+
fn is_deployer(&self) -> bool;
2021
/// Verify that the user's current project count is lower than the account limit in [Claim::limits].
2122
fn can_create_project(&self, current_count: u32) -> bool;
2223
/// Verify that the user has permission to provision RDS instances.
@@ -31,12 +32,21 @@ pub trait ClaimExt {
3132
projects_dal: &G,
3233
project_name: &str,
3334
) -> Result<bool, client::Error>;
35+
/// Verify if the claim subject has ownership of a project.
36+
async fn owns_project_id<G: ProjectsDal>(
37+
&self,
38+
projects_dal: &G,
39+
project_id: &str,
40+
) -> Result<bool, client::Error>;
3441
}
3542

3643
impl ClaimExt for Claim {
3744
fn is_admin(&self) -> bool {
3845
self.scopes.contains(&Scope::Admin)
3946
}
47+
fn is_deployer(&self) -> bool {
48+
self.tier == AccountTier::Deployer
49+
}
4050

4151
fn can_create_project(&self, current_count: u32) -> bool {
4252
self.is_admin() || self.limits.project_limit() > current_count
@@ -71,7 +81,17 @@ impl ClaimExt for Claim {
7181
project_name: &str,
7282
) -> Result<bool, client::Error> {
7383
let token = self.token.as_ref().expect("token to be set");
74-
let projects = projects_dal.get_user_projects(token).await?;
75-
Ok(projects.iter().any(|project| project.name == project_name))
84+
projects_dal.head_user_project(token, project_name).await
85+
}
86+
87+
#[instrument(skip_all)]
88+
async fn owns_project_id<G: ProjectsDal>(
89+
&self,
90+
projects_dal: &G,
91+
project_id: &str,
92+
) -> Result<bool, client::Error> {
93+
let token = self.token.as_ref().expect("token to be set");
94+
let projects = projects_dal.get_user_project_ids(token).await?;
95+
Ok(projects.iter().any(|id| id == project_id))
7696
}
7797
}

deployer/src/persistence/mod.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use shuttle_common::{claims::Claim, resource::Type};
88
use shuttle_proto::{
99
provisioner::{self, DatabaseRequest},
1010
resource_recorder::{
11-
self, record_request, RecordRequest, ResourceIds, ResourceResponse, ResourcesResponse,
12-
ResultResponse, ServiceResourcesRequest,
11+
self, record_request, ProjectResourcesRequest, RecordRequest, ResourceIds,
12+
ResourceResponse, ResourcesResponse, ResultResponse,
1313
},
1414
};
1515
use sqlx::{
@@ -364,17 +364,18 @@ impl ResourceManager for Persistence {
364364
service_id: &Ulid,
365365
claim: Claim,
366366
) -> Result<ResourcesResponse> {
367-
let mut req = tonic::Request::new(ServiceResourcesRequest {
368-
service_id: service_id.to_string(),
367+
let mut req = tonic::Request::new(ProjectResourcesRequest {
368+
project_id: self.project_id.to_string(),
369369
});
370+
370371
req.extensions_mut().insert(claim.clone());
371372

372-
info!(%service_id, "Getting resources from resource-recorder");
373+
info!(%self.project_id, "Getting resources from resource-recorder");
373374
let res = self
374375
.resource_recorder_client
375376
.as_mut()
376377
.expect("to have the resource recorder set up")
377-
.get_service_resources(req)
378+
.get_project_resources(req)
378379
.await
379380
.map_err(PersistenceError::ResourceRecorder)
380381
.map(|res| res.into_inner())?;
@@ -384,8 +385,7 @@ impl ResourceManager for Persistence {
384385
info!("Got no resources from resource-recorder");
385386
// Check if there are cached resources on the local persistence.
386387
let resources: std::result::Result<Vec<Resource>, sqlx::Error> =
387-
sqlx::query_as("SELECT * FROM resources WHERE service_id = ?")
388-
.bind(service_id.to_string())
388+
sqlx::query_as("SELECT * FROM resources")
389389
.fetch_all(&self.pool)
390390
.await;
391391

@@ -410,17 +410,18 @@ impl ResourceManager for Persistence {
410410
self.insert_resources(local_resources, service_id, claim.clone())
411411
.await?;
412412

413-
let mut req = tonic::Request::new(ServiceResourcesRequest {
414-
service_id: service_id.to_string(),
413+
let mut req = tonic::Request::new(ProjectResourcesRequest {
414+
project_id: self.project_id.to_string(),
415415
});
416+
416417
req.extensions_mut().insert(claim);
417418

418419
info!("Getting resources from resource-recorder again");
419420
let res = self
420421
.resource_recorder_client
421422
.as_mut()
422423
.expect("to have the resource recorder set up")
423-
.get_service_resources(req)
424+
.get_project_resources(req)
424425
.await
425426
.map_err(PersistenceError::ResourceRecorder)
426427
.map(|res| res.into_inner())?;
@@ -433,8 +434,7 @@ impl ResourceManager for Persistence {
433434
info!("Deleting local resources");
434435
// Now that we know that the resources are in resource-recorder,
435436
// we can safely delete them from here to prevent de-sync issues and to not hinder project deletion
436-
sqlx::query("DELETE FROM resources WHERE service_id = ?")
437-
.bind(service_id.to_string())
437+
sqlx::query("DELETE FROM resources")
438438
.execute(&self.pool)
439439
.await?;
440440

proto/resource-recorder.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ service ResourceRecorder {
1010
// Get the resources belonging to a project
1111
rpc GetProjectResources(ProjectResourcesRequest) returns (ResourcesResponse);
1212

13-
// Get the resources belonging to a service
13+
// Discontinued
1414
rpc GetServiceResources(ServiceResourcesRequest) returns (ResourcesResponse);
1515

1616
// Get a resource

proto/src/generated/resource_recorder.rs

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

provisioner/src/lib.rs

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use rand::Rng;
1414
use shuttle_common::backends::auth::VerifyClaim;
1515
use shuttle_common::backends::client::gateway;
1616
use shuttle_common::backends::ClaimExt;
17-
use shuttle_common::claims::Scope;
17+
use shuttle_common::claims::{Claim, Scope};
1818
use shuttle_common::models::project::ProjectName;
1919
pub use shuttle_proto::provisioner::provisioner_server::ProvisionerServer;
2020
use shuttle_proto::provisioner::{
@@ -460,6 +460,21 @@ impl ShuttleProvisioner {
460460

461461
Ok(DatabaseDeletionResponse {})
462462
}
463+
464+
async fn verify_ownership(&self, claim: &Claim, project_name: &str) -> Result<(), Status> {
465+
if !claim.is_admin()
466+
&& !claim.is_deployer()
467+
&& !claim
468+
.owns_project(&self.gateway_client, project_name)
469+
.await
470+
.map_err(|_| Status::internal("could not verify project ownership"))?
471+
{
472+
let status = Status::permission_denied("the request lacks the authorizations");
473+
error!(error = &status as &dyn std::error::Error);
474+
return Err(status);
475+
}
476+
Ok(())
477+
}
463478
}
464479

465480
#[tonic::async_trait]
@@ -470,24 +485,12 @@ impl Provisioner for ShuttleProvisioner {
470485
request: Request<DatabaseRequest>,
471486
) -> Result<Response<DatabaseResponse>, Status> {
472487
request.verify(Scope::ResourcesWrite)?;
473-
474488
let claim = request.get_claim()?;
475-
476489
let request = request.into_inner();
477490
if !ProjectName::is_valid(&request.project_name) {
478491
return Err(Status::invalid_argument("invalid project name"));
479492
}
480-
481-
// Check project ownership.
482-
if !claim
483-
.owns_project(&self.gateway_client, &request.project_name)
484-
.await
485-
.map_err(|_| Status::internal("can not verify project ownership"))?
486-
{
487-
let status = Status::permission_denied("the request lacks the authorizations");
488-
error!(error = &status as &dyn std::error::Error);
489-
return Err(status);
490-
}
493+
self.verify_ownership(&claim, &request.project_name).await?;
491494

492495
let db_type = request.db_type.unwrap();
493496

@@ -539,22 +542,11 @@ impl Provisioner for ShuttleProvisioner {
539542
) -> Result<Response<DatabaseDeletionResponse>, Status> {
540543
request.verify(Scope::ResourcesWrite)?;
541544
let claim = request.get_claim()?;
542-
543545
let request = request.into_inner();
544546
if !ProjectName::is_valid(&request.project_name) {
545547
return Err(Status::invalid_argument("invalid project name"));
546548
}
547-
548-
// Check project ownership.
549-
if !claim
550-
.owns_project(&self.gateway_client, &request.project_name)
551-
.await
552-
.map_err(|_| Status::internal("can not verify project ownership"))?
553-
{
554-
let status = Status::permission_denied("the request lacks the authorizations");
555-
error!(error = &status as &dyn std::error::Error);
556-
return Err(status);
557-
}
549+
self.verify_ownership(&claim, &request.project_name).await?;
558550

559551
let db_type = request.db_type.unwrap();
560552

0 commit comments

Comments
 (0)