1
0
mirror of https://github.com/matrix-org/matrix-authentication-service.git synced 2025-07-29 22:01:14 +03:00

data-model: Make the user_id optional in the OAuth 2.0 sessions

This commit is contained in:
Quentin Gliech
2023-09-04 16:32:43 +02:00
parent 3691090757
commit 7e247830c9
16 changed files with 133 additions and 85 deletions

View File

@ -66,7 +66,7 @@ pub struct Session {
pub id: Ulid,
pub state: SessionState,
pub created_at: DateTime<Utc>,
pub user_id: Ulid,
pub user_id: Option<Ulid>,
pub user_session_id: Option<Ulid>,
pub client_id: Ulid,
pub scope: Scope,

View File

@ -63,7 +63,7 @@ pub enum Requester {
BrowserSession(BrowserSession),
/// The requester is a OAuth2 session, with an access token.
OAuth2Session(Session, User),
OAuth2Session(Session, Option<User>),
}
trait OwnerId {
@ -90,7 +90,7 @@ impl OwnerId for mas_data_model::UserEmail {
impl OwnerId for Session {
fn owner_id(&self) -> Option<Ulid> {
Some(self.user_id)
self.user_id
}
}
@ -126,7 +126,7 @@ impl Requester {
fn user(&self) -> Option<&User> {
match self {
Self::BrowserSession(session) => Some(&session.user),
Self::OAuth2Session(_session, user) => Some(user),
Self::OAuth2Session(_session, user) => user.as_ref(),
Self::Anonymous => None,
}
}

View File

@ -22,7 +22,7 @@ use ulid::Ulid;
use url::Url;
use super::{BrowserSession, NodeType, User};
use crate::state::ContextExt;
use crate::{state::ContextExt, UserId};
/// The state of an OAuth 2.0 session.
#[derive(Enum, Copy, Clone, Eq, PartialEq)]
@ -108,17 +108,25 @@ impl OAuth2Session {
}
/// User authorized for this session.
pub async fn user(&self, ctx: &Context<'_>) -> Result<User, async_graphql::Error> {
pub async fn user(&self, ctx: &Context<'_>) -> Result<Option<User>, async_graphql::Error> {
let state = ctx.state();
let Some(user_id) = self.0.user_id else {
return Ok(None);
};
if !ctx.requester().is_owner_or_admin(&UserId(user_id)) {
return Err(async_graphql::Error::new("Unauthorized"));
}
let mut repo = state.repository().await?;
let user = repo
.user()
.lookup(self.0.user_id)
.lookup(user_id)
.await?
.context("Could not load user")?;
repo.cancel().await?;
Ok(User(user))
Ok(Some(User(user)))
}
}

View File

@ -96,24 +96,26 @@ impl OAuth2SessionMutations {
return Ok(EndOAuth2SessionPayload::NotFound);
}
let user = repo
.user()
.lookup(session.user_id)
.await?
.context("Could not load user")?;
if let Some(user_id) = session.user_id {
let user = repo
.user()
.lookup(user_id)
.await?
.context("Could not load user")?;
// Scan the scopes of the session to find if there is any device that should be
// deleted from the Matrix server.
// TODO: this should be moved in a higher level "end oauth session" method.
// XXX: this might not be the right semantic, but it's the best we
// can do for now, since we're not explicitly storing devices for OAuth2
// sessions.
for scope in &*session.scope {
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
// Scan the scopes of the session to find if there is any device that should be
// deleted from the Matrix server.
// TODO: this should be moved in a higher level "end oauth session" method.
// XXX: this might not be the right semantic, but it's the best we
// can do for now, since we're not explicitly storing devices for OAuth2
// sessions.
for scope in &*session.scope {
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
}
}
}

View File

@ -31,8 +31,8 @@ impl ViewerQuery {
match requester {
Requester::BrowserSession(session) => Viewer::user(session.user.clone()),
Requester::OAuth2Session(_session, user) => Viewer::user(user.clone()),
Requester::Anonymous => Viewer::anonymous(),
Requester::OAuth2Session(_session, Some(user)) => Viewer::user(user.clone()),
Requester::OAuth2Session(_, None) | Requester::Anonymous => Viewer::anonymous(),
}
}

View File

@ -29,6 +29,7 @@ use futures_util::TryStreamExt;
use headers::{authorization::Bearer, Authorization, ContentType, HeaderValue};
use hyper::header::CACHE_CONTROL;
use mas_axum_utils::{cookies::CookieJar, FancyError, SessionInfo, SessionInfoExt};
use mas_data_model::User;
use mas_graphql::{Requester, Schema};
use mas_matrix::HomeserverConnection;
use mas_policy::{InstantiateError, Policy, PolicyFactory};
@ -204,13 +205,22 @@ async fn get_requester(
.await?
.ok_or(RouteError::LoadFailed)?;
let user = repo
.user()
.lookup(session.user_id)
.await?
.ok_or(RouteError::LoadFailed)?;
// Load the user if there is one
let user = if let Some(user_id) = session.user_id {
let user = repo
.user()
.lookup(user_id)
.await?
.ok_or(RouteError::LoadFailed)?;
Some(user)
} else {
None
};
if !token.is_valid(clock.now()) || !session.is_valid() || !user.is_valid() {
// If there is a user for this session, check that it is not locked
let user_valid = user.as_ref().map_or(false, User::is_valid);
if !token.is_valid(clock.now()) || !session.is_valid() || !user_valid {
return Err(RouteError::InvalidToken);
}

View File

@ -184,24 +184,32 @@ pub(crate) async fn post(
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
let user = repo
.user()
.lookup(session.user_id)
.await?
.filter(User::is_valid)
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
// The session might not have a user on it (for Client Credentials grants for
// example), so we're optionally fetching the user
let (sub, username) = if let Some(user_id) = session.user_id {
let user = repo
.user()
.lookup(user_id)
.await?
// Fail if the user is not valid (e.g. locked)
.filter(User::is_valid)
.ok_or(RouteError::UnknownToken)?;
(Some(user.sub), Some(user.username))
} else {
(None, None)
};
IntrospectionResponse {
active: true,
scope: Some(session.scope),
client_id: Some(session.client_id.to_string()),
username: Some(user.username),
username,
token_type: Some(OAuthTokenTypeHint::AccessToken),
exp: Some(token.expires_at),
iat: Some(token.created_at),
nbf: Some(token.created_at),
sub: Some(user.sub),
sub,
aud: None,
iss: None,
jti: Some(token.jti()),
@ -224,24 +232,32 @@ pub(crate) async fn post(
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
let user = repo
.user()
.lookup(session.user_id)
.await?
.filter(User::is_valid)
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
// The session might not have a user on it (for Client Credentials grants for
// example), so we're optionally fetching the user
let (sub, username) = if let Some(user_id) = session.user_id {
let user = repo
.user()
.lookup(user_id)
.await?
// Fail if the user is not valid (e.g. locked)
.filter(User::is_valid)
.ok_or(RouteError::UnknownToken)?;
(Some(user.sub), Some(user.username))
} else {
(None, None)
};
IntrospectionResponse {
active: true,
scope: Some(session.scope),
client_id: Some(session.client_id.to_string()),
username: Some(user.username),
username,
token_type: Some(OAuthTokenTypeHint::RefreshToken),
exp: None,
iat: Some(token.created_at),
nbf: Some(token.created_at),
sub: Some(user.sub),
sub,
aud: None,
iss: None,
jti: Some(token.jti()),

View File

@ -199,25 +199,29 @@ pub(crate) async fn post(
return Err(RouteError::UnauthorizedClient);
}
// Fetch the user
let user = repo
.user()
.lookup(session.user_id)
.await?
.ok_or(RouteError::UnknownToken)?;
// If the session is associated with a user, make sure we schedule a device
// deletion job for all the devices associated with the session.
if let Some(user_id) = session.user_id {
// Fetch the user
let user = repo
.user()
.lookup(user_id)
.await?
.ok_or(RouteError::UnknownToken)?;
// Scan the scopes of the session to find if there is any device that should be
// deleted from the Matrix server.
// TODO: this should be moved in a higher level "end oauth session" method.
// XXX: this might not be the right semantic, but it's the best we
// can do for now, since we're not explicitly storing devices for OAuth2
// sessions.
for scope in &*session.scope {
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
// Scan the scopes of the session to find if there is any device that should be
// deleted from the Matrix server.
// TODO: this should be moved in a higher level "end oauth session" method.
// XXX: this might not be the right semantic, but it's the best we
// can do for now, since we're not explicitly storing devices for OAuth2
// sessions.
for scope in &*session.scope {
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
}
}
}

View File

@ -65,6 +65,9 @@ pub enum RouteError {
#[from] AuthorizationVerificationError<mas_storage::RepositoryError>,
),
#[error("session is not allowed to access the userinfo endpoint")]
Unauthorized,
#[error("no suitable key found for signing")]
InvalidSigningKey,
@ -86,7 +89,9 @@ impl IntoResponse for RouteError {
Self::Internal(_) | Self::InvalidSigningKey | Self::NoSuchClient | Self::NoSuchUser => {
(StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response()
}
Self::AuthorizationVerificationError(_e) => StatusCode::UNAUTHORIZED.into_response(),
Self::AuthorizationVerificationError(_) | Self::Unauthorized => {
StatusCode::UNAUTHORIZED.into_response()
}
}
}
}
@ -102,9 +107,19 @@ pub async fn get(
) -> Result<Response, RouteError> {
let session = user_authorization.protected(&mut repo, &clock).await?;
// This endpoint requires the `openid` scope.
if !session.scope.contains("openid") {
return Err(RouteError::Unauthorized);
}
// Fail if the session is not associated with a user.
let Some(user_id) = session.user_id else {
return Err(RouteError::Unauthorized);
};
let user = repo
.user()
.lookup(session.user_id)
.lookup(user_id)
.await?
.ok_or(RouteError::NoSuchUser)?;

View File

@ -135,7 +135,6 @@ impl<'c> OAuth2AccessTokenRepository for PgOAuth2AccessTokenRepository<'c> {
fields(
db.statement,
%session.id,
user.id = %session.user_id,
client.id = %session.client_id,
access_token.id,
),

View File

@ -406,7 +406,6 @@ impl<'c> OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantReposi
%grant.id,
client.id = %grant.client_id,
%session.id,
user.id = %session.user_id,
),
err,
)]

View File

@ -143,7 +143,6 @@ impl<'c> OAuth2RefreshTokenRepository for PgOAuth2RefreshTokenRepository<'c> {
fields(
db.statement,
%session.id,
user.id = %session.user_id,
client.id = %session.client_id,
refresh_token.id,
),

View File

@ -86,7 +86,7 @@ impl TryFrom<OAuthSessionLookup> for Session {
state,
created_at: value.created_at,
client_id: value.oauth2_client_id.into(),
user_id: value.user_id.into(),
user_id: Some(value.user_id.into()),
user_session_id: value.user_session_id.map(Ulid::from),
scope,
})
@ -186,7 +186,7 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
id,
state: SessionState::Valid,
created_at,
user_id: user_session.user.id,
user_id: Some(user_session.user.id),
user_session_id: Some(user_session.id),
client_id: client.id,
scope,
@ -200,7 +200,6 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
db.statement,
%session.id,
%session.scope,
user.id = %session.user_id,
client.id = %session.client_id,
),
err,