Skip to content

Commit bf4dca8

Browse files
committed
models/user: Move User::rights() fn into controllers::helpers module
1 parent 09656a3 commit bf4dca8

File tree

10 files changed

+75
-68
lines changed

10 files changed

+75
-68
lines changed

src/controllers/crate_owner_invitation.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use crate::app::AppState;
22
use crate::auth::AuthCheck;
33
use crate::auth::Authentication;
4+
use crate::controllers::helpers::authorization::Rights;
45
use crate::controllers::helpers::pagination::{Page, PaginationOptions, PaginationQueryParams};
56
use crate::models::crate_owner_invitation::AcceptError;
6-
use crate::models::{Crate, CrateOwnerInvitation, Rights, User};
7+
use crate::models::{Crate, CrateOwnerInvitation, User};
78
use crate::schema::{crate_owner_invitations, crates, users};
89
use crate::util::errors::{bad_request, custom, forbidden, internal, AppResult, BoxedAppError};
910
use crate::util::RequestUtils;
@@ -157,7 +158,7 @@ async fn prepare_list(
157158
// Only allow crate owners to query pending invitations for their crate.
158159
let krate: Crate = Crate::by_name(&crate_name).first(conn).await?;
159160
let owners = krate.owners(conn).await?;
160-
if user.rights(&*state.github, &owners).await? != Rights::Full {
161+
if Rights::get(user, &*state.github, &owners).await? != Rights::Full {
161162
let detail = "only crate owners can query pending invitations for their crate";
162163
return Err(forbidden(detail));
163164
}

src/controllers/helpers.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::util::errors::AppResult;
22
use axum::response::{IntoResponse, Response};
33
use axum_extra::json;
44

5+
pub mod authorization;
56
pub(crate) mod pagination;
67

78
pub(crate) use self::pagination::Paginate;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
use crate::models::{Owner, User};
2+
use crates_io_github::{GitHubClient, GitHubError};
3+
use oauth2::AccessToken;
4+
use secrecy::ExposeSecret;
5+
6+
/// Access rights to the crate (publishing and ownership management)
7+
/// NOTE: The order of these variants matters!
8+
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Copy)]
9+
pub enum Rights {
10+
None,
11+
Publish,
12+
Full,
13+
}
14+
15+
impl Rights {
16+
/// Given this set of owners, determines the strongest rights the
17+
/// user has.
18+
///
19+
/// Short-circuits on `Full` because you can't beat it. In practice, we'll always
20+
/// see `[user, user, user, ..., team, team, team]`, so we could shortcircuit on
21+
/// `Publish` as well, but this is a non-obvious invariant so we don't bother.
22+
/// Sweet free optimization if teams are proving burdensome to check.
23+
/// More than one team isn't really expected, though.
24+
pub async fn get(
25+
user: &User,
26+
gh_client: &dyn GitHubClient,
27+
owners: &[Owner],
28+
) -> Result<Self, GitHubError> {
29+
let token = AccessToken::new(user.gh_access_token.expose_secret().to_string());
30+
31+
let mut best = Self::None;
32+
for owner in owners {
33+
match *owner {
34+
Owner::User(ref other_user) => {
35+
if other_user.id == user.id {
36+
return Ok(Self::Full);
37+
}
38+
}
39+
Owner::Team(ref team) => {
40+
// Phones home to GitHub to ask if this User is a member of the given team.
41+
// Note that we're assuming that the given user is the one interested in
42+
// the answer. If this is not the case, then we could accidentally leak
43+
// private membership information here.
44+
let is_team_member = gh_client
45+
.team_membership(team.org_id, team.github_id, &user.gh_login, &token)
46+
.await?
47+
.is_some_and(|m| m.is_active());
48+
49+
if is_team_member {
50+
best = Self::Publish;
51+
}
52+
}
53+
}
54+
}
55+
Ok(best)
56+
}
57+
}

src/controllers/krate/delete.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use crate::app::AppState;
22
use crate::auth::AuthCheck;
3+
use crate::controllers::helpers::authorization::Rights;
34
use crate::controllers::krate::CratePath;
45
use crate::email::Email;
5-
use crate::models::{NewDeletedCrate, Rights};
6+
use crate::models::NewDeletedCrate;
67
use crate::schema::{crate_downloads, crates, dependencies};
78
use crate::util::errors::{custom, AppResult, BoxedAppError};
89
use crate::worker::jobs;
@@ -68,7 +69,7 @@ pub async fn delete_crate(
6869
// Check that the user is an owner of the crate (team owners are not allowed to delete crates)
6970
let user = auth.user();
7071
let owners = krate.owners(&mut conn).await?;
71-
match user.rights(&*app.github, &owners).await? {
72+
match Rights::get(user, &*app.github, &owners).await? {
7273
Rights::Full => {}
7374
Rights::Publish => {
7475
let msg = "team members don't have permission to delete crates";

src/controllers/krate/owners.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
//! All routes related to managing owners of a crate
22
3+
use crate::controllers::helpers::authorization::Rights;
34
use crate::controllers::krate::CratePath;
45
use crate::models::krate::OwnerRemoveError;
56
use crate::models::{
67
krate::NewOwnerInvite, token::EndpointScope, CrateOwner, NewCrateOwnerInvitation,
78
NewCrateOwnerInvitationOutcome, NewTeam,
89
};
9-
use crate::models::{Crate, Owner, Rights, Team, User};
10+
use crate::models::{Crate, Owner, Team, User};
1011
use crate::util::errors::{bad_request, crate_not_found, custom, AppResult, BoxedAppError};
1112
use crate::views::EncodableOwner;
1213
use crate::{app::AppState, App};
@@ -177,7 +178,7 @@ async fn modify_owners(
177178

178179
let owners = krate.owners(conn).await?;
179180

180-
match user.rights(&*app.github, &owners).await? {
181+
match Rights::get(user, &*app.github, &owners).await? {
181182
Rights::Full => {}
182183
// Yes!
183184
Rights::Publish => {

src/controllers/krate/publish.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ use url::Url;
2828

2929
use crate::models::{
3030
default_versions::Version as DefaultVersion, Category, Crate, DependencyKind, Keyword,
31-
NewCrate, NewVersion, NewVersionOwnerAction, Rights, VersionAction,
31+
NewCrate, NewVersion, NewVersionOwnerAction, VersionAction,
3232
};
3333

34+
use crate::controllers::helpers::authorization::Rights;
3435
use crate::licenses::parse_license_expr;
3536
use crate::middleware::log_request::RequestLogExt;
3637
use crate::models::token::EndpointScope;
@@ -356,7 +357,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
356357
};
357358

358359
let owners = krate.owners(conn).await?;
359-
if user.rights(&*app.github, &owners).await? < Rights::Publish {
360+
if Rights::get(user, &*app.github, &owners).await? < Rights::Publish {
360361
return Err(custom(StatusCode::FORBIDDEN, MISSING_RIGHTS_ERROR_MESSAGE));
361362
}
362363

src/controllers/version/update.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use super::CrateVersionPath;
22
use crate::app::AppState;
33
use crate::auth::{AuthCheck, Authentication};
4+
use crate::controllers::helpers::authorization::Rights;
45
use crate::models::token::EndpointScope;
5-
use crate::models::{
6-
Crate, NewVersionOwnerAction, Rights, Version, VersionAction, VersionOwnerAction,
7-
};
6+
use crate::models::{Crate, NewVersionOwnerAction, Version, VersionAction, VersionOwnerAction};
87
use crate::rate_limiter::LimitedAction;
98
use crate::schema::versions;
109
use crate::util::errors::{bad_request, custom, AppResult};
@@ -122,7 +121,7 @@ pub async fn perform_version_yank_update(
122121

123122
let yanked = yanked.unwrap_or(version.yanked);
124123

125-
if user.rights(&*state.github, &owners).await? < Rights::Publish {
124+
if Rights::get(user, &*state.github, &owners).await? < Rights::Publish {
126125
if user.is_admin {
127126
let action = if yanked { "yanking" } else { "unyanking" };
128127
warn!(

src/models.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ pub use self::follow::Follow;
1212
pub use self::keyword::{CrateKeyword, Keyword};
1313
pub use self::krate::{Crate, CrateName, NewCrate, RecentCrateDownloads};
1414
pub use self::owner::{CrateOwner, Owner, OwnerKind};
15-
pub use self::rights::Rights;
1615
pub use self::team::{NewTeam, Team};
1716
pub use self::token::ApiToken;
1817
pub use self::user::{NewUser, User};
@@ -32,7 +31,6 @@ mod follow;
3231
mod keyword;
3332
pub mod krate;
3433
mod owner;
35-
mod rights;
3634
pub mod team;
3735
pub mod token;
3836
pub mod user;

src/models/rights.rs

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/models/user.rs

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,11 @@ use diesel::prelude::*;
55
use diesel::sql_types::Integer;
66
use diesel::upsert::excluded;
77
use diesel_async::{AsyncPgConnection, RunQueryDsl};
8-
use oauth2::AccessToken;
9-
use secrecy::{ExposeSecret, SecretString};
8+
use secrecy::SecretString;
109

11-
use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind, Rights};
10+
use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind};
1211
use crate::schema::{crate_owners, emails, users};
1312
use crates_io_diesel_helpers::lower;
14-
use crates_io_github::{GitHubClient, GitHubError};
1513

1614
/// The model representing a row in the `users` database table.
1715
#[derive(Clone, Debug, Queryable, Identifiable, Selectable)]
@@ -56,48 +54,6 @@ impl User {
5654
Ok(users.collect())
5755
}
5856

59-
/// Given this set of owners, determines the strongest rights the
60-
/// user has.
61-
///
62-
/// Shortcircuits on `Full` because you can't beat it. In practice we'll always
63-
/// see `[user, user, user, ..., team, team, team]`, so we could shortcircuit on
64-
/// `Publish` as well, but this is a non-obvious invariant so we don't bother.
65-
/// Sweet free optimization if teams are proving burdensome to check.
66-
/// More than one team isn't really expected, though.
67-
pub async fn rights(
68-
&self,
69-
gh_client: &dyn GitHubClient,
70-
owners: &[Owner],
71-
) -> Result<Rights, GitHubError> {
72-
let token = AccessToken::new(self.gh_access_token.expose_secret().to_string());
73-
74-
let mut best = Rights::None;
75-
for owner in owners {
76-
match *owner {
77-
Owner::User(ref other_user) => {
78-
if other_user.id == self.id {
79-
return Ok(Rights::Full);
80-
}
81-
}
82-
Owner::Team(ref team) => {
83-
// Phones home to GitHub to ask if this User is a member of the given team.
84-
// Note that we're assuming that the given user is the one interested in
85-
// the answer. If this is not the case, then we could accidentally leak
86-
// private membership information here.
87-
let is_team_member = gh_client
88-
.team_membership(team.org_id, team.github_id, &self.gh_login, &token)
89-
.await?
90-
.is_some_and(|m| m.is_active());
91-
92-
if is_team_member {
93-
best = Rights::Publish;
94-
}
95-
}
96-
}
97-
}
98-
Ok(best)
99-
}
100-
10157
/// Queries the database for the verified emails
10258
/// belonging to a given user
10359
pub async fn verified_email(

0 commit comments

Comments
 (0)