Skip to content

Commit 63d6431

Browse files
sandhosesandhose
authored
Support for the stable scopes (element-hq#4686)
2 parents 54d8322 + 7e018a0 commit 63d6431

File tree

17 files changed

+260
-88
lines changed

17 files changed

+260
-88
lines changed

‎crates/data-model/src/compat/device.rs‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ use serde::{Deserialize, Serialize};
1313
use thiserror::Error;
1414

1515
static GENERATED_DEVICE_ID_LENGTH: usize = 10;
16-
static DEVICE_SCOPE_PREFIX: &str = "urn:matrix:org.matrix.msc2967.client:device:";
16+
static UNSTABLE_DEVICE_SCOPE_PREFIX: &str = "urn:matrix:org.matrix.msc2967.client:device:";
17+
static STABLE_DEVICE_SCOPE_PREFIX: &str = "urn:matrix:client:device:";
1718

1819
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
1920
#[serde(transparent)]
@@ -28,24 +29,31 @@ pub enum ToScopeTokenError {
2829
}
2930

3031
impl Device {
31-
/// Get the corresponding [`ScopeToken`] for that device
32+
/// Get the corresponding stable and unstable [`ScopeToken`] for that device
3233
///
3334
/// # Errors
3435
///
3536
/// Returns an error if the device ID contains characters that can't be
3637
/// encoded in a scope
37-
pub fn to_scope_token(&self) -> Result<ScopeToken, ToScopeTokenError> {
38-
format!("{DEVICE_SCOPE_PREFIX}{}", self.id)
39-
.parse()
40-
.map_err(|_| ToScopeTokenError::InvalidCharacters)
38+
pub fn to_scope_token(&self) -> Result<[ScopeToken; 2], ToScopeTokenError> {
39+
Ok([
40+
format!("{STABLE_DEVICE_SCOPE_PREFIX}{}", self.id)
41+
.parse()
42+
.map_err(|_| ToScopeTokenError::InvalidCharacters)?,
43+
format!("{UNSTABLE_DEVICE_SCOPE_PREFIX}{}", self.id)
44+
.parse()
45+
.map_err(|_| ToScopeTokenError::InvalidCharacters)?,
46+
])
4147
}
4248

4349
/// Get the corresponding [`Device`] from a [`ScopeToken`]
4450
///
4551
/// Returns `None` if the [`ScopeToken`] is not a device scope
4652
#[must_use]
4753
pub fn from_scope_token(token: &ScopeToken) -> Option<Self> {
48-
let id = token.as_str().strip_prefix(DEVICE_SCOPE_PREFIX)?;
54+
let stable = token.as_str().strip_prefix(STABLE_DEVICE_SCOPE_PREFIX);
55+
let unstable = token.as_str().strip_prefix(UNSTABLE_DEVICE_SCOPE_PREFIX);
56+
let id = stable.or(unstable)?;
4957
Some(Device::from(id.to_owned()))
5058
}
5159

@@ -89,12 +97,23 @@ mod test {
8997
#[test]
9098
fn test_device_id_to_from_scope_token() {
9199
let device = Device::from("AABBCCDDEE".to_owned());
92-
let scope_token = device.to_scope_token().unwrap();
100+
let [stable_scope_token, unstable_scope_token] = device.to_scope_token().unwrap();
93101
assert_eq!(
94-
scope_token.as_str(),
102+
stable_scope_token.as_str(),
103+
"urn:matrix:client:device:AABBCCDDEE"
104+
);
105+
assert_eq!(
106+
unstable_scope_token.as_str(),
95107
"urn:matrix:org.matrix.msc2967.client:device:AABBCCDDEE"
96108
);
97-
assert_eq!(Device::from_scope_token(&scope_token), Some(device));
109+
assert_eq!(
110+
Device::from_scope_token(&unstable_scope_token).as_ref(),
111+
Some(&device)
112+
);
113+
assert_eq!(
114+
Device::from_scope_token(&stable_scope_token).as_ref(),
115+
Some(&device)
116+
);
98117
assert_eq!(Device::from_scope_token(&OPENID), None);
99118
}
100119
}

‎crates/handlers/src/admin/model.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ impl OAuth2Session {
375375
user_id: Some(Ulid::from_bytes([0x04; 16])),
376376
user_session_id: Some(Ulid::from_bytes([0x05; 16])),
377377
client_id: Ulid::from_bytes([0x06; 16]),
378-
scope: "urn:matrix:org.matrix.msc2967.client:api:*".to_owned(),
378+
scope: "urn:matrix:client:api:*".to_owned(),
379379
user_agent: Some("Mozilla/5.0".to_owned()),
380380
last_active_at: Some(DateTime::default()),
381381
last_active_ip: Some("127.0.0.1".parse().unwrap()),

‎crates/handlers/src/graphql/query/session.rs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use mas_storage::{
1111
compat::{CompatSessionFilter, CompatSessionRepository},
1212
oauth2::OAuth2SessionFilter,
1313
};
14-
use oauth2_types::scope::Scope;
1514

1615
use crate::graphql::{
1716
UserId,
@@ -77,20 +76,11 @@ impl SessionQuery {
7776
))));
7877
}
7978

80-
// Then, try to find an OAuth 2.0 session. Because we don't have any dedicated
81-
// device column, we're looking up using the device scope.
82-
// All device IDs can't necessarily be encoded as a scope. If it's not the case,
83-
// we'll skip looking for OAuth 2.0 sessions.
84-
let Ok(scope_token) = device.to_scope_token() else {
85-
repo.cancel().await?;
86-
87-
return Ok(None);
88-
};
89-
let scope = Scope::from_iter([scope_token]);
79+
// Then, try to find an OAuth 2.0 session.
9080
let filter = OAuth2SessionFilter::new()
9181
.for_user(&user)
9282
.active_only()
93-
.with_scope(&scope);
83+
.for_device(&device);
9484
let sessions = repo.oauth2_session().list(filter, pagination).await?;
9585

9686
// It's possible to have multiple active OAuth 2.0 sessions. For now, we just

‎crates/handlers/src/oauth2/introspection.rs‎

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
55
// Please see LICENSE files in the repository root for full details.
66

7-
use std::sync::{Arc, LazyLock};
7+
use std::{
8+
collections::BTreeSet,
9+
sync::{Arc, LazyLock},
10+
};
811

912
use axum::{Json, extract::State, http::HeaderValue, response::IntoResponse};
1013
use hyper::{HeaderMap, StatusCode};
@@ -25,7 +28,7 @@ use mas_storage::{
2528
use oauth2_types::{
2629
errors::{ClientError, ClientErrorCode},
2730
requests::{IntrospectionRequest, IntrospectionResponse},
28-
scope::ScopeToken,
31+
scope::{Scope, ScopeToken},
2932
};
3033
use opentelemetry::{Key, KeyValue, metrics::Counter};
3134
use thiserror::Error;
@@ -207,9 +210,33 @@ const INACTIVE: IntrospectionResponse = IntrospectionResponse {
207210
device_id: None,
208211
};
209212

210-
const API_SCOPE: ScopeToken = ScopeToken::from_static("urn:matrix:org.matrix.msc2967.client:api:*");
213+
const UNSTABLE_API_SCOPE: ScopeToken =
214+
ScopeToken::from_static("urn:matrix:org.matrix.msc2967.client:api:*");
215+
const STABLE_API_SCOPE: ScopeToken = ScopeToken::from_static("urn:matrix:client:api:*");
211216
const SYNAPSE_ADMIN_SCOPE: ScopeToken = ScopeToken::from_static("urn:synapse:admin:*");
212217

218+
/// Normalize a scope by adding the stable and unstable API scopes equivalents
219+
/// if missing
220+
fn normalize_scope(mut scope: Scope) -> Scope {
221+
// Here we abuse the fact that the scope is a BTreeSet to not care about
222+
// duplicates
223+
let mut to_add = BTreeSet::new();
224+
for token in &*scope {
225+
if token == &STABLE_API_SCOPE {
226+
to_add.insert(UNSTABLE_API_SCOPE);
227+
} else if token == &UNSTABLE_API_SCOPE {
228+
to_add.insert(STABLE_API_SCOPE);
229+
} else if let Some(device) = Device::from_scope_token(token) {
230+
let tokens = device
231+
.to_scope_token()
232+
.expect("from/to scope token rountrip should never fail");
233+
to_add.extend(tokens);
234+
}
235+
}
236+
scope.append(&mut to_add);
237+
scope
238+
}
239+
213240
#[tracing::instrument(
214241
name = "handlers.oauth2.introspection.post",
215242
fields(client.id = credentials.client_id()),
@@ -340,9 +367,11 @@ pub(crate) async fn post(
340367
],
341368
);
342369

370+
let scope = normalize_scope(session.scope);
371+
343372
IntrospectionResponse {
344373
active: true,
345-
scope: Some(session.scope),
374+
scope: Some(scope),
346375
client_id: Some(session.client_id.to_string()),
347376
username,
348377
token_type: Some(OAuthTokenTypeHint::AccessToken),
@@ -411,9 +440,11 @@ pub(crate) async fn post(
411440
],
412441
);
413442

443+
let scope = normalize_scope(session.scope);
444+
414445
IntrospectionResponse {
415446
active: true,
416-
scope: Some(session.scope),
447+
scope: Some(scope),
417448
client_id: Some(session.client_id.to_string()),
418449
username,
419450
token_type: Some(OAuthTokenTypeHint::RefreshToken),
@@ -475,9 +506,9 @@ pub(crate) async fn post(
475506
.transpose()?
476507
};
477508

478-
let scope = [API_SCOPE]
509+
let scope = [STABLE_API_SCOPE, UNSTABLE_API_SCOPE]
479510
.into_iter()
480-
.chain(device_scope_opt)
511+
.chain(device_scope_opt.into_iter().flatten())
481512
.chain(synapse_admin_scope_opt)
482513
.collect();
483514

@@ -559,9 +590,9 @@ pub(crate) async fn post(
559590
.transpose()?
560591
};
561592

562-
let scope = [API_SCOPE]
593+
let scope = [STABLE_API_SCOPE, UNSTABLE_API_SCOPE]
563594
.into_iter()
564-
.chain(device_scope_opt)
595+
.chain(device_scope_opt.into_iter().flatten())
565596
.chain(synapse_admin_scope_opt)
566597
.collect();
567598

@@ -907,7 +938,7 @@ mod tests {
907938
let refresh_token = response["refresh_token"].as_str().unwrap();
908939
let device_id = response["device_id"].as_str().unwrap();
909940
let expected_scope: Scope =
910-
format!("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:{device_id}")
941+
format!("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:{device_id} urn:matrix:client:api:* urn:matrix:client:device:{device_id}")
911942
.parse()
912943
.unwrap();
913944

@@ -940,7 +971,7 @@ mod tests {
940971
assert_eq!(response.token_type, Some(OAuthTokenTypeHint::AccessToken));
941972
assert_eq!(
942973
response.scope.map(|s| s.to_string()),
943-
Some("urn:matrix:org.matrix.msc2967.client:api:*".to_owned())
974+
Some("urn:matrix:client:api:* urn:matrix:org.matrix.msc2967.client:api:*".to_owned())
944975
);
945976
assert_eq!(response.device_id.as_deref(), Some(device_id));
946977

‎crates/oauth2-types/src/scope.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@
1010
1111
#![allow(clippy::module_name_repetitions)]
1212

13-
use std::{borrow::Cow, collections::BTreeSet, iter::FromIterator, ops::Deref, str::FromStr};
13+
use std::{
14+
borrow::Cow,
15+
collections::BTreeSet,
16+
iter::FromIterator,
17+
ops::{Deref, DerefMut},
18+
str::FromStr,
19+
};
1420

1521
use serde::{Deserialize, Serialize};
1622
use thiserror::Error;
@@ -121,6 +127,12 @@ impl Deref for Scope {
121127
}
122128
}
123129

130+
impl DerefMut for Scope {
131+
fn deref_mut(&mut self) -> &mut Self::Target {
132+
&mut self.0
133+
}
134+
}
135+
124136
impl FromStr for Scope {
125137
type Err = InvalidScope;
126138

@@ -248,6 +260,7 @@ mod tests {
248260
);
249261

250262
assert!(Scope::from_str("http://example.com").is_ok());
251-
assert!(Scope::from_str("urn:matrix:org.matrix.msc2967.client:*").is_ok());
263+
assert!(Scope::from_str("urn:matrix:client:api:*").is_ok());
264+
assert!(Scope::from_str("urn:matrix:org.matrix.msc2967.client:api:*").is_ok());
252265
}
253266
}

‎crates/storage-pg/.sqlx/query-373f7eb215b0e515b000a37e55bd055954f697f257de026b74ec408938a52a1a.json‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎crates/storage-pg/.sqlx/query-5da7a197e0008f100ad4daa78f4aa6515f0fc9eb54075e8d6d15520d25b75172.json‎

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/storage-pg/src/app_session.rs‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -497,17 +497,24 @@ impl AppSessionRepository for PgAppSessionRepository<'_> {
497497
.instrument(span)
498498
.await?;
499499

500-
if let Ok(device_as_scope_token) = device.to_scope_token() {
500+
if let Ok([stable_device_as_scope_token, unstable_device_as_scope_token]) =
501+
device.to_scope_token()
502+
{
501503
let span = tracing::info_span!(
502504
"db.app_session.finish_sessions_to_replace_device.oauth2_sessions",
503505
{ DB_QUERY_TEXT } = tracing::field::Empty,
504506
);
505507
sqlx::query!(
506508
"
507-
UPDATE oauth2_sessions SET finished_at = $3 WHERE user_id = $1 AND $2 = ANY(scope_list) AND finished_at IS NULL
509+
UPDATE oauth2_sessions
510+
SET finished_at = $4
511+
WHERE user_id = $1
512+
AND ($2 = ANY(scope_list) OR $3 = ANY(scope_list))
513+
AND finished_at IS NULL
508514
",
509515
Uuid::from(user.id),
510-
device_as_scope_token.as_str(),
516+
stable_device_as_scope_token.as_str(),
517+
unstable_device_as_scope_token.as_str(),
511518
finished_at
512519
)
513520
.record(&span)
@@ -650,7 +657,10 @@ mod tests {
650657
.unwrap();
651658

652659
let device2 = Device::generate(&mut rng);
653-
let scope = Scope::from_iter([OPENID, device2.to_scope_token().unwrap()]);
660+
let scope: Scope = [OPENID]
661+
.into_iter()
662+
.chain(device2.to_scope_token().unwrap().into_iter())
663+
.collect();
654664

655665
// We're moving the clock forward by 1 minute between each session to ensure
656666
// we're getting consistent ordering in lists.

‎crates/storage-pg/src/oauth2/session.rs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ use mas_storage::{
1515
};
1616
use oauth2_types::scope::{Scope, ScopeToken};
1717
use rand::RngCore;
18-
use sea_query::{Expr, PgFunc, PostgresQueryBuilder, Query, enum_def, extension::postgres::PgExpr};
18+
use sea_query::{
19+
Condition, Expr, PgFunc, PostgresQueryBuilder, Query, SimpleExpr, enum_def,
20+
extension::postgres::PgExpr,
21+
};
1922
use sea_query_binder::SqlxBinder;
2023
use sqlx::PgConnection;
2124
use ulid::Ulid;
@@ -126,12 +129,19 @@ impl Filter for OAuth2SessionFilter<'_> {
126129
.ne(Expr::all(static_clients))
127130
}
128131
}))
129-
.add_option(self.device().map(|device| {
130-
if let Ok(scope_token) = device.to_scope_token() {
131-
Expr::val(scope_token.to_string()).eq(PgFunc::any(Expr::col((
132-
OAuth2Sessions::Table,
133-
OAuth2Sessions::ScopeList,
134-
))))
132+
.add_option(self.device().map(|device| -> SimpleExpr {
133+
if let Ok([stable_scope_token, unstable_scope_token]) = device.to_scope_token() {
134+
Condition::any()
135+
.add(
136+
Expr::val(stable_scope_token.to_string()).eq(PgFunc::any(Expr::col((
137+
OAuth2Sessions::Table,
138+
OAuth2Sessions::ScopeList,
139+
)))),
140+
)
141+
.add(Expr::val(unstable_scope_token.to_string()).eq(PgFunc::any(
142+
Expr::col((OAuth2Sessions::Table, OAuth2Sessions::ScopeList)),
143+
)))
144+
.into()
135145
} else {
136146
// If the device ID can't be encoded as a scope token, match no rows
137147
Expr::val(false).into()

‎docs/api/spec.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@
469469
"user_id": "040G2081040G2081040G208104",
470470
"user_session_id": "050M2GA1850M2GA1850M2GA185",
471471
"client_id": "060R30C1G60R30C1G60R30C1G6",
472-
"scope": "urn:matrix:org.matrix.msc2967.client:api:*",
472+
"scope": "urn:matrix:client:api:*",
473473
"user_agent": "Mozilla/5.0",
474474
"last_active_at": "1970-01-01T00:00:00Z",
475475
"last_active_ip": "127.0.0.1",

0 commit comments

Comments
 (0)