Skip to content

Commit 387d65d

Browse files
committed
WIP
1 parent 4e9e8ed commit 387d65d

File tree

3 files changed

+274
-6
lines changed

3 files changed

+274
-6
lines changed

src/auth.rs

+176-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::controllers;
22
use crate::db::RequestTransaction;
33
use crate::middleware::log_request;
4+
use crate::models::token::{CrateScope, EndpointScope};
45
use crate::models::{ApiToken, User};
56
use crate::util::errors::{
67
account_locked, forbidden, internal, AppError, AppResult, InsecurelyGeneratedTokenRevoked,
@@ -13,17 +14,43 @@ use http::header;
1314
#[derive(Debug, Clone)]
1415
pub struct AuthCheck {
1516
allow_token: bool,
17+
endpoint_scope: Option<EndpointScope>,
18+
crate_name: Option<String>,
1619
}
1720

1821
impl AuthCheck {
1922
#[must_use]
2023
pub fn default() -> Self {
21-
Self { allow_token: true }
24+
Self {
25+
allow_token: true,
26+
endpoint_scope: None,
27+
crate_name: None,
28+
}
2229
}
2330

2431
#[must_use]
2532
pub fn only_cookie() -> Self {
26-
Self { allow_token: false }
33+
Self {
34+
allow_token: false,
35+
endpoint_scope: None,
36+
crate_name: None,
37+
}
38+
}
39+
40+
pub fn with_endpoint_scope(&self, endpoint_scope: EndpointScope) -> Self {
41+
Self {
42+
allow_token: self.allow_token,
43+
endpoint_scope: Some(endpoint_scope),
44+
crate_name: self.crate_name.clone(),
45+
}
46+
}
47+
48+
pub fn for_crate(&self, crate_name: &str) -> Self {
49+
Self {
50+
allow_token: self.allow_token,
51+
endpoint_scope: self.endpoint_scope,
52+
crate_name: Some(crate_name.to_string()),
53+
}
2754
}
2855

2956
pub fn check(&self, request: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
@@ -47,13 +74,57 @@ impl AuthCheck {
4774
log_request::add_custom_metadata("tokenid", id);
4875
}
4976

50-
if !self.allow_token && auth.token.is_some() {
51-
let error_message = "API Token authentication was explicitly disallowed for this API";
52-
return Err(internal(error_message).chain(forbidden()));
77+
if let Some(ref token) = auth.token {
78+
if !self.allow_token {
79+
let error_message =
80+
"API Token authentication was explicitly disallowed for this API";
81+
return Err(internal(error_message).chain(forbidden()));
82+
}
83+
84+
if !self.endpoint_scope_matches(token.endpoint_scopes.as_ref()) {
85+
let error_message = "Endpoint scope mismatch";
86+
return Err(internal(error_message).chain(forbidden()));
87+
}
88+
89+
if !self.crate_scope_matches(token.crate_scopes.as_ref()) {
90+
let error_message = "Crate scope mismatch";
91+
return Err(internal(error_message).chain(forbidden()));
92+
}
5393
}
5494

5595
Ok(auth)
5696
}
97+
98+
fn endpoint_scope_matches(&self, token_scopes: Option<&Vec<EndpointScope>>) -> bool {
99+
match (&token_scopes, &self.endpoint_scope) {
100+
// The token is a legacy token.
101+
(None, _) => true,
102+
103+
// The token is NOT a legacy token, and the endpoint only allows legacy tokens.
104+
(Some(_), None) => false,
105+
106+
// The token is NOT a legacy token, and the endpoint allows a certain endpoint scope or a legacy token.
107+
(Some(token_scopes), Some(endpoint_scope)) => token_scopes.contains(endpoint_scope),
108+
}
109+
}
110+
111+
fn crate_scope_matches(&self, token_scopes: Option<&Vec<CrateScope>>) -> bool {
112+
match (&token_scopes, &self.crate_name) {
113+
// The token is a legacy token.
114+
(None, _) => true,
115+
116+
// The token does not have any crate scopes.
117+
(Some(token_scopes), _) if token_scopes.is_empty() => true,
118+
119+
// The token has crate scopes, but the endpoint does not deal with crates.
120+
(Some(_), None) => false,
121+
122+
// The token is NOT a legacy token, and the endpoint allows a certain endpoint scope or a legacy token.
123+
(Some(token_scopes), Some(crate_name)) => token_scopes
124+
.iter()
125+
.any(|token_scope| token_scope.matches(crate_name)),
126+
}
127+
}
57128
}
58129

59130
#[derive(Debug)]
@@ -120,3 +191,103 @@ fn authenticate_user(req: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
120191
// Unable to authenticate the user
121192
return Err(internal("no cookie session or auth header found").chain(forbidden()));
122193
}
194+
195+
#[cfg(test)]
196+
mod tests {
197+
use super::*;
198+
199+
fn cs(scope: &str) -> CrateScope {
200+
CrateScope::try_from(scope).unwrap()
201+
}
202+
203+
#[test]
204+
fn regular_endpoint() {
205+
let auth_check = AuthCheck::default();
206+
207+
assert!(auth_check.endpoint_scope_matches(None));
208+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
209+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
210+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
211+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
212+
213+
assert!(auth_check.crate_scope_matches(None));
214+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
215+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
216+
}
217+
218+
#[test]
219+
fn publish_new_endpoint() {
220+
let auth_check = AuthCheck::default()
221+
.with_endpoint_scope(EndpointScope::PublishNew)
222+
.for_crate("tokio-console");
223+
224+
assert!(auth_check.endpoint_scope_matches(None));
225+
assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
226+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
227+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
228+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
229+
230+
assert!(auth_check.crate_scope_matches(None));
231+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
232+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
233+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
234+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
235+
}
236+
237+
#[test]
238+
fn publish_update_endpoint() {
239+
let auth_check = AuthCheck::default()
240+
.with_endpoint_scope(EndpointScope::PublishUpdate)
241+
.for_crate("tokio-console");
242+
243+
assert!(auth_check.endpoint_scope_matches(None));
244+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
245+
assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
246+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
247+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
248+
249+
assert!(auth_check.crate_scope_matches(None));
250+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
251+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
252+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
253+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
254+
}
255+
256+
#[test]
257+
fn yank_endpoint() {
258+
let auth_check = AuthCheck::default()
259+
.with_endpoint_scope(EndpointScope::Yank)
260+
.for_crate("tokio-console");
261+
262+
assert!(auth_check.endpoint_scope_matches(None));
263+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
264+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
265+
assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
266+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
267+
268+
assert!(auth_check.crate_scope_matches(None));
269+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
270+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
271+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
272+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
273+
}
274+
275+
#[test]
276+
fn owner_change_endpoint() {
277+
let auth_check = AuthCheck::default()
278+
.with_endpoint_scope(EndpointScope::ChangeOwners)
279+
.for_crate("tokio-console");
280+
281+
assert!(auth_check.endpoint_scope_matches(None));
282+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishNew])));
283+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::PublishUpdate])));
284+
assert!(!auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::Yank])));
285+
assert!(auth_check.endpoint_scope_matches(Some(&vec![EndpointScope::ChangeOwners])));
286+
287+
assert!(auth_check.crate_scope_matches(None));
288+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-console")])));
289+
assert!(auth_check.crate_scope_matches(Some(&vec![cs("tokio-*")])));
290+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
291+
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
292+
}
293+
}

src/controllers/krate/owners.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use crate::auth::AuthCheck;
44
use crate::controllers::prelude::*;
5+
use crate::models::token::EndpointScope;
56
use crate::models::{Crate, Owner, Rights, Team, User};
67
use crate::views::EncodableOwner;
78

@@ -80,7 +81,12 @@ fn parse_owners_request(req: &mut dyn RequestExt) -> AppResult<Vec<String>> {
8081
}
8182

8283
fn modify_owners(req: &mut dyn RequestExt, add: bool) -> EndpointResult {
83-
let auth = AuthCheck::default().check(req)?;
84+
let crate_name = &req.params()["crate_id"];
85+
86+
let auth = AuthCheck::default()
87+
.with_endpoint_scope(EndpointScope::ChangeOwners)
88+
.for_crate(crate_name)
89+
.check(req)?;
8490

8591
let logins = parse_owners_request(req)?;
8692
let app = req.app();

src/tests/owners.rs

+91
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use cargo_registry::{
1414
Emails,
1515
};
1616

17+
use cargo_registry::models::token::{CrateScope, EndpointScope};
1718
use chrono::{Duration, Utc};
1819
use conduit::StatusCode;
1920
use diesel::prelude::*;
@@ -305,6 +306,96 @@ fn owner_change_via_token() {
305306
);
306307
}
307308

309+
#[test]
310+
fn owner_change_via_change_owner_token() {
311+
let (app, _, _, token) =
312+
TestApp::full().with_scoped_token(None, Some(vec![EndpointScope::ChangeOwners]));
313+
314+
let user2 = app.db_new_user("user-2");
315+
let user2 = user2.as_model();
316+
317+
let krate =
318+
app.db(|conn| CrateBuilder::new("foo_crate", token.as_model().user_id).expect_build(conn));
319+
320+
let url = format!("/api/v1/crates/{}/owners", krate.name);
321+
let body = json!({ "owners": [user2.gh_login] });
322+
let body = serde_json::to_vec(&body).unwrap();
323+
let response = token.put::<()>(&url, &body);
324+
assert_eq!(response.status(), StatusCode::OK);
325+
assert_eq!(
326+
response.into_json(),
327+
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
328+
);
329+
}
330+
331+
#[test]
332+
fn owner_change_via_change_owner_token_with_matching_crate_scope() {
333+
let crate_scopes = Some(vec![CrateScope::try_from("foo_crate").unwrap()]);
334+
let endpoint_scopes = Some(vec![EndpointScope::ChangeOwners]);
335+
let (app, _, _, token) = TestApp::full().with_scoped_token(crate_scopes, endpoint_scopes);
336+
337+
let user2 = app.db_new_user("user-2");
338+
let user2 = user2.as_model();
339+
340+
let krate =
341+
app.db(|conn| CrateBuilder::new("foo_crate", token.as_model().user_id).expect_build(conn));
342+
343+
let url = format!("/api/v1/crates/{}/owners", krate.name);
344+
let body = json!({ "owners": [user2.gh_login] });
345+
let body = serde_json::to_vec(&body).unwrap();
346+
let response = token.put::<()>(&url, &body);
347+
assert_eq!(response.status(), StatusCode::OK);
348+
assert_eq!(
349+
response.into_json(),
350+
json!({ "ok": true, "msg": "user user-2 has been invited to be an owner of crate foo_crate" })
351+
);
352+
}
353+
354+
#[test]
355+
fn owner_change_via_change_owner_token_with_wrong_crate_scope() {
356+
let crate_scopes = Some(vec![CrateScope::try_from("bar").unwrap()]);
357+
let endpoint_scopes = Some(vec![EndpointScope::ChangeOwners]);
358+
let (app, _, _, token) = TestApp::full().with_scoped_token(crate_scopes, endpoint_scopes);
359+
360+
let user2 = app.db_new_user("user-2");
361+
let user2 = user2.as_model();
362+
363+
let krate =
364+
app.db(|conn| CrateBuilder::new("foo_crate", token.as_model().user_id).expect_build(conn));
365+
366+
let url = format!("/api/v1/crates/{}/owners", krate.name);
367+
let body = json!({ "owners": [user2.gh_login] });
368+
let body = serde_json::to_vec(&body).unwrap();
369+
let response = token.put::<()>(&url, &body);
370+
assert_eq!(response.status(), StatusCode::FORBIDDEN);
371+
assert_eq!(
372+
response.into_json(),
373+
json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
374+
);
375+
}
376+
377+
#[test]
378+
fn owner_change_via_publish_token() {
379+
let (app, _, _, token) =
380+
TestApp::full().with_scoped_token(None, Some(vec![EndpointScope::PublishUpdate]));
381+
382+
let user2 = app.db_new_user("user-2");
383+
let user2 = user2.as_model();
384+
385+
let krate =
386+
app.db(|conn| CrateBuilder::new("foo_crate", token.as_model().user_id).expect_build(conn));
387+
388+
let url = format!("/api/v1/crates/{}/owners", krate.name);
389+
let body = json!({ "owners": [user2.gh_login] });
390+
let body = serde_json::to_vec(&body).unwrap();
391+
let response = token.put::<()>(&url, &body);
392+
assert_eq!(response.status(), StatusCode::FORBIDDEN);
393+
assert_eq!(
394+
response.into_json(),
395+
json!({ "errors": [{ "detail": "must be logged in to perform that action" }] })
396+
);
397+
}
398+
308399
#[test]
309400
fn owner_change_without_auth() {
310401
let (app, anon, cookie) = TestApp::full().with_user();

0 commit comments

Comments
 (0)