Skip to content

Commit ec653b0

Browse files
Sh099078Khoyo
andcommitted
editoast: ignore user headers when skip-authz is present
Update whoami endpoint response accordingly. The special handling was required when editoast could start with --disable-authorization. It is not possible for the frontend to skip authorization with the `x-osrd-skip-authz` header as it will be stripped by the gateway. Co-authored-by: Younes Khoudli <younes.khoudli@epita.fr> Signed-off-by: Younes Khoudli <younes.khoudli@epita.fr> Signed-off-by: Loup Federico <16464925+Sh099078@users.noreply.github.com>
1 parent 38edf85 commit ec653b0

3 files changed

Lines changed: 17 additions & 62 deletions

File tree

editoast/src/authentication.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,7 @@ impl State {
138138
let user = user.expect("providing the request user is required when Mode::Authenticated or Mode::Impersonating");
139139
Ok(State::Authenticated { user, roles })
140140
}
141-
Mode::Skip { .. } => {
142-
// TODO review: Arguably the skip header should prevent any authentication info from being used
143-
// however this would change whoami behavior for Tartine RMI
144-
if let Some(user) = user {
145-
Ok(State::Authenticated { user, roles })
146-
} else {
147-
Ok(State::Skip)
148-
}
149-
}
141+
Mode::Skip { .. } => Ok(State::Skip),
150142
}
151143
}
152144
}

editoast/src/views/authz.rs

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -130,31 +130,18 @@ pub(in crate::views) struct WhoamiResponse {
130130
))
131131
)]
132132
pub(in crate::views) async fn whoami(
133-
Extension(authn): Extension<crate::authentication::Mode>,
134133
Extension(authn_state): Extension<crate::authentication::State>,
135134
) -> Result<Json<WhoamiResponse>> {
136-
let skip = matches!(authn, crate::authentication::Mode::Skip { .. });
137-
if let Some(editoast_models::User { id, name }) = authn_state.user() {
138-
let mut roles = HashSet::from_iter(authn_state.roles().iter().copied());
139-
// Authorization is skipped by header, but identity headers are provided so we could fetch
140-
// the user's roles, though Admin may be lacking.
141-
if skip {
142-
roles.insert(Role::Admin);
135+
match authn_state {
136+
crate::authentication::State::Skip => Err(AuthorizationError::Unauthenticated)?,
137+
crate::authentication::State::Authenticated { user, roles } => {
138+
let roles = HashSet::from_iter(roles);
139+
Ok(Json(WhoamiResponse {
140+
id: user.id,
141+
name: user.name,
142+
roles,
143+
}))
143144
}
144-
Ok(Json(WhoamiResponse {
145-
id: *id,
146-
name: name.clone(),
147-
roles,
148-
}))
149-
} else if skip {
150-
// TODO: don't return -1 and a hardcoded name, return a different schema instead, requires frontend changes
151-
Ok(Json(WhoamiResponse {
152-
id: -1,
153-
name: "OSRD user".to_string(),
154-
roles: HashSet::from([Role::Admin]),
155-
}))
156-
} else {
157-
Err(AuthorizationError::Forbidden.into())
158145
}
159146
}
160147

@@ -1832,38 +1819,23 @@ mod tests {
18321819
.create()
18331820
.await;
18341821

1835-
let user_data = app
1836-
.get("/authz/me")
1822+
app.get("/authz/me")
18371823
.by_user(user.as_ref())
18381824
.skip_authz()
18391825
.await
1840-
.assert_status_ok()
1841-
.json::<WhoamiResponse>();
1842-
1843-
assert_eq!(
1844-
user_data,
1845-
WhoamiResponse {
1846-
id: user.id,
1847-
name: "Bob".to_string(),
1848-
roles: HashSet::from([Role::Admin]),
1849-
}
1850-
);
1826+
.assert_status_unauthorized();
18511827
}
18521828

18531829
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
18541830
async fn whoami_skip_with_unprivileged_user_info() {
18551831
let app = test_app!().build();
18561832
let user = app.user("test", "test").create().await;
18571833

1858-
let WhoamiResponse { roles, .. } = app
1859-
.get("/authz/me")
1834+
app.get("/authz/me")
18601835
.by_user(user.as_ref())
18611836
.skip_authz()
18621837
.await
1863-
.assert_status_ok()
1864-
.json::<WhoamiResponse>();
1865-
1866-
assert_eq!(roles, HashSet::from([Role::Admin]));
1838+
.assert_status_unauthorized();
18671839
}
18681840

18691841
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -1990,23 +1962,13 @@ mod tests {
19901962
async fn user_groups_skip_authz() {
19911963
let app = test_app!().build();
19921964
let user = app.user("test", "test").create().await;
1993-
let group = app.group("group").with_members([&user]).create().await;
19941965

19951966
// With a user in the request
1996-
let groups = app
1997-
.get("/authz/me/groups")
1967+
app.get("/authz/me/groups")
19981968
.by_user(user.as_ref())
19991969
.skip_authz()
20001970
.await
2001-
.assert_status_ok()
2002-
.json::<Vec<Group>>();
2003-
assert_eq!(
2004-
groups,
2005-
vec![editoast_models::Group {
2006-
id: group.id,
2007-
name: "group".to_owned(),
2008-
}]
2009-
);
1971+
.assert_status_unauthorized();
20101972

20111973
// Without a user in the request
20121974
app.get("/authz/me/groups")

editoast/src/views/server/middlewares.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ pub(in crate::views) async fn authentication_validation_middleware(
239239
let state = crate::authentication::State::try_new(authn, user, roles.clone())?;
240240
span.record("authz.state", tracing::field::debug(&state));
241241
req.extensions_mut().insert(state);
242+
req.extensions_mut().remove::<crate::authentication::Mode>();
242243
Ok(next.run(req).await)
243244
}
244245

0 commit comments

Comments
 (0)