Skip to content

Commit d8b5e16

Browse files
authored
Accept personal access tokens on the Admin API (#5183)
2 parents eeba7e1 + e5a54f2 commit d8b5e16

File tree

8 files changed

+194
-56
lines changed

8 files changed

+194
-56
lines changed

crates/data-model/src/users.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ impl User {
3030
pub fn is_valid(&self) -> bool {
3131
self.locked_at.is_none() && self.deactivated_at.is_none()
3232
}
33+
34+
/// Returns `true` if the user is a valid actor, for example
35+
/// of a personal session.
36+
///
37+
/// Currently: this is `true` unless the user is deactivated.
38+
///
39+
/// This is a weaker form of validity: `is_valid` always implies
40+
/// `is_valid_actor`, but some users (currently: locked users)
41+
/// can be valid actors for personal sessions but aren't valid
42+
/// except through administrative access.
43+
#[must_use]
44+
pub fn is_valid_actor(&self) -> bool {
45+
self.deactivated_at.is_none()
46+
}
3347
}
3448

3549
impl User {

crates/handlers/src/activity_tracker/bound.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
use std::net::IpAddr;
88

9-
use mas_data_model::{BrowserSession, Clock, CompatSession, Session};
9+
use mas_data_model::{
10+
BrowserSession, Clock, CompatSession, Session, personal::session::PersonalSession,
11+
};
1012

1113
use crate::activity_tracker::ActivityTracker;
1214

@@ -37,6 +39,13 @@ impl Bound {
3739
.await;
3840
}
3941

42+
/// Record activity in a personal session.
43+
pub async fn record_personal_session(&self, clock: &dyn Clock, session: &PersonalSession) {
44+
self.tracker
45+
.record_personal_session(clock, session, self.ip)
46+
.await;
47+
}
48+
4049
/// Record activity in a compatibility session.
4150
pub async fn record_compat_session(&self, clock: &dyn Clock, session: &CompatSession) {
4251
self.tracker

crates/handlers/src/activity_tracker/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ impl ActivityTracker {
113113
}
114114
}
115115

116-
/// Record activity in a personal access token session.
117-
pub async fn record_personal_access_token_session(
116+
/// Record activity in a personal session.
117+
pub async fn record_personal_session(
118118
&self,
119119
clock: &dyn Clock,
120120
session: &PersonalSession,

crates/handlers/src/admin/call_context.rs

Lines changed: 140 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ use axum_extra::TypedHeader;
1616
use headers::{Authorization, authorization::Bearer};
1717
use hyper::StatusCode;
1818
use mas_axum_utils::record_error;
19-
use mas_data_model::{BoxClock, Session, User};
19+
use mas_data_model::{
20+
BoxClock, Session, TokenFormatError, TokenType, User,
21+
personal::session::{PersonalSession, PersonalSessionOwner},
22+
};
2023
use mas_storage::{BoxRepository, RepositoryError};
24+
use oauth2_types::scope::Scope;
2125
use ulid::Ulid;
2226

2327
use super::response::ErrorResponse;
@@ -41,6 +45,10 @@ pub enum Rejection {
4145
#[error("Invalid repository operation")]
4246
Repository(#[from] RepositoryError),
4347

48+
/// The access token was not of the correct type for the Admin API
49+
#[error("Invalid type of access token")]
50+
InvalidAccessTokenType(#[from] Option<TokenFormatError>),
51+
4452
/// The access token could not be found in the database
4553
#[error("Unknown access token")]
4654
UnknownAccessToken,
@@ -90,7 +98,8 @@ impl IntoResponse for Rejection {
9098
| Rejection::TokenExpired
9199
| Rejection::SessionRevoked
92100
| Rejection::UserLocked
93-
| Rejection::MissingScope => StatusCode::UNAUTHORIZED,
101+
| Rejection::MissingScope
102+
| Rejection::InvalidAccessTokenType(_) => StatusCode::UNAUTHORIZED,
94103

95104
Rejection::RepositorySetup(_)
96105
| Rejection::Repository(_)
@@ -113,7 +122,7 @@ pub struct CallContext {
113122
pub repo: BoxRepository,
114123
pub clock: BoxClock,
115124
pub user: Option<User>,
116-
pub session: Session,
125+
pub session: CallerSession,
117126
}
118127

119128
impl<S> FromRequestParts<S> for CallContext
@@ -154,56 +163,126 @@ where
154163
})?;
155164

156165
let token = token.token();
166+
let token_type = TokenType::check(token)?;
167+
168+
let session = match token_type {
169+
TokenType::AccessToken => {
170+
// Look for the access token in the database
171+
let token = repo
172+
.oauth2_access_token()
173+
.find_by_token(token)
174+
.await?
175+
.ok_or(Rejection::UnknownAccessToken)?;
176+
177+
// Look for the associated session in the database
178+
let session = repo
179+
.oauth2_session()
180+
.lookup(token.session_id)
181+
.await?
182+
.ok_or_else(|| Rejection::LoadSession(token.session_id))?;
183+
184+
if !session.is_valid() {
185+
return Err(Rejection::SessionRevoked);
186+
}
187+
188+
if !token.is_valid(clock.now()) {
189+
return Err(Rejection::TokenExpired);
190+
}
157191

158-
// Look for the access token in the database
159-
let token = repo
160-
.oauth2_access_token()
161-
.find_by_token(token)
162-
.await?
163-
.ok_or(Rejection::UnknownAccessToken)?;
164-
165-
// Look for the associated session in the database
166-
let session = repo
167-
.oauth2_session()
168-
.lookup(token.session_id)
169-
.await?
170-
.ok_or_else(|| Rejection::LoadSession(token.session_id))?;
171-
172-
// Record the activity on the session
173-
activity_tracker
174-
.record_oauth2_session(&clock, &session)
175-
.await;
192+
// Record the activity on the session
193+
activity_tracker
194+
.record_oauth2_session(&clock, &session)
195+
.await;
196+
197+
CallerSession::OAuth2Session(session)
198+
}
199+
TokenType::PersonalAccessToken => {
200+
// Look for the access token in the database
201+
let token = repo
202+
.personal_access_token()
203+
.find_by_token(token)
204+
.await?
205+
.ok_or(Rejection::UnknownAccessToken)?;
206+
207+
// Look for the associated session in the database
208+
let session = repo
209+
.personal_session()
210+
.lookup(token.session_id)
211+
.await?
212+
.ok_or_else(|| Rejection::LoadSession(token.session_id))?;
213+
214+
if !session.is_valid() {
215+
return Err(Rejection::SessionRevoked);
216+
}
217+
218+
if !token.is_valid(clock.now()) {
219+
return Err(Rejection::TokenExpired);
220+
}
221+
222+
// Check the validity of the owner of the personal session
223+
match session.owner {
224+
PersonalSessionOwner::User(owner_user_id) => {
225+
let owner_user = repo
226+
.user()
227+
.lookup(owner_user_id)
228+
.await?
229+
.ok_or_else(|| Rejection::LoadUser(owner_user_id))?;
230+
if !owner_user.is_valid() {
231+
return Err(Rejection::UserLocked);
232+
}
233+
}
234+
PersonalSessionOwner::OAuth2Client(_) => {
235+
// nop: Client owners are always valid
236+
}
237+
}
238+
239+
// Record the activity on the session
240+
activity_tracker
241+
.record_personal_session(&clock, &session)
242+
.await;
243+
244+
CallerSession::PersonalSession(session)
245+
}
246+
_other => {
247+
return Err(Rejection::InvalidAccessTokenType(None));
248+
}
249+
};
176250

177251
// Load the user if there is one
178-
let user = if let Some(user_id) = session.user_id {
252+
let user = if let Some(user_id) = session.user_id() {
179253
let user = repo
180254
.user()
181255
.lookup(user_id)
182256
.await?
183257
.ok_or_else(|| Rejection::LoadUser(user_id))?;
258+
259+
match session {
260+
CallerSession::OAuth2Session(_) => {
261+
// For OAuth2 sessions: check that the user is valid enough
262+
// to be a user.
263+
if !user.is_valid() {
264+
return Err(Rejection::UserLocked);
265+
}
266+
}
267+
CallerSession::PersonalSession(_) => {
268+
// For personal sessions: check that the actor is valid enough
269+
// to be an actor.
270+
if !user.is_valid_actor() {
271+
return Err(Rejection::UserLocked);
272+
}
273+
}
274+
}
275+
184276
Some(user)
185277
} else {
278+
// Double check we're not using a PersonalSession
279+
assert!(matches!(session, CallerSession::OAuth2Session(_)));
186280
None
187281
};
188282

189-
// If there is a user for this session, check that it is not locked
190-
if let Some(user) = &user
191-
&& !user.is_valid()
192-
{
193-
return Err(Rejection::UserLocked);
194-
}
195-
196-
if !session.is_valid() {
197-
return Err(Rejection::SessionRevoked);
198-
}
199-
200-
if !token.is_valid(clock.now()) {
201-
return Err(Rejection::TokenExpired);
202-
}
203-
204283
// For now, we only check that the session has the admin scope
205284
// Later we might want to check other route-specific scopes
206-
if !session.scope.contains("urn:mas:admin") {
285+
if !session.scope().contains("urn:mas:admin") {
207286
return Err(Rejection::MissingScope);
208287
}
209288

@@ -215,3 +294,26 @@ where
215294
})
216295
}
217296
}
297+
298+
/// The session representing the caller of the Admin API;
299+
/// could either be an OAuth session or a personal session.
300+
pub enum CallerSession {
301+
OAuth2Session(Session),
302+
PersonalSession(PersonalSession),
303+
}
304+
305+
impl CallerSession {
306+
pub fn scope(&self) -> &Scope {
307+
match self {
308+
CallerSession::OAuth2Session(session) => &session.scope,
309+
CallerSession::PersonalSession(session) => &session.scope,
310+
}
311+
}
312+
313+
pub fn user_id(&self) -> Option<Ulid> {
314+
match self {
315+
CallerSession::OAuth2Session(session) => session.user_id,
316+
CallerSession::PersonalSession(session) => Some(session.actor_user_id),
317+
}
318+
}
319+
}

crates/handlers/src/admin/v1/personal_sessions/add.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use axum::{Json, response::IntoResponse};
88
use chrono::Duration;
99
use hyper::StatusCode;
1010
use mas_axum_utils::record_error;
11-
use mas_data_model::{BoxRng, TokenType, personal::session::PersonalSessionOwner};
11+
use mas_data_model::{BoxRng, TokenType};
1212
use oauth2_types::scope::Scope;
1313
use schemars::JsonSchema;
1414
use serde::Deserialize;
@@ -19,6 +19,7 @@ use crate::{
1919
call_context::CallContext,
2020
model::{InconsistentPersonalSession, PersonalSession},
2121
response::{ErrorResponse, SingleResponse},
22+
v1::personal_sessions::personal_session_owner_from_caller,
2223
},
2324
impl_from_error_for_route,
2425
};
@@ -100,13 +101,7 @@ pub async fn handler(
100101
NoApi(mut rng): NoApi<BoxRng>,
101102
Json(params): Json<Request>,
102103
) -> Result<(StatusCode, Json<SingleResponse<PersonalSession>>), RouteError> {
103-
let owner = if let Some(user_id) = session.user_id {
104-
// User-owned session
105-
PersonalSessionOwner::User(user_id)
106-
} else {
107-
// No admin user means this is a client-owned session
108-
PersonalSessionOwner::OAuth2Client(session.client_id)
109-
};
104+
let owner = personal_session_owner_from_caller(&session);
110105

111106
let actor_user = repo
112107
.user()

crates/handlers/src/admin/v1/personal_sessions/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,31 @@ mod list;
99
mod regenerate;
1010
mod revoke;
1111

12+
use mas_data_model::personal::session::PersonalSessionOwner;
13+
1214
pub use self::{
1315
add::{doc as add_doc, handler as add},
1416
get::{doc as get_doc, handler as get},
1517
list::{doc as list_doc, handler as list},
1618
regenerate::{doc as regenerate_doc, handler as regenerate},
1719
revoke::{doc as revoke_doc, handler as revoke},
1820
};
21+
use crate::admin::call_context::CallerSession;
22+
23+
/// Given the [`CallerSession`] of a caller of the Admin API,
24+
/// return the [`PersonalSessionOwner`] that should own created personal
25+
/// sessions.
26+
fn personal_session_owner_from_caller(caller: &CallerSession) -> PersonalSessionOwner {
27+
match caller {
28+
CallerSession::OAuth2Session(session) => {
29+
if let Some(user_id) = session.user_id {
30+
PersonalSessionOwner::User(user_id)
31+
} else {
32+
PersonalSessionOwner::OAuth2Client(session.client_id)
33+
}
34+
}
35+
CallerSession::PersonalSession(session) => {
36+
PersonalSessionOwner::User(session.actor_user_id)
37+
}
38+
}
39+
}

crates/handlers/src/admin/v1/personal_sessions/regenerate.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use axum::{Json, response::IntoResponse};
88
use chrono::Duration;
99
use hyper::StatusCode;
1010
use mas_axum_utils::record_error;
11-
use mas_data_model::{BoxRng, TokenType, personal::session::PersonalSessionOwner};
11+
use mas_data_model::{BoxRng, TokenType};
1212
use schemars::JsonSchema;
1313
use serde::Deserialize;
1414
use tracing::error;
@@ -19,6 +19,7 @@ use crate::{
1919
model::{InconsistentPersonalSession, PersonalSession},
2020
params::UlidPathParam,
2121
response::{ErrorResponse, SingleResponse},
22+
v1::personal_sessions::personal_session_owner_from_caller,
2223
},
2324
impl_from_error_for_route,
2425
};
@@ -111,11 +112,7 @@ pub async fn handler(
111112

112113
// If the owner is not the current caller, then currently we reject the
113114
// regeneration.
114-
let caller = if let Some(user_id) = caller_session.user_id {
115-
PersonalSessionOwner::User(user_id)
116-
} else {
117-
PersonalSessionOwner::OAuth2Client(caller_session.client_id)
118-
};
115+
let caller = personal_session_owner_from_caller(&caller_session);
119116
if session.owner != caller {
120117
return Err(RouteError::SessionNotYours);
121118
}

crates/handlers/src/oauth2/introspection.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@ pub(crate) async fn post(
700700
};
701701

702702
activity_tracker
703-
.record_personal_access_token_session(&clock, &session, ip)
703+
.record_personal_session(&clock, &session, ip)
704704
.await;
705705

706706
INTROSPECTION_COUNTER.add(

0 commit comments

Comments
 (0)