Schedule SyncDevicesJob after dangerous_hard_limit_eviction so the homeserver stays up to date (#5679)

Follow-up to https://github.com/element-hq/matrix-authentication-service/pull/5607#discussion_r3189451555

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()

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")]);

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