Allow setting an explicit upstream account name (#3600)

crates/cli/src/commands/manage.rs

@@ -955,9 +955,10 @@ impl UserCreationRequest<'_> {
         }
 
         for (provider, subject) in upstream_provider_mappings {
+            // Note that we don't pass a human_account_name here, as we don't ask for it
             let link = repo
                 .upstream_oauth_link()
-                .add(rng, clock, provider, subject)
+                .add(rng, clock, provider, subject, None)
                 .await?;
 
             repo.upstream_oauth_link()

crates/cli/src/sync.rs

@@ -67,6 +67,9 @@ fn map_claims_imports(
                 mas_data_model::UpsreamOAuthProviderSetEmailVerification::Import
             }
         },
+        account_name: mas_data_model::UpstreamOAuthProviderSubjectPreference {
+            template: config.account_name.template.clone(),
+        },
     }
 }
 

crates/config/src/sections/upstream_oauth2.rs

@@ -285,6 +285,23 @@ impl EmailImportPreference {
     }
 }
 
+/// What should be done for the account name attribute
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)]
+pub struct AccountNameImportPreference {
+    /// The Jinja2 template to use for the account name. This name is only used
+    /// for display purposes.
+    ///
+    /// If not provided, it will be ignored.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub template: Option<String>,
+}
+
+impl AccountNameImportPreference {
+    const fn is_default(&self) -> bool {
+        self.template.is_none()
+    }
+}
+
 /// How claims should be imported
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)]
 pub struct ClaimsImports {
@@ -307,6 +324,13 @@ pub struct ClaimsImports {
     /// `email_verified` claims
     #[serde(default, skip_serializing_if = "EmailImportPreference::is_default")]
     pub email: EmailImportPreference,
+
+    /// Set a human-readable name for the upstream account for display purposes
+    #[serde(
+        default,
+        skip_serializing_if = "AccountNameImportPreference::is_default"
+    )]
+    pub account_name: AccountNameImportPreference,
 }
 
 impl ClaimsImports {

crates/data-model/src/upstream_oauth2/link.rs

@@ -14,5 +14,6 @@ pub struct UpstreamOAuthLink {
     pub provider_id: Ulid,
     pub user_id: Option<Ulid>,
     pub subject: String,
+    pub human_account_name: Option<String>,
     pub created_at: DateTime<Utc>,
 }

crates/data-model/src/upstream_oauth2/provider.rs

@@ -301,10 +301,14 @@ pub struct ClaimsImports {
     #[serde(default)]
     pub email: ImportPreference,
 
+    #[serde(default)]
+    pub account_name: SubjectPreference,
+
     #[serde(default)]
     pub verify_email: SetEmailVerification,
 }
 
+// XXX: this should have another name
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
 pub struct SubjectPreference {
     #[serde(default)]

crates/handlers/src/upstream_oauth2/callback.rs

@@ -359,7 +359,7 @@ pub(crate) async fn handler(
         .as_deref()
         .unwrap_or("{{ user.sub }}");
     let subject = env
-        .render_str(template, context)
+        .render_str(template, context.clone())
         .map_err(RouteError::ExtractSubject)?;
 
     if subject.is_empty() {
@@ -375,8 +375,26 @@ pub(crate) async fn handler(
     let link = if let Some(link) = maybe_link {
         link
     } else {
+        // Try to render the human account name if we have one,
+        // but just log if it fails
+        let human_account_name = provider
+            .claims_imports
+            .account_name
+            .template
+            .as_deref()
+            .and_then(|template| match env.render_str(template, context) {
+                Ok(name) => Some(name),
+                Err(e) => {
+                    tracing::warn!(
+                        error = &e as &dyn std::error::Error,
+                        "Failed to render account name"
+                    );
+                    None
+                }
+            });
+
         repo.upstream_oauth_link()
-            .add(&mut rng, &clock, &provider, subject)
+            .add(&mut rng, &clock, &provider, subject, human_account_name)
             .await?
     };
 

crates/handlers/src/upstream_oauth2/link.rs

@@ -332,7 +332,7 @@ pub(crate) async fn get(
                 .await?
                 .ok_or(RouteError::ProviderNotFound)?;
 
-            let ctx = UpstreamRegister::default();
+            let ctx = UpstreamRegister::new(link.clone(), provider.clone());
 
             let env = environment();
 
@@ -596,7 +596,7 @@ pub(crate) async fn post(
                 .map_or(false, |v| v == "true");
 
             // Create a template context in case we need to re-render because of an error
-            let ctx = UpstreamRegister::default();
+            let ctx = UpstreamRegister::new(link.clone(), provider.clone());
 
             let display_name = if provider
                 .claims_imports
@@ -954,7 +954,13 @@ mod tests {
 
         let link = repo
             .upstream_oauth_link()
-            .add(&mut rng, &state.clock, &provider, "subject".to_owned())
+            .add(
+                &mut rng,
+                &state.clock,
+                &provider,
+                "subject".to_owned(),
+                None,
+            )
             .await
             .unwrap();
 

crates/storage-pg/migrations/20241129091057_upstream_oauth2_link_account_name.sql

@@ -0,0 +1,9 @@
+-- Copyright 2024 New Vector Ltd.
+--
+-- SPDX-License-Identifier: AGPL-3.0-only
+-- Please see LICENSE in the repository root for full details.
+
+-- Add the human_account_name column to the upstream_oauth_links table to store
+-- a human-readable name for the upstream account
+ALTER TABLE "upstream_oauth_links"
+  ADD COLUMN "human_account_name" TEXT;

crates/storage-pg/src/iden.rs

@@ -122,5 +122,6 @@ pub enum UpstreamOAuthLinks {
     UpstreamOAuthProviderId,
     UserId,
     Subject,
+    HumanAccountName,
     CreatedAt,
 }

crates/storage-pg/src/upstream_oauth2/link.rs

@@ -47,6 +47,7 @@ struct LinkLookup {
     upstream_oauth_provider_id: Uuid,
     user_id: Option<Uuid>,
     subject: String,
+    human_account_name: Option<String>,
     created_at: DateTime<Utc>,
 }
 
@@ -57,6 +58,7 @@ impl From<LinkLookup> for UpstreamOAuthLink {
             provider_id: Ulid::from(value.upstream_oauth_provider_id),
             user_id: value.user_id.map(Ulid::from),
             subject: value.subject,
+            human_account_name: value.human_account_name,
             created_at: value.created_at,
         }
     }
@@ -124,6 +126,7 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
                     upstream_oauth_provider_id,
                     user_id,
                     subject,
+                    human_account_name,
                     created_at
                 FROM upstream_oauth_links
                 WHERE upstream_oauth_link_id = $1
@@ -163,6 +166,7 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
                     upstream_oauth_provider_id,
                     user_id,
                     subject,
+                    human_account_name,
                     created_at
                 FROM upstream_oauth_links
                 WHERE upstream_oauth_provider_id = $1
@@ -186,6 +190,7 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
             db.query.text,
             upstream_oauth_link.id,
             upstream_oauth_link.subject = subject,
+            upstream_oauth_link.human_account_name = human_account_name,
             %upstream_oauth_provider.id,
             %upstream_oauth_provider.issuer,
             %upstream_oauth_provider.client_id,
@@ -198,6 +203,7 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
         clock: &dyn Clock,
         upstream_oauth_provider: &UpstreamOAuthProvider,
         subject: String,
+        human_account_name: Option<String>,
     ) -> Result<UpstreamOAuthLink, Self::Error> {
         let created_at = clock.now();
         let id = Ulid::from_datetime_with_source(created_at.into(), rng);
@@ -210,12 +216,14 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
                     upstream_oauth_provider_id,
                     user_id,
                     subject,
+                    human_account_name,
                     created_at
-                ) VALUES ($1, $2, NULL, $3, $4)
+                ) VALUES ($1, $2, NULL, $3, $4, $5)
             "#,
             Uuid::from(id),
             Uuid::from(upstream_oauth_provider.id),
             &subject,
+            human_account_name.as_deref(),
             created_at,
         )
         .traced()
@@ -227,6 +235,7 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
             provider_id: upstream_oauth_provider.id,
             user_id: None,
             subject,
+            human_account_name,
             created_at,
         })
     }
@@ -300,6 +309,13 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
                 Expr::col((UpstreamOAuthLinks::Table, UpstreamOAuthLinks::Subject)),
                 LinkLookupIden::Subject,
             )
+            .expr_as(
+                Expr::col((
+                    UpstreamOAuthLinks::Table,
+                    UpstreamOAuthLinks::HumanAccountName,
+                )),
+                LinkLookupIden::HumanAccountName,
+            )
             .expr_as(
                 Expr::col((UpstreamOAuthLinks::Table, UpstreamOAuthLinks::CreatedAt)),
                 LinkLookupIden::CreatedAt,

crates/storage-pg/src/upstream_oauth2/mod.rs

@@ -124,7 +124,7 @@ mod tests {
         // Create a link
         let link = repo
             .upstream_oauth_link()
-            .add(&mut rng, &clock, &provider, "a-subject".to_owned())
+            .add(&mut rng, &clock, &provider, "a-subject".to_owned(), None)
             .await
             .unwrap();
 

crates/storage/src/upstream_oauth2/link.rs

@@ -128,6 +128,7 @@ pub trait UpstreamOAuthLinkRepository: Send + Sync {
     /// * `upsream_oauth_provider`: The upstream OAuth provider for which to
     ///   create the link
     /// * `subject`: The subject of the upstream OAuth link to create
+    /// * `human_account_name`: A human-readable name for the upstream account
     ///
     /// # Errors
     ///
@@ -138,6 +139,7 @@ pub trait UpstreamOAuthLinkRepository: Send + Sync {
         clock: &dyn Clock,
         upstream_oauth_provider: &UpstreamOAuthProvider,
         subject: String,
+        human_account_name: Option<String>,
     ) -> Result<UpstreamOAuthLink, Self::Error>;
 
     /// Associate an upstream OAuth link to a user
@@ -201,6 +203,7 @@ repository_impl!(UpstreamOAuthLinkRepository:
         clock: &dyn Clock,
         upstream_oauth_provider: &UpstreamOAuthProvider,
         subject: String,
+        human_account_name: Option<String>,
     ) -> Result<UpstreamOAuthLink, Self::Error>;
 
     async fn associate_to_user(