Skip to content

Commit cbeec1a

Browse files
committed
unclaimed
1 parent 0272f81 commit cbeec1a

File tree

3 files changed

+89
-66
lines changed

3 files changed

+89
-66
lines changed

lib/api_run/src/run.rs

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ use bencher_endpoint::{CorsResponse, Endpoint, Post, ResponseCreated};
22
use bencher_json::{JsonNewRun, JsonReport, ResourceName, RunContext, Slug};
33
use bencher_rbac::project::Permission;
44
use bencher_schema::{
5+
conn_lock,
56
context::ApiContext,
6-
error::{bad_request_error, forbidden_error, issue_error},
7+
error::{bad_request_error, unauthorized_error},
78
model::{
89
project::{report::QueryReport, QueryProject},
910
user::auth::{AuthUser, PubBearerToken},
@@ -48,40 +49,29 @@ async fn post_inner(
4849
auth_user: Option<AuthUser>,
4950
json_run: JsonNewRun,
5051
) -> Result<JsonReport, HttpError> {
51-
let query_project = match (auth_user.as_ref(), json_run.project.as_ref()) {
52-
(Some(auth_user), Some(project)) => {
53-
QueryProject::get_or_create(log, context, auth_user, project)
54-
.await
55-
.map_err(|e| forbidden_error(e.to_string()))?
56-
},
57-
(Some(auth_user), None) => {
58-
let project_name = project_name(&json_run)?;
59-
let project_slug = project_slug(&json_run)?;
60-
QueryProject::get_or_create_from_context(
61-
log,
62-
context,
63-
auth_user,
64-
project_name,
65-
project_slug,
66-
)
67-
.await
68-
.map_err(|e| forbidden_error(e.to_string()))?
69-
},
70-
_ => return Err(bad_request_error("Not yet supported")),
52+
let project_name_fn = || project_name(&json_run);
53+
let project_slug_fn = || project_slug(&json_run);
54+
let query_project = if let Some(project) = json_run.project.as_ref() {
55+
QueryProject::get_or_create(log, context, auth_user.as_ref(), project, project_name_fn)
56+
.await?
57+
} else {
58+
QueryProject::get_or_create_from_context(
59+
log,
60+
context,
61+
auth_user.as_ref(),
62+
project_name_fn,
63+
project_slug_fn,
64+
)
65+
.await?
7166
};
7267

73-
// Verify that the user is allowed
74-
// This should always succeed if the logic above is correct
7568
if let Some(auth_user) = auth_user.as_ref() {
76-
query_project
77-
.try_allowed(&context.rbac, auth_user, Permission::Create)
78-
.map_err(|e| {
79-
issue_error(
80-
"Failed to check run permissions",
81-
"Failed check the run permissions before creating a report",
82-
e,
83-
)
84-
})?;
69+
query_project.try_allowed(&context.rbac, auth_user, Permission::Create)?;
70+
} else if !query_project.is_unclaimed(conn_lock!(context))? {
71+
return Err(unauthorized_error(format!(
72+
"This project ({}) has already been claimed.",
73+
query_project.slug
74+
)));
8575
}
8676
QueryReport::create(
8777
log,

lib/bencher_schema/src/model/organization/mod.rs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use bencher_json::{
88
DateTime, JsonNewOrganization, JsonOrganization, Jwt, OrganizationUuid, ResourceId,
99
ResourceName, Slug,
1010
};
11-
use bencher_rbac::Organization;
11+
use bencher_rbac::{organization::Permission, Organization};
1212
use diesel::{ExpressionMethods, QueryDsl, Queryable, RunQueryDsl};
1313
use dropshot::HttpError;
1414
use organization_role::{InsertOrganizationRole, QueryOrganizationRole};
@@ -62,6 +62,7 @@ impl QueryOrganization {
6262
if let Ok(query_organization) =
6363
Self::from_resource_id(conn_lock!(context), &user_slug.clone().into())
6464
{
65+
query_organization.try_allowed(&context.rbac, auth_user, Permission::View)?;
6566
return Ok(query_organization);
6667
}
6768

@@ -72,21 +73,17 @@ impl QueryOrganization {
7273
Self::create(context, auth_user, json_organization).await
7374
}
7475

75-
pub async fn get_or_create_from_context(
76+
pub async fn get_or_create_from_project(
7677
context: &ApiContext,
7778
project_name: &ResourceName,
7879
project_slug: &Slug,
7980
) -> Result<Self, HttpError> {
8081
if let Ok(query_organization) =
8182
Self::from_resource_id(conn_lock!(context), &project_slug.clone().into())
8283
{
83-
// Get the total number of members for the organization
84-
let total_members =
85-
QueryOrganizationRole::count(conn_lock!(context), query_organization.id)?;
86-
// If the project is part of an organization that has zero members,
84+
// If the project is part of an organization that is unclaimed,
8785
// then the project can have anonymous reports.
88-
// That is, the project has not yet been claimed.
89-
return if total_members == 0 {
86+
return if query_organization.is_unclaimed(conn_lock!(context))? {
9087
Ok(query_organization)
9188
} else {
9289
Err(unauthorized_error(format!(
@@ -147,7 +144,7 @@ impl QueryOrganization {
147144
rbac: &Rbac,
148145
organization: &ResourceId,
149146
auth_user: &AuthUser,
150-
permission: bencher_rbac::organization::Permission,
147+
permission: Permission,
151148
) -> Result<Self, HttpError> {
152149
// Do not leak information about organizations.
153150
// Always return the same error.
@@ -161,7 +158,7 @@ impl QueryOrganization {
161158
rbac: &Rbac,
162159
organization: &ResourceId,
163160
auth_user: &AuthUser,
164-
permission: bencher_rbac::organization::Permission,
161+
permission: Permission,
165162
) -> Result<Self, HttpError> {
166163
let query_organization = Self::from_resource_id(conn, organization)?;
167164
query_organization.try_allowed(rbac, auth_user, permission)?;
@@ -173,7 +170,7 @@ impl QueryOrganization {
173170
rbac: &Rbac,
174171
organization_id: OrganizationId,
175172
auth_user: &AuthUser,
176-
permission: bencher_rbac::organization::Permission,
173+
permission: Permission,
177174
) -> Result<Self, HttpError> {
178175
// Do not leak information about organizations.
179176
// Always return the same error.
@@ -189,7 +186,7 @@ impl QueryOrganization {
189186
rbac: &Rbac,
190187
organization_id: OrganizationId,
191188
auth_user: &AuthUser,
192-
permission: bencher_rbac::organization::Permission,
189+
permission: Permission,
193190
) -> Result<Self, HttpError> {
194191
let query_organization = Self::get(conn, organization_id)?;
195192
query_organization.try_allowed(rbac, auth_user, permission)?;
@@ -200,12 +197,18 @@ impl QueryOrganization {
200197
&self,
201198
rbac: &Rbac,
202199
auth_user: &AuthUser,
203-
permission: bencher_rbac::organization::Permission,
200+
permission: Permission,
204201
) -> Result<(), HttpError> {
205202
rbac.is_allowed_organization(auth_user, permission, self)
206203
.map_err(forbidden_error)
207204
}
208205

206+
pub fn is_unclaimed(&self, conn: &mut DbConnection) -> Result<bool, HttpError> {
207+
let total_members = QueryOrganizationRole::count(conn, self.id)?;
208+
// If the organization that has zero members, then it is unclaimed.
209+
Ok(total_members == 0)
210+
}
211+
209212
pub fn into_json(self) -> JsonOrganization {
210213
let Self {
211214
uuid,

lib/bencher_schema/src/model/project/mod.rs

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,16 @@ impl QueryProject {
8989
.map_err(resource_not_found_err!(Project, slug.clone()))
9090
}
9191

92-
pub async fn get_or_create(
92+
pub async fn get_or_create<NameFn>(
9393
log: &Logger,
9494
context: &ApiContext,
95-
auth_user: &AuthUser,
95+
auth_user: Option<&AuthUser>,
9696
project: &ResourceId,
97-
) -> Result<Self, HttpError> {
97+
project_name_fn: NameFn,
98+
) -> Result<Self, HttpError>
99+
where
100+
NameFn: FnOnce() -> Result<ResourceName, HttpError>,
101+
{
98102
let query_project = Self::from_resource_id(conn_lock!(context), project);
99103

100104
let http_error = match query_project {
@@ -105,35 +109,52 @@ impl QueryProject {
105109
let Ok(kind) = ResourceIdKind::try_from(project) else {
106110
return Err(http_error);
107111
};
108-
let slug = match kind {
112+
let project_slug = match kind {
109113
ResourceIdKind::Uuid(_) => return Err(http_error),
110114
ResourceIdKind::Slug(slug) => slug,
111115
};
112116

113-
let query_organization =
114-
QueryOrganization::get_or_create_from_user(context, auth_user).await?;
115-
let json_project = JsonNewProject {
116-
name: slug.clone().into(),
117-
slug: Some(slug.clone()),
118-
url: None,
119-
visibility: None,
120-
};
121-
Self::create(log, context, auth_user, &query_organization, json_project).await
117+
Self::get_or_create_inner(log, context, auth_user, project_name_fn, project_slug).await
122118
}
123119

124-
pub async fn get_or_create_from_context(
120+
pub async fn get_or_create_from_context<NameFn, SlugFn>(
125121
log: &Logger,
126122
context: &ApiContext,
127-
auth_user: &AuthUser,
128-
project_name: ResourceName,
129-
project_slug: Slug,
130-
) -> Result<Self, HttpError> {
123+
auth_user: Option<&AuthUser>,
124+
project_name_fn: NameFn,
125+
project_slug_fn: SlugFn,
126+
) -> Result<Self, HttpError>
127+
where
128+
NameFn: FnOnce() -> Result<ResourceName, HttpError>,
129+
SlugFn: FnOnce() -> Result<Slug, HttpError>,
130+
{
131+
let project_slug = project_slug_fn()?;
131132
if let Ok(query_project) = Self::from_slug(conn_lock!(context), &project_slug) {
132133
return Ok(query_project);
133134
}
134135

135-
let query_organization =
136-
QueryOrganization::get_or_create_from_user(context, auth_user).await?;
136+
Self::get_or_create_inner(log, context, auth_user, project_name_fn, project_slug).await
137+
}
138+
139+
async fn get_or_create_inner<NameFn>(
140+
log: &Logger,
141+
context: &ApiContext,
142+
auth_user: Option<&AuthUser>,
143+
project_name_fn: NameFn,
144+
project_slug: Slug,
145+
) -> Result<Self, HttpError>
146+
where
147+
NameFn: FnOnce() -> Result<ResourceName, HttpError>,
148+
{
149+
let project_name = project_name_fn()?;
150+
let query_organization = if let Some(auth_user) = auth_user {
151+
QueryOrganization::get_or_create_from_user(context, auth_user).await?
152+
} else {
153+
QueryOrganization::get_or_create_from_project(context, &project_name, &project_slug)
154+
.await?
155+
};
156+
// The choice was either to relax the schema constraint to allow duplicate project names
157+
// or to append a number to the project name to ensure uniqueness.
137158
let name = Self::unique_name(context, &query_organization, project_name).await?;
138159
let json_project = JsonNewProject {
139160
name,
@@ -142,7 +163,11 @@ impl QueryProject {
142163
visibility: None,
143164
};
144165

145-
Self::create(log, context, auth_user, &query_organization, json_project).await
166+
if let Some(auth_user) = auth_user {
167+
Self::create(log, context, auth_user, &query_organization, json_project).await
168+
} else {
169+
Self::create_inner(log, context, &query_organization, json_project).await
170+
}
146171
}
147172

148173
async fn unique_name(
@@ -368,6 +393,11 @@ impl QueryProject {
368393
.map_err(forbidden_error)
369394
}
370395

396+
pub fn is_unclaimed(&self, conn: &mut DbConnection) -> Result<bool, HttpError> {
397+
let query_organization = QueryOrganization::get(conn, self.organization_id)?;
398+
query_organization.is_unclaimed(conn)
399+
}
400+
371401
#[cfg(feature = "plus")]
372402
pub fn perf_url(&self, console_url: &url::Url) -> Result<Option<url::Url>, HttpError> {
373403
if !self.is_public() {

0 commit comments

Comments
 (0)