diff --git a/Cargo.lock b/Cargo.lock index 87b4769fb..38a6bee92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4962,8 +4962,8 @@ dependencies = [ [[package]] name = "ruma" -version = "0.16.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.17.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "assign", "js_int", @@ -4981,8 +4981,8 @@ dependencies = [ [[package]] name = "ruma-appservice-api" -version = "0.16.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.17.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "http", "js_int", @@ -4994,8 +4994,8 @@ dependencies = [ [[package]] name = "ruma-client-api" -version = "0.24.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.25.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "as_variant", "assign", @@ -5016,11 +5016,11 @@ dependencies = [ [[package]] name = "ruma-common" -version = "0.19.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.20.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "as_variant", - "base64 0.22.1", + "base64 0.23.1", "bytes", "date_header", "form_urlencoded", @@ -5049,8 +5049,8 @@ dependencies = [ [[package]] name = "ruma-events" -version = "0.34.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.35.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "as_variant", "indexmap 2.14.2", @@ -5070,11 +5070,10 @@ dependencies = [ [[package]] name = "ruma-federation-api" -version = "0.15.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.16.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "bytes", - "headers", "http", "http-auth", "httparse", @@ -5094,7 +5093,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.12.1" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "js_int", "thiserror 2.0.21", @@ -5102,8 +5101,8 @@ dependencies = [ [[package]] name = "ruma-macros" -version = "0.19.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.20.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "as_variant", "cfg-if", @@ -5118,8 +5117,8 @@ dependencies = [ [[package]] name = "ruma-push-gateway-api" -version = "0.15.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.16.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "js_int", "ruma-common", @@ -5130,10 +5129,10 @@ dependencies = [ [[package]] name = "ruma-signatures" -version = "0.21.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.22.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ - "base64 0.22.1", + "base64 0.23.1", "ed25519-dalek 3.0.0", "memchr", "pkcs8 0.11.0", @@ -5147,8 +5146,8 @@ dependencies = [ [[package]] name = "ruma-state-res" -version = "0.17.0" -source = "git+https://github.com/ruma/ruma.git?rev=fe8570dbca23f4263165748ed898aad126b23d85#fe8570dbca23f4263165748ed898aad126b23d85" +version = "0.18.0" +source = "git+https://github.com/gingershaped/ruwuma.git?rev=ea474080ab00d68edd484871d57488cd76a36259#ea474080ab00d68edd484871d57488cd76a36259" dependencies = [ "js_int", "ruma-common", diff --git a/Cargo.toml b/Cargo.toml index 058d3ed63..6ec1e6aea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -346,8 +346,8 @@ version = "1.1.1" # Used for matrix spec type definitions and helpers [workspace.dependencies.ruma] -git = "https://github.com/ruma/ruma.git" -rev = "fe8570dbca23f4263165748ed898aad126b23d85" +git = "https://github.com/gingershaped/ruwuma.git" +rev = "ea474080ab00d68edd484871d57488cd76a36259" features = [ "appservice-api-c", "client-api", @@ -378,6 +378,7 @@ features = [ "unstable-msc4380", "unstable-msc4143", "unstable-msc4293", + "unstable-msc4363", "unstable-msc4406", "unstable-msc4439", "unstable-msc4354", diff --git a/src/api/router/auth.rs b/src/api/router/auth.rs index 6ba31ac94..1a4d3a783 100644 --- a/src/api/router/auth.rs +++ b/src/api/router/auth.rs @@ -18,7 +18,7 @@ AuthScheme, NoAccessToken, NoAuthentication, }, client, - error::{ErrorKind, UnknownTokenErrorData}, + error::{ErrorKind, InsufficientUserAuthenticationErrorData, UnknownTokenErrorData}, federation::authentication::ServerSignatures, }, assign, @@ -315,7 +315,7 @@ async fn check_access_token( .get_session_info_for_device(&sender_user, &sender_device) .await { - session_info.scopes + session_info.scopes() } else { let mut scopes = BTreeSet::from_iter([OAuthClientScope::ApiFullAccess]); @@ -330,9 +330,15 @@ async fn check_access_token( .iter() .any(|scope| user_scopes.contains(scope)) { - return Err!(Request(Forbidden( - "You do not have permission to access this endpoint." - ))); + return Err(Error::Request( + ErrorKind::InsufficientUserAuthentication(Box::new( + assign!(InsufficientUserAuthenticationErrorData::new(), { + scope: required_scopes.iter().cloned().collect() + }), + )), + "You do not have permission to access this endpoint.".into(), + StatusCode::UNAUTHORIZED, + )); } Ok(ClientIdentity::User { sender_user, sender_device }) diff --git a/src/service/migrations.rs b/src/service/migrations.rs index d92a4bfeb..48bced19b 100644 --- a/src/service/migrations.rs +++ b/src/service/migrations.rs @@ -22,7 +22,8 @@ use futures::{FutureExt, StreamExt, TryStreamExt}; use itertools::Itertools; use ruma::{ - OwnedEventId, OwnedRoomId, OwnedUserId, UserId, + OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedUserId, UserId, + api::OAuthClientScope, events::{ AnyStrippedStateEvent, GlobalAccountDataEventType, StateEventType, push_rules::PushRulesEvent, @@ -31,7 +32,7 @@ push::Ruleset, serde::{Raw, from_raw_json_value}, }; -use serde_json::value::to_raw_value; +use serde_json::{Value, value::to_raw_value}; use crate::{Services, media, rooms, rooms::short::ShortStateHash}; @@ -160,6 +161,10 @@ macro_rules! migration { // This is backwards compatible because old versions will recreate the keyspace if it's // missing. migration!(drop_server_signingkeys), + // v22: Old versions will fail to deserialize the fixed OAuth scope names, + // preventing users from signing in. + VersionBump(22), + migration!(fix_oauth_scope_names), ] }; @@ -936,3 +941,31 @@ async fn drop_server_signingkeys(services: &Services) -> Result<()> { Ok(()) } + +async fn fix_oauth_scope_names(services: &Services) -> Result { + let db = &services.db; + let userdeviceid_oauthsessioninfo = db["userdeviceid_oauthsessioninfo"].clone(); + + userdeviceid_oauthsessioninfo + .stream::<(OwnedUserId, OwnedDeviceId), Value>() + .ignore_err() + .for_each(async |((user_id, device_id), mut session_info)| { + let map = session_info.as_object_mut().unwrap(); + let scopes = map.get_mut("scopes").unwrap().as_array_mut().unwrap(); + + if let Some(old_scope_index) = scopes + .iter() + .position(|value| *value == Value::String("ClientApi".to_owned())) + { + scopes[old_scope_index] = + Value::String(OAuthClientScope::ApiFullAccess.to_string()); + } + + userdeviceid_oauthsessioninfo.put((user_id, device_id), Json(session_info)); + }) + .await; + + info!("Fixed OAuth session scope names"); + + Ok(()) +} diff --git a/src/service/oauth/client_metadata.rs b/src/service/oauth/client_metadata.rs index 078452088..75d6d15b3 100644 --- a/src/service/oauth/client_metadata.rs +++ b/src/service/oauth/client_metadata.rs @@ -1,7 +1,7 @@ -use std::{collections::BTreeSet, hash::Hash}; +use std::{collections::BTreeSet, fmt::Debug, hash::Hash}; use itertools::Itertools; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de::DeserializeOwned}; use url::Url; #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] @@ -150,7 +150,7 @@ pub enum ResponseType { pub(super) fn btreeset_skip_err<'de, D, V>(de: D) -> Result, D::Error> where D: Deserializer<'de>, - V: Deserialize<'de> + Hash + Eq + Ord, + V: DeserializeOwned + Hash + Eq + Ord + Debug, { use std::marker::PhantomData; @@ -162,7 +162,7 @@ struct BTreeSetVisitor { impl<'de, V> Visitor<'de> for BTreeSetVisitor where - V: Deserialize<'de> + Hash + Eq + Ord, + V: DeserializeOwned + Hash + Eq + Ord + Debug, { type Value = BTreeSet; @@ -176,8 +176,14 @@ fn visit_seq(self, mut seq: A) -> Result { let mut set = BTreeSet::new(); - while let Some(element) = seq.next_element().transpose() { - if let Ok(element) = element { + while let Some(element) = seq.next_element::().transpose() { + // This trip through serde_json::Value is necessary because the + // serde_json serializer will not advance to the next + // element if an element fails to deserialize. Without it this + // would loop forever. + if let Ok(element) = element + && let Ok(element) = serde_json::from_value(element) + { set.insert(element); } } diff --git a/src/service/oauth/grant.rs b/src/service/oauth/grant.rs index 1e346ee13..dcbc146df 100644 --- a/src/service/oauth/grant.rs +++ b/src/service/oauth/grant.rs @@ -1,10 +1,8 @@ use std::{ borrow::Cow, - collections::HashSet, + collections::BTreeSet, error::Error, fmt::{Debug, Display}, - hash::Hash, - mem::discriminant, }; use regex::regex; @@ -82,42 +80,25 @@ pub enum Prompt { Unknown, } -#[derive(Debug, Clone, Deserialize, Serialize)] -pub enum RequestedScope { - Device(OwnedDeviceId), - ClientApi, - ServerAdministration, +#[derive(Debug, Clone, Default, Deserialize, Serialize)] +pub struct RequestedScopes { + pub device_id: Option, + pub scopes: BTreeSet, } -impl RequestedScope { - #[must_use] - pub fn as_client_scope(&self) -> Option { - match self { - | Self::ClientApi => Some(OAuthClientScope::ApiFullAccess), - | Self::Device(_) => None, - | Self::ServerAdministration => Some(OAuthClientScope::ServerAdministration), - } - } -} - -impl PartialEq for RequestedScope { - fn eq(&self, other: &Self) -> bool { discriminant(self) == discriminant(other) } -} - -impl Eq for RequestedScope {} - -impl Hash for RequestedScope { - fn hash(&self, state: &mut H) { discriminant(self).hash(state); } -} - -impl Display for RequestedScope { +impl Display for RequestedScopes { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - | Self::ClientApi => write!(f, "urn:matrix:client:api:*"), - | Self::Device(device_id) => write!(f, "urn:matrix:client:device:{device_id}"), - | Self::ServerAdministration => - write!(f, "urn:matrix:client:cc.c10y.msc4484.server_administration"), + let mut scopes: Vec<_> = self + .scopes + .iter() + .map(OAuthClientScope::to_string) + .collect(); + + if let Some(device_id) = &self.device_id { + scopes.push(device_id.to_string()); } + + f.write_str(&scopes.join(" ")) } } @@ -126,7 +107,7 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { impl RawScopes { #[allow(clippy::trivial_regex)] - pub fn to_scopes(&self) -> Result, String> { + pub fn to_scopes(&self) -> Result { let client_api_token_regex = regex!(r"urn:matrix:(client|org.matrix.msc2967.client):api:\*"); let device_token_regex = @@ -134,17 +115,19 @@ pub fn to_scopes(&self) -> Result, String> { let server_administration_regex = regex!(r"urn:matrix:client:cc.c10y.msc4484.server_administration"); - let mut scopes = HashSet::new(); + let mut scopes = RequestedScopes::default(); for token in self.0.split(' ') { let scope_was_new = { if client_api_token_regex.is_match(token) { - scopes.insert(RequestedScope::ClientApi) + scopes.scopes.insert(OAuthClientScope::ApiFullAccess) } else if server_administration_regex.is_match(token) { - scopes.insert(RequestedScope::ServerAdministration) + scopes.scopes.insert(OAuthClientScope::ServerAdministration) } else if let Some(captures) = device_token_regex.captures(token) { scopes - .insert(RequestedScope::Device(captures.get(2).unwrap().as_str().into())) + .device_id + .replace(captures.get(2).unwrap().as_str().into()) + .is_none() } else { continue; } diff --git a/src/service/oauth/mod.rs b/src/service/oauth/mod.rs index 3ccb2c29b..8bf3b0e75 100644 --- a/src/service/oauth/mod.rs +++ b/src/service/oauth/mod.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeSet, HashMap, HashSet}, + collections::{BTreeSet, HashMap}, sync::{Arc, Mutex}, time::{Duration, SystemTime}, }; @@ -23,7 +23,7 @@ client_metadata::{ApplicationType, ClientMetadata, GrantType, ResponseType}, grant::{ AuthorizationCodeQuery, AuthorizationCodeResponse, CodeChallengeMethod, - DeviceCodeRequest, DeviceCodeResponse, ErrorCode, OAuthError, RequestedScope, + DeviceCodeRequest, DeviceCodeResponse, ErrorCode, OAuthError, RequestedScopes, ResponseMode, TokenRequest, TokenRequestType, TokenResponse, TokenType, }, }, @@ -56,12 +56,21 @@ struct Services { #[derive(Debug, Deserialize, Serialize)] pub struct SessionInfo { pub client_id: String, - // Ignore unknown scopes, old databases might have a device scope in here #[serde(deserialize_with = "client_metadata::btreeset_skip_err")] - pub scopes: BTreeSet, + scopes: BTreeSet, + #[serde(default)] + temporary_scopes: BTreeSet, + #[serde(default)] current_refresh_token: String, } +impl SessionInfo { + #[must_use] + pub fn scopes(&self) -> BTreeSet { + self.scopes.union(&self.temporary_scopes).cloned().collect() + } +} + #[derive(Debug, Deserialize, Serialize)] struct RefreshTokenInfo { client_id: String, @@ -71,7 +80,7 @@ struct RefreshTokenInfo { struct PendingAuthCodeGrant { authorizing_user: OwnedUserId, - requested_scopes: HashSet, + requested_scopes: RequestedScopes, client_name: Option, expected_client_id: String, expected_redirect_uri: Url, @@ -95,7 +104,7 @@ pub(crate) fn is_valid_for(&self, client_id: &str) -> bool { struct PendingDeviceCodeGrant { state: DeviceCodeGrantState, - requested_scopes: HashSet, + requested_scopes: RequestedScopes, client_name: Option, client_id: String, requested_at: SystemTime, @@ -127,7 +136,7 @@ pub(crate) fn is_valid_for(&self, client_id: &str) -> bool { pub struct DeviceCodeGrantInfo { pub device_code: String, pub client_metadata: ClientMetadata, - pub requested_scopes: HashSet, + pub requested_scopes: RequestedScopes, } /// A time-limited grant for a client to perform some sensitive action. @@ -544,79 +553,98 @@ pub async fn revoke_token(&self, token: String) -> Result<(), OAuthError> { async fn create_session( &self, authorizing_user: OwnedUserId, - requested_scopes: HashSet, + requested_scopes: RequestedScopes, client_name: Option, client_id: String, ) -> Result { - self.check_requested_scopes(&authorizing_user, &requested_scopes) + self.check_requested_scopes(&authorizing_user, &requested_scopes.scopes) .await?; let access_token = DeviceToken::new_random().with_max_age(Self::ACCESS_TOKEN_MAX_AGE); - let refresh_token = Self::generate_token(); + let response_scope = requested_scopes.to_string(); let device_id = requested_scopes - .iter() - .find_map(|scope| { - if let RequestedScope::Device(device_id) = scope { - Some(device_id.to_owned()) - } else { - None - } - }) + .device_id .ok_or_else(|| OAuthError::invalid_grant("No device ID scope supplied"))?; - if self + let session_info = if self .services .users .get_device_metadata(&authorizing_user, &device_id) .await .is_ok() { - return Err(OAuthError::new_static( - ErrorCode::InvalidScope, - "A device with the supplied ID already exists for this user", - )); - } + // We're doing step-up authentication for an existing device + let Some(mut session_info) = self + .get_session_info_for_device(&authorizing_user, &device_id) + .await + else { + // The device ID doesn't refer to an OAuth device + return Err(OAuthError::new_static( + ErrorCode::InvalidScope, + "A device with the supplied ID already exists for this user", + )); + }; - let device_id = self.services - .users - .create_device( - &authorizing_user, - Some(device_id), - Some(access_token.clone()), - client_name, - None, - ) - .await - // This can only panic if the authorizing user suffered a spontaneous existence - // failure during authentication, which should(?) be impossible(?) - .expect("failed to create device"); + session_info.temporary_scopes = requested_scopes.scopes; - info!( - ?client_id, - ?authorizing_user, - ?device_id, - ?requested_scopes, - "Created new oauth session" - ); + self.services + .users + .set_token(&authorizing_user, &device_id, access_token.clone()) + .await + .expect("should be able to set access token"); - self.db.userdeviceid_oauthsessioninfo.put( - (&authorizing_user, &device_id), - Json(SessionInfo { + info!( + ?client_id, + ?authorizing_user, + ?device_id, + scopes = ?session_info.temporary_scopes, + "Granted temporary scopes to device" + ); + + session_info + } else { + self.services + .users + .create_device( + &authorizing_user, + Some(device_id.clone()), + Some(access_token.clone()), + client_name, + None, + ) + .await + // This can only panic if the authorizing user suffered a spontaneous existence + // failure during authentication, which should(?) be impossible(?) + .expect("should be able to create device"); + + info!( + ?client_id, + ?authorizing_user, + ?device_id, + scopes = ?requested_scopes.scopes, + "Created new OAuth device" + ); + + SessionInfo { client_id: client_id.clone(), - current_refresh_token: refresh_token.clone(), - scopes: requested_scopes - .iter() - .filter_map(RequestedScope::as_client_scope) - .collect(), - }), - ); + current_refresh_token: Self::generate_token(), + scopes: requested_scopes.scopes, + temporary_scopes: BTreeSet::new(), + } + }; + + let refresh_token = session_info.current_refresh_token.clone(); + + self.db + .userdeviceid_oauthsessioninfo + .put((&authorizing_user, &device_id), Json(session_info)); self.db.refreshtoken_refreshtokeninfo.raw_put( &refresh_token, Json(RefreshTokenInfo { - client_id: client_id.clone(), - user_id: authorizing_user.clone(), + client_id, + user_id: authorizing_user, device_id, }), ); @@ -625,7 +653,7 @@ async fn create_session( access_token: access_token.into_token(), token_type: TokenType::Bearer, expires_in: Self::ACCESS_TOKEN_MAX_AGE.as_secs(), - scope: requested_scopes.iter().join(" "), + scope: response_scope, refresh_token, }) } @@ -658,9 +686,12 @@ async fn refresh_session( assert_eq!(&client_id, &session_info.client_id, "session info client id mismatch"); + // TODO: this doesn't include the device ID "scope", do any clients rely + // on that? + let scope = session_info.scopes.iter().join(" "); + let new_access_token = DeviceToken::new_random().with_max_age(Self::ACCESS_TOKEN_MAX_AGE); let new_refresh_token = Self::generate_token(); - let scope = session_info.scopes.iter().join(" "); session_info .current_refresh_token .clone_from(&new_refresh_token); @@ -675,6 +706,8 @@ async fn refresh_session( .await .expect("should be able to set token"); + session_info.temporary_scopes.clear(); + self.db.userdeviceid_oauthsessioninfo.put( (&refresh_token_info.user_id, &refresh_token_info.device_id), Json(session_info), @@ -712,9 +745,9 @@ pub async fn remove_session(&self, user_id: &UserId, device_id: &DeviceId) { async fn check_requested_scopes( &self, authorizing_user: &UserId, - requested_scopes: &HashSet, + scopes: &BTreeSet, ) -> Result<(), OAuthError> { - if requested_scopes.contains(&RequestedScope::ServerAdministration) + if scopes.contains(&OAuthClientScope::ServerAdministration) && !self.services.admin.user_is_admin(authorizing_user).await { return Err(OAuthError::new( diff --git a/src/web/pages/oauth/grant.rs b/src/web/pages/oauth/grant.rs index 6f1c46c73..bde0b7d48 100644 --- a/src/web/pages/oauth/grant.rs +++ b/src/web/pages/oauth/grant.rs @@ -5,8 +5,9 @@ routing::on, }; use conduwuit_service::oauth::{ + SessionInfo, client_metadata::ClientMetadata, - grant::{AuthorizationCodeQuery, DeviceCodeVerifyQuery, Prompt, RequestedScope}, + grant::{AuthorizationCodeQuery, DeviceCodeVerifyQuery, Prompt, RequestedScopes}, }; use ruma::{OwnedUserId, api::OAuthClientScope}; use serde::{Deserialize, de::IgnoredAny}; @@ -37,6 +38,7 @@ struct Grant use "grant.html.j2" { user_avatar: Avatar, client_metadata: ClientMetadata, scopes: ClientScopes, + existing_session_info: Option, device_code: Option } } @@ -102,13 +104,23 @@ async fn route_authorization_code( return Err(WebError::BadRequest("Invalid client ID".to_owned())); }; - let scopes = query - .scope - .to_scopes() - .map_err(WebError::BadRequest)? - .iter() - .filter_map(RequestedScope::as_client_scope) - .collect(); + let RequestedScopes { device_id, mut scopes } = + query.scope.to_scopes().map_err(WebError::BadRequest)?; + + let existing_session_info = if let Some(device_id) = &device_id { + services + .oauth + .get_session_info_for_device(&user_id, device_id) + .await + } else { + None + }; + + if let Some(existing_session_info) = &existing_session_info { + // For step-up auth, don't show scopes that the device already has + let existing_scopes = existing_session_info.scopes(); + scopes.retain(|scope| !existing_scopes.contains(scope)); + } let user_avatar = Avatar::for_local_user(&services, &user_id).await; @@ -124,6 +136,7 @@ async fn route_authorization_code( user_avatar, client, ClientScopes { scopes }, + existing_session_info, None, )) } @@ -189,11 +202,23 @@ async fn route_device_code( )); }; - let scopes = grant_info - .requested_scopes - .iter() - .filter_map(RequestedScope::as_client_scope) - .collect(); + let RequestedScopes { device_id, mut scopes } = grant_info.requested_scopes; + + let existing_session_info = if let Some(device_id) = &device_id { + services + .oauth + .get_session_info_for_device(&user_id, device_id) + .await + } else { + None + }; + + if let Some(existing_session_info) = &existing_session_info { + // For step-up auth, don't show scopes that the device already + // has + let existing_scopes = existing_session_info.scopes(); + scopes.retain(|scope| !existing_scopes.contains(scope)); + } let user_avatar = Avatar::for_local_user(&services, &user_id).await; @@ -209,6 +234,7 @@ async fn route_device_code( user_avatar, grant_info.client_metadata, ClientScopes { scopes }, + existing_session_info, Some(grant_info.device_code), )) }, diff --git a/src/web/pages/resources/grant.css b/src/web/pages/resources/grant.css index 157998753..8b95f1896 100644 --- a/src/web/pages/resources/grant.css +++ b/src/web/pages/resources/grant.css @@ -8,7 +8,6 @@ margin-inline: 1em; color: var(--secondary); font-size: x-large; - font-weight: bold; user-select: none; } } diff --git a/src/web/pages/templates/_components/client_scopes.html.j2 b/src/web/pages/templates/_components/client_scopes.html.j2 index ef814307e..6a4f8097c 100644 --- a/src/web/pages/templates/_components/client_scopes.html.j2 +++ b/src/web/pages/templates/_components/client_scopes.html.j2 @@ -4,7 +4,7 @@ {% when OAuthClientScope::ApiFullAccess %}
  • Access and control all features of your Matrix account
    • {% when OAuthClientScope::ServerAdministration %} -
  • Access your homeserver's administrator tools
    • +
  • ⚠️ Access your homeserver's administrator tools
    • {% when _ %}
  • missingno
    • {% endmatch %} diff --git a/src/web/pages/templates/device_info.html.j2 b/src/web/pages/templates/device_info.html.j2 index 53590c6a9..a8b242600 100644 --- a/src/web/pages/templates/device_info.html.j2 +++ b/src/web/pages/templates/device_info.html.j2 @@ -26,7 +26,7 @@ Device information

    {% if let Some((_, session_info)) = client_metadata %} This device has permission to: - {{ ClientScopes { scopes: session_info.scopes.clone() } }} + {{ ClientScopes { scopes: session_info.scopes() } }} {% else if device_card.dehydrated %} This is your dehydrated device. It saves encryption keys for you while you're offline. {% else %} @@ -45,7 +45,7 @@ Device information

    Scopes: - {% for scope in session_info.scopes %} + {% for scope in session_info.scopes() %} {{ scope | safe }} {% endfor %} diff --git a/src/web/pages/templates/grant.html.j2 b/src/web/pages/templates/grant.html.j2 index bc5a09624..23f59ef5e 100644 --- a/src/web/pages/templates/grant.html.j2 +++ b/src/web/pages/templates/grant.html.j2 @@ -27,10 +27,16 @@ Authorize client

    - {{ client_name }} ({{ client_metadata.client_uri.domain().unwrap() }}) would like - your permission to: - - {{ scopes }} + {{ client_name }} ({{ client_metadata.client_uri.domain().unwrap() }}) + {% if let Some(existing_session_info) = existing_session_info %} + would like temporary additional permissions to: + {{ scopes }} + {{ client_name }} already has permission to: + {{ ClientScopes { scopes: existing_session_info.scopes() } }} + {% else %} + would like your permission to: + {{ scopes }} + {% endif %}

    {% if scopes.scopes.contains(&OAuthClientScope::ServerAdministration) %}