diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index d4c22b550..37bcaa0c6 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -457,6 +457,17 @@ pub(crate) async fn post( // Now we can create the device on the homeserver, without holding the // transaction + // + // Normally, devices get synced to the homeserver in a `SyncDevicesJob` but we + // want the device to be created synchronously on the homeserver, so that + // when we respond, the access token works completely. If the device doesn't + // exist on the homeserver side, token introspection from Synapse to MAS + // will work but Synapse will return a 401 because it doesn't see the + // device. + // + // We're using an upsert so if the device already exists for some reason (like + // when we're replacing it, or a concurrent device sync happening) it won't + // have any effect. if let Err(err) = homeserver .upsert_device( &user.username, @@ -502,6 +513,7 @@ pub(crate) async fn post( /// Given the violations from [`Policy::evaluate_compat_login`], return the /// appropriate `RouteError` response. async fn process_violations_for_compat_login( + rng: &mut (dyn RngCore + Send), clock: &dyn Clock, repo: &mut BoxRepository, session_limit_config: Option<&SessionLimitConfig>, @@ -596,6 +608,11 @@ async fn process_violations_for_compat_login( .finish(clock, compat_session.to_owned()) .await?; } + + // Schedule a device sync with the homeserver + repo.queue_job() + .schedule_job(rng, clock, SyncDevicesJob::new_for_id(user.id)) + .await?; } else { // Tell the user about the limit return Err(RouteError::PolicyHardSessionLimitReached); @@ -840,6 +857,7 @@ async fn token_login( }) .await?; process_violations_for_compat_login( + rng, clock, repo, session_limit_config, @@ -966,8 +984,15 @@ async fn user_password_login( requester: policy_requester, }) .await?; - process_violations_for_compat_login(clock, repo, session_limit_config, &user, res.violations) - .await?; + process_violations_for_compat_login( + rng, + clock, + repo, + session_limit_config, + &user, + res.violations, + ) + .await?; let session = repo .compat_session() diff --git a/crates/handlers/src/compat/logout.rs b/crates/handlers/src/compat/logout.rs index 4642cc54b..8800fcf44 100644 --- a/crates/handlers/src/compat/logout.rs +++ b/crates/handlers/src/compat/logout.rs @@ -117,13 +117,21 @@ pub(crate) async fn post( // XXX: this is probably not the right error .ok_or(RouteError::InvalidAuthorization)?; + // This will make the access token invalid + repo.compat_session().finish(&clock, session).await?; + // Schedule a job to sync the devices of the user with the homeserver + // + // Doing this in a background job is ok as the access token will be invalid + // right away (from the session being finished above) and we do actually + // want to do a full device list sync (as opposed to + // `homeserver.delete_device(...)`), because we're not sure whether we want + // to delete the device (if there is for example a concurrent logout and + // login with the same device ID). repo.queue_job() .schedule_job(&mut rng, &clock, SyncDevicesJob::new(&user)) .await?; - repo.compat_session().finish(&clock, session).await?; - repo.save().await?; LOGOUT_COUNTER.add(1, &[KeyValue::new(RESULT, "success")]); diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index e070b640b..925f4209d 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -573,6 +573,15 @@ async fn authorization_code_grant( // Look for device to provision for scope in &*session.scope { if let Some(device) = Device::from_scope_token(scope) { + // Normally, devices get synced to the homeserver in a `SyncDevicesJob` but we + // want the device to be created synchronously on the homeserver, so + // that when we respond, the access token works completely. If the + // device doesn't exist on the homeserver side, token introspection + // from Synapse to MAS will work but Synapse will return a 401 + // because it doesn't see the device. + // + // We're using an upsert so if the device already exists for some reason + // (like when a concurrent device sync happening) it won't have any effect. homeserver .upsert_device( &browser_session.user.username, @@ -977,6 +986,15 @@ async fn device_code_grant( // Look for device to provision for scope in &*session.scope { if let Some(device) = Device::from_scope_token(scope) { + // Normally, devices get synced to the homeserver in a `SyncDevicesJob` but we + // want the device to be created synchronously on the homeserver, so + // that when we respond, the access token works completely. If the + // device doesn't exist on the homeserver side, token introspection + // from Synapse to MAS will work but Synapse will return a 401 + // because it doesn't see the device. + // + // We're using an upsert so if the device already exists for some reason + // (like when a concurrent device sync happening) it won't have any effect. homeserver .upsert_device(&browser_session.user.username, device.as_str(), None) .await