matrix/authentication-service
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-authentication-service.git synced 2025-07-31 09:24:31 +03:00
Code Activity

Add the user_id directly on oauth2_sessions and make the scope a text list

Browse Source
This commit is contained in:
Quentin Gliech
2023-08-29 12:32:22 +02:00
parent feb59344f3
commit 438a10332a
21 changed files with 186 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
crates/data-model/src/oauth2/session.rs
View File

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

6
crates/graphql/src/lib.rs
View File

@ -88,6 +88,12 @@ impl OwnerId for mas_data_model::UserEmail {
}
}
impl OwnerId for Session {
fn owner_id(&self) -> Option<Ulid> {
Some(self.user_id)
}
}
impl OwnerId for mas_data_model::CompatSession {
fn owner_id(&self) -> Option<Ulid> {
Some(self.user_id)

20
crates/graphql/src/model/oauth.rs
View File

@ -90,31 +90,35 @@ impl OAuth2Session {
pub async fn browser_session(
&self,
ctx: &Context<'_>,
) -> Result<BrowserSession, async_graphql::Error> {
) -> Result<Option<BrowserSession>, async_graphql::Error> {
let Some(user_session_id) = self.0.user_session_id else {
return Ok(None);
};
let state = ctx.state();
let mut repo = state.repository().await?;
let browser_session = repo
.browser_session()
.lookup(self.0.user_session_id)
.lookup(user_session_id)
.await?
.context("Could not load browser session")?;
repo.cancel().await?;
Ok(BrowserSession(browser_session))
Ok(Some(BrowserSession(browser_session)))
}
/// User authorized for this session.
pub async fn user(&self, ctx: &Context<'_>) -> Result<User, async_graphql::Error> {
let state = ctx.state();
let mut repo = state.repository().await?;
let browser_session = repo
.browser_session()
.lookup(self.0.user_session_id)
let user = repo
.user()
.lookup(self.0.user_id)
.await?
.context("Could not load browser session")?;
.context("Could not load user")?;
repo.cancel().await?;
Ok(User(browser_session.user))
Ok(User(user))
}
}

17
crates/graphql/src/mutations/oauth2_session.rs
View File

@ -92,17 +92,16 @@ impl OAuth2SessionMutations {
return Ok(EndOAuth2SessionPayload::NotFound);
};
// XXX: again, the user_id should be directly stored in the session.
let user_session = repo
.browser_session()
.lookup(session.user_session_id)
.await?
.context("Could not load user session")?;
if !requester.is_owner_or_admin(&user_session) {
if !requester.is_owner_or_admin(&session) {
return Ok(EndOAuth2SessionPayload::NotFound);
}
let user = repo
.user()
.lookup(session.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.
@ -113,7 +112,7 @@ impl OAuth2SessionMutations {
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user_session.user, &device))
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
}
}

9
crates/handlers/src/graphql/mod.rs
View File

@ -196,15 +196,12 @@ async fn get_requester(
.await?
.ok_or(RouteError::LoadFailed)?;
// XXX: The user_id should really be directly on the OAuth session
let browser_session = repo
.browser_session()
.lookup(session.user_session_id)
let user = repo
.user()
.lookup(session.user_id)
.await?
.ok_or(RouteError::LoadFailed)?;
let user = browser_session.user;
if !token.is_valid(clock.now()) || !session.is_valid() || !user.is_valid() {
return Err(RouteError::InvalidToken);
}

28
crates/handlers/src/oauth2/introspection.rs
View File

@ -18,13 +18,13 @@ use mas_axum_utils::{
client_authorization::{ClientAuthorization, CredentialsVerificationError},
http_client_factory::HttpClientFactory,
};
use mas_data_model::{TokenFormatError, TokenType};
use mas_data_model::{TokenFormatError, TokenType, User};
use mas_iana::oauth::{OAuthClientAuthenticationMethod, OAuthTokenTypeHint};
use mas_keystore::Encrypter;
use mas_storage::{
compat::{CompatAccessTokenRepository, CompatRefreshTokenRepository, CompatSessionRepository},
oauth2::{OAuth2AccessTokenRepository, OAuth2RefreshTokenRepository, OAuth2SessionRepository},
user::{BrowserSessionRepository, UserRepository},
user::UserRepository,
BoxClock, BoxRepository, Clock,
};
use oauth2_types::{
@ -184,11 +184,11 @@ pub(crate) async fn post(
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
let browser_session = repo
.browser_session()
.lookup(session.user_session_id)
let user = repo
.user()
.lookup(session.user_id)
.await?
.filter(|b| b.user.is_valid())
.filter(User::is_valid)
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
@ -196,12 +196,12 @@ pub(crate) async fn post(
active: true,
scope: Some(session.scope),
client_id: Some(session.client_id.to_string()),
username: Some(browser_session.user.username),
username: Some(user.username),
token_type: Some(OAuthTokenTypeHint::AccessToken),
exp: Some(token.expires_at),
iat: Some(token.created_at),
nbf: Some(token.created_at),
sub: Some(browser_session.user.sub),
sub: Some(user.sub),
aud: None,
iss: None,
jti: Some(token.jti()),
@ -224,11 +224,11 @@ pub(crate) async fn post(
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
let browser_session = repo
.browser_session()
.lookup(session.user_session_id)
let user = repo
.user()
.lookup(session.user_id)
.await?
.filter(|b| b.user.is_valid())
.filter(User::is_valid)
// XXX: is that the right error to bubble up?
.ok_or(RouteError::UnknownToken)?;
@ -236,12 +236,12 @@ pub(crate) async fn post(
active: true,
scope: Some(session.scope),
client_id: Some(session.client_id.to_string()),
username: Some(browser_session.user.username),
username: Some(user.username),
token_type: Some(OAuthTokenTypeHint::RefreshToken),
exp: None,
iat: Some(token.created_at),
nbf: Some(token.created_at),
sub: Some(browser_session.user.sub),
sub: Some(user.sub),
aud: None,
iss: None,
jti: Some(token.jti()),

11
crates/handlers/src/oauth2/revoke.rs
View File

@ -23,7 +23,6 @@ use mas_iana::oauth::OAuthTokenTypeHint;
use mas_keystore::Encrypter;
use mas_storage::{
job::{DeleteDeviceJob, JobRepositoryExt},
user::BrowserSessionRepository,
BoxClock, BoxRepository, RepositoryAccess,
};
use oauth2_types::{
@ -200,10 +199,10 @@ pub(crate) async fn post(
return Err(RouteError::UnauthorizedClient);
}
// Fetch the user session
let user_session = repo
.browser_session()
.lookup(session.user_session_id)
// Fetch the user
let user = repo
.user()
.lookup(session.user_id)
.await?
.ok_or(RouteError::UnknownToken)?;
@ -217,7 +216,7 @@ pub(crate) async fn post(
if let Some(device) = Device::from_scope_token(scope) {
// Schedule a job to delete the device.
repo.job()
.schedule_job(DeleteDeviceJob::new(&user_session.user, &device))
.schedule_job(DeleteDeviceJob::new(&user, &device))
.await?;
}
}

7
crates/handlers/src/oauth2/token.rs
View File

@ -296,9 +296,14 @@ async fn authorization_code_grant(
}
};
let Some(user_session_id) = session.user_session_id else {
tracing::warn!("No user session associated with this OAuth2 session");
return Err(RouteError::InvalidGrant);
};
let browser_session = repo
.browser_session()
.lookup(session.user_session_id)
.lookup(user_session_id)
.await?
.ok_or(RouteError::NoSuchBrowserSession)?;

23
crates/handlers/src/oauth2/userinfo.rs
View File

@ -29,9 +29,7 @@ use mas_jose::{
use mas_keystore::Keystore;
use mas_router::UrlBuilder;
use mas_storage::{
oauth2::OAuth2ClientRepository,
user::{BrowserSessionRepository, UserEmailRepository},
BoxClock, BoxRepository, BoxRng,
oauth2::OAuth2ClientRepository, user::UserEmailRepository, BoxClock, BoxRepository, BoxRng,
};
use oauth2_types::scope;
use serde::Serialize;
@ -73,8 +71,8 @@ pub enum RouteError {
#[error("failed to load client")]
NoSuchClient,
#[error("failed to load browser session")]
NoSuchBrowserSession,
#[error("failed to load user")]
NoSuchUser,
}
impl_from_error_for_route!(mas_storage::RepositoryError);
@ -85,10 +83,7 @@ impl IntoResponse for RouteError {
fn into_response(self) -> axum::response::Response {
sentry::capture_error(&self);
match self {
Self::Internal(_)
| Self::InvalidSigningKey
| Self::NoSuchClient
| Self::NoSuchBrowserSession => {
Self::Internal(_) | Self::InvalidSigningKey | Self::NoSuchClient | Self::NoSuchUser => {
(StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response()
}
Self::AuthorizationVerificationError(_e) => StatusCode::UNAUTHORIZED.into_response(),
@ -107,13 +102,11 @@ pub async fn get(
) -> Result<Response, RouteError> {
let session = user_authorization.protected(&mut repo, &clock).await?;
let browser_session = repo
.browser_session()
.lookup(session.user_session_id)
let user = repo
.user()
.lookup(session.user_id)
.await?
.ok_or(RouteError::NoSuchBrowserSession)?;
let user = browser_session.user;
.ok_or(RouteError::NoSuchUser)?;
let user_email = if session.scope.contains(&scope::EMAIL) {
repo.user_email().get_primary(&user).await?

19
crates/storage-pg/.sqlx/query-1b547552eed4128f2227c681ff2d45586cdb0c20b98393f89036fbf0f1d2dee2.json generated Normal file
View File

@ -0,0 +1,19 @@
{
"db_name": "PostgreSQL",
"query": "\n INSERT INTO oauth2_sessions\n ( oauth2_session_id\n , user_id\n , user_session_id\n , oauth2_client_id\n , scope_list\n , created_at\n )\n VALUES ($1, $2, $3, $4, $5, $6)\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Uuid",
"Uuid",
"Uuid",
"Uuid",
"TextArray",
"Timestamptz"
]
},
"nullable": []
},
"hash": "1b547552eed4128f2227c681ff2d45586cdb0c20b98393f89036fbf0f1d2dee2"
}

20
crates/storage-pg/.sqlx/query-f0ace1af3775192a555c4ebb59b81183f359771f9f77e5fad759d38d872541d1.json → crates/storage-pg/.sqlx/query-31b7910705963c1a0fa1d55614299aea1a2fa782d40ec53e93c03fe0d73ab0f4.json generated
View File

@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT oauth2_session_id\n , user_session_id\n , oauth2_client_id\n , scope\n , created_at\n , finished_at\n FROM oauth2_sessions\n\n WHERE oauth2_session_id = $1\n ",
"query": "\n SELECT oauth2_session_id\n , user_id\n , user_session_id\n , oauth2_client_id\n , scope_list\n , created_at\n , finished_at\n FROM oauth2_sessions\n\n WHERE oauth2_session_id = $1\n ",
"describe": {
"columns": [
{
@ -10,26 +10,31 @@
},
{
"ordinal": 1,
"name": "user_session_id",
"name": "user_id",
"type_info": "Uuid"
},
{
"ordinal": 2,
"name": "oauth2_client_id",
"name": "user_session_id",
"type_info": "Uuid"
},
{
"ordinal": 3,
"name": "scope",
"type_info": "Text"
"name": "oauth2_client_id",
"type_info": "Uuid"
},
{
"ordinal": 4,
"name": "scope_list",
"type_info": "TextArray"
},
{
"ordinal": 5,
"name": "created_at",
"type_info": "Timestamptz"
},
{
"ordinal": 5,
"ordinal": 6,
"name": "finished_at",
"type_info": "Timestamptz"
}
@ -42,11 +47,12 @@
"nullable": [
false,
false,
true,
false,
false,
false,
true
]
},
"hash": "f0ace1af3775192a555c4ebb59b81183f359771f9f77e5fad759d38d872541d1"
"hash": "31b7910705963c1a0fa1d55614299aea1a2fa782d40ec53e93c03fe0d73ab0f4"
}

18
crates/storage-pg/.sqlx/query-583ae9a0db9cd55fa57a179339550f3dab1bfc76f35ad488e1560ea37f7ed029.json generated
View File

@ -1,18 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n INSERT INTO oauth2_sessions\n ( oauth2_session_id\n , user_session_id\n , oauth2_client_id\n , scope\n , created_at\n )\n VALUES ($1, $2, $3, $4, $5)\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Uuid",
"Uuid",
"Uuid",
"Text",
"Timestamptz"
]
},
"nullable": []
},
"hash": "583ae9a0db9cd55fa57a179339550f3dab1bfc76f35ad488e1560ea37f7ed029"
}

49
crates/storage-pg/migrations/20230829092920_oauth2_sessions_user_id_scope_list.sql Normal file
View File

@ -0,0 +1,49 @@
-- Copyright 2023 The Matrix.org Foundation C.I.C.
--
-- Licensed under the Apache License, Version 2.0 (the "License");
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.
-- We need to be able to do OAuth 2.0 sessions without a user session,
-- and we would like to find sessions with a particular scope.
--
-- This migration edits the "oauth2_sessions" table to:
-- * Add a "user_id" column
-- * Make the "user_session_id" nullable
-- * Infer the "user_id" from the "user_session_id" for existing rows
-- * Add a "scope_list" column, which is the "scope" column in array form
-- * Import the "scope" column into the "scope_list" column for existing rows by splitting on spaces
-- * Sets both columns as NOT NULL once the migration is complete
-- * Drop the "scope" column
-- * Index the "scope_list" column with a GIN index
ALTER TABLE "oauth2_sessions"
ADD COLUMN "user_id" UUID
REFERENCES "users" ("user_id") ON DELETE CASCADE,
ADD COLUMN "scope_list" TEXT[];
UPDATE "oauth2_sessions"
SET "user_id" = "user_sessions"."user_id"
FROM "user_sessions"
WHERE "oauth2_sessions"."user_session_id" = "user_sessions"."user_session_id";
UPDATE "oauth2_sessions"
SET "scope_list" = string_to_array("scope", ' ')
WHERE "scope_list" IS NULL;
ALTER TABLE "oauth2_sessions"
ALTER COLUMN "user_session_id" DROP NOT NULL,
ALTER COLUMN "user_id" SET NOT NULL,
ALTER COLUMN "scope_list" SET NOT NULL,
DROP COLUMN "scope";
CREATE INDEX "oauth2_sessions_scope_list_idx"
ON "oauth2_sessions" USING GIN ("scope_list");

3
crates/storage-pg/src/iden.rs
View File

@ -72,10 +72,11 @@ pub enum OAuth2Sessions {
Table,
#[iden = "oauth2_session_id"]
OAuth2SessionId,
UserId,
UserSessionId,
#[iden = "oauth2_client_id"]
OAuth2ClientId,
Scope,
ScopeList,
CreatedAt,
FinishedAt,
}

2
crates/storage-pg/src/oauth2/access_token.rs
View File

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

2
crates/storage-pg/src/oauth2/authorization_grant.rs
View File

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

2
crates/storage-pg/src/oauth2/refresh_token.rs
View File

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

61
crates/storage-pg/src/oauth2/session.rs
View File

@ -19,7 +19,7 @@ use mas_storage::{
oauth2::{OAuth2SessionFilter, OAuth2SessionRepository},
Clock, Page, Pagination,
};
use oauth2_types::scope::Scope;
use oauth2_types::scope::{Scope, ScopeToken};
use rand::RngCore;
use sea_query::{enum_def, Expr, PostgresQueryBuilder, Query};
use sea_query_binder::SqlxBinder;
@ -51,10 +51,10 @@ impl<'c> PgOAuth2SessionRepository<'c> {
#[enum_def]
struct OAuthSessionLookup {
oauth2_session_id: Uuid,
user_session_id: Uuid,
user_id: Uuid,
user_session_id: Option<Uuid>,
oauth2_client_id: Uuid,
scope: String,
#[allow(dead_code)]
scope_list: Vec<String>,
created_at: DateTime<Utc>,
finished_at: Option<DateTime<Utc>>,
}
@ -64,7 +64,12 @@ impl TryFrom<OAuthSessionLookup> for Session {
fn try_from(value: OAuthSessionLookup) -> Result<Self, Self::Error> {
let id = Ulid::from(value.oauth2_session_id);
let scope = value.scope.parse().map_err(|e| {
let scope: Result<Scope, _> = value
.scope_list
.iter()
.map(|s| s.parse::<ScopeToken>())
.collect();
let scope = scope.map_err(|e| {
DatabaseInconsistencyError::on("oauth2_sessions")
.column("scope")
.row(id)
@ -81,7 +86,8 @@ impl TryFrom<OAuthSessionLookup> for Session {
state,
created_at: value.created_at,
client_id: value.oauth2_client_id.into(),
user_session_id: value.user_session_id.into(),
user_id: value.user_id.into(),
user_session_id: value.user_session_id.map(Ulid::from),
scope,
})
}
@ -105,9 +111,10 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
OAuthSessionLookup,
r#"
SELECT oauth2_session_id
, user_id
, user_session_id
, oauth2_client_id
, scope
, scope_list
, created_at
, finished_at
FROM oauth2_sessions
@ -150,21 +157,25 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
let id = Ulid::from_datetime_with_source(created_at.into(), rng);
tracing::Span::current().record("session.id", tracing::field::display(id));
let scope_list: Vec<String> = scope.iter().map(|s| s.as_str().to_owned()).collect();
sqlx::query!(
r#"
INSERT INTO oauth2_sessions
( oauth2_session_id
, user_id
, user_session_id
, oauth2_client_id
, scope
, scope_list
, created_at
)
VALUES ($1, $2, $3, $4, $5)
VALUES ($1, $2, $3, $4, $5, $6)
"#,
Uuid::from(id),
Uuid::from(user_session.user.id),
Uuid::from(user_session.id),
Uuid::from(client.id),
scope.to_string(),
&scope_list,
created_at,
)
.traced()
@ -175,7 +186,8 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
id,
state: SessionState::Valid,
created_at,
user_session_id: user_session.id,
user_id: user_session.user.id,
user_session_id: Some(user_session.id),
client_id: client.id,
scope,
})
@ -188,7 +200,7 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
db.statement,
%session.id,
%session.scope,
user_session.id = %session.user_session_id,
user.id = %session.user_id,
client.id = %session.client_id,
),
err,
@ -237,6 +249,10 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::OAuth2SessionId)),
OAuthSessionLookupIden::Oauth2SessionId,
)
.expr_as(
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::UserId)),
OAuthSessionLookupIden::UserId,
)
.expr_as(
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::UserSessionId)),
OAuthSessionLookupIden::UserSessionId,
@ -246,8 +262,8 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
OAuthSessionLookupIden::Oauth2ClientId,
)
.expr_as(
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::Scope)),
OAuthSessionLookupIden::Scope,
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::ScopeList)),
OAuthSessionLookupIden::ScopeList,
)
.expr_as(
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::CreatedAt)),
@ -259,22 +275,7 @@ impl<'c> OAuth2SessionRepository for PgOAuth2SessionRepository<'c> {
)
.from(OAuth2Sessions::Table)
.and_where_option(filter.user().map(|user| {
// Check for user ownership by querying the user_sessions table
// The query plan is the same as if we were joining the tables instead
Expr::exists(
Query::select()
.expr(Expr::cust("1"))
.from(UserSessions::Table)
.and_where(
Expr::col((UserSessions::Table, UserSessions::UserId))
.eq(Uuid::from(user.id)),
)
.and_where(
Expr::col((UserSessions::Table, UserSessions::UserSessionId))
.equals((OAuth2Sessions::Table, OAuth2Sessions::UserSessionId)),
)
.take(),
)
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::UserId)).eq(Uuid::from(user.id))
}))
.and_where_option(filter.client().map(|client| {
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::OAuth2ClientId))

2
frontend/schema.graphql
View File

@ -575,7 +575,7 @@ type Oauth2Session implements Node & CreationEvent {
"""
The browser session which started this OAuth 2.0 session.
"""
browserSession: BrowserSession!
browserSession: BrowserSession
"""
User authorized for this session.
"""

2
frontend/src/gql/graphql.ts
View File

@ -427,7 +427,7 @@ export type Oauth2Session = CreationEvent &
Node & {
__typename?: "Oauth2Session";
/** The browser session which started this OAuth 2.0 session. */
browserSession: BrowserSession;
browserSession?: Maybe<BrowserSession>;
/** OAuth 2.0 client used by this session. */
client: Oauth2Client;
/** When the object was created. */

3
frontend/src/gql/schema.ts
View File

@ -1187,13 +1187,10 @@ export default {
{
name: "browserSession",
type: {
kind: "NON_NULL",
ofType: {
kind: "OBJECT",
name: "BrowserSession",
ofType: null,
},
},
args: [],
},
{