update sso details if changed, refactor

crates/lib/mysql_queries/src/queries/google_sign_in_accounts/mod.rs

@@ -1,2 +1,3 @@
 pub mod get_google_sign_in_account_by_subject;
-pub mod insert_google_sign_in_account;
+pub mod insert_google_sign_in_account;
+pub mod update_google_sign_in_account;

crates/lib/mysql_queries/src/queries/google_sign_in_accounts/update_google_sign_in_account.rs

@@ -0,0 +1,61 @@
+use sqlx;
+use sqlx::Executor;
+
+use crate::utils::transactor::Transactor;
+use errors::AnyhowResult;
+
+pub struct UpdateGoogleSignInArgs<'e, 't> {
+  pub subject: &'e str,
+
+  pub email_address: &'e str,
+
+  pub is_email_verified: bool,
+
+  pub maybe_locale: Option<&'e str>,
+
+  pub maybe_name: Option<&'e str>,
+  pub maybe_given_name: Option<&'e str>,
+  pub maybe_family_name: Option<&'e str>,
+
+  pub creator_ip_address: &'e str,
+
+  pub transactor: Transactor<'e, 't>,
+}
+
+pub async fn update_google_sign_in_account<'e, 't>(
+  args: UpdateGoogleSignInArgs<'e, 't>
+) -> AnyhowResult<()>
+{
+  let query = sqlx::query!(
+      r#"
+UPDATE google_sign_in_accounts
+SET
+  email_address = ?,
+  is_email_verified = ?,
+
+  maybe_locale = ?,
+
+  maybe_name = ?,
+  maybe_given_name = ?,
+  maybe_family_name = ?,
+
+  ip_address_update = ?,
+
+  version = version + 1
+WHERE
+  subject = ?
+        "#,
+    args.email_address,
+    args.is_email_verified,
+    args.maybe_locale,
+    args.maybe_name,
+    args.maybe_given_name,
+    args.maybe_family_name,
+    args.creator_ip_address,
+    args.subject,
+  );
+
+  let _r = args.transactor.execute(query).await?;
+
+  Ok(())
+}

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/google_sso_handler.rs

@@ -10,7 +10,7 @@ use std::fmt::Formatter;
 use crate::http_server::endpoints::beta_keys::redeem_beta_key_handler::RedeemBetaKeyError;
 use crate::http_server::endpoints::users::create_account_handler::{CreateAccountErrorResponse, CreateAccountSuccessResponse};
 use crate::http_server::endpoints::users::google_sso::check_claims::check_claims;
-use crate::http_server::endpoints::users::google_sso::handle_new_sso_account::handle_new_sso_account;
+use crate::http_server::endpoints::users::google_sso::handle_new_sso_account::{handle_new_sso_account, NewSsoArgs};
 use crate::http_server::endpoints::users::session_info_handler::SessionInfoError;
 use crate::http_server::session::http::http_user_session_manager::HttpUserSessionManager;
 use crate::http_server::validations::is_reserved_username::is_reserved_username;
@@ -48,7 +48,7 @@ use tokens::tokens::user_sessions::UserSessionToken;
 use tokens::tokens::users::UserToken;
 use user_input_common::check_for_slurs::contains_slurs;
 use utoipa::ToSchema;
-
+use crate::http_server::endpoints::users::google_sso::handle_existing_sso_link::{handle_existing_sso_link, ExistingLinkArgs};
 /* ALGORITHM
 --> [SSO RECORD LOOKUP]
   --> I. SSO Record Exists
@@ -93,12 +93,12 @@ use utoipa::ToSchema;
    - Other email providers may behave weirdly with their own canonicalization schemes.
 
  TODO:
-  - Handle existing account linkage
-  - Should we store canonical emails?
-  - User source enum
   - Store all the claims info
   - Verify all paths (including legacy sign up paths)
   - Update claims info on re-login (if diff exists)
+
+ Decisions:
+  - Should we store canonical emails? --> No, you can create more than one account per gmail.
  */
 
 #[derive(ToSchema, Deserialize)]
@@ -211,13 +211,20 @@ pub async fn google_sso_handler(
     claims.email(),
     claims.email_verified());
 
-  let subject = claims.subject()
+  let claims_subject = claims.subject()
       .map(|s| s.to_string())
       .ok_or_else(|| {
         warn!("no subject in google claims");
         GoogleCreateAccountErrorResponse::bad_request()
       })?;
 
+  let claims_email_address = claims.email()
+      .map(|email| email.to_string())
+      .ok_or_else(|| {
+        warn!("no email address in google claims");
+        GoogleCreateAccountErrorResponse::bad_request()
+      })?;
+
   let mut mysql_connection = mysql_pool.acquire()
       .await
       .map_err(|e| {
@@ -225,7 +232,7 @@ pub async fn google_sso_handler(
         GoogleCreateAccountErrorResponse::server_error()
       })?;
 
-  let maybe_sso_account = get_google_sign_in_account(&subject, &mut *mysql_connection)
+  let maybe_sso_account = get_google_sign_in_account(&claims_subject, &mut *mysql_connection)
       .await
       .map_err(|err| {
         warn!("error getting google sign in account: {:?}", err);
@@ -238,29 +245,25 @@ pub async fn google_sso_handler(
 
   match maybe_sso_account {
     Some(sso_account) => {
-      let user_token = match sso_account.maybe_user_token {
-        Some(token) => token.clone(),
-        None => {
-          // NB: If accounts get into this state (e.g. if we support de-linking), we'll need to
-          // consider how to migrate accounts and handle all the various account states.
-          // For now, we'll just deny this possibility.
-          warn!("no user token for existing google sign in account!");
-          return Err(GoogleCreateAccountErrorResponse::server_error());
-        },
-      };
-
-      // TODO: Handle updating SSO details (eg new profile picture).
+      let user_token = handle_existing_sso_link(ExistingLinkArgs {
+        http_request: &http_request,
+        sso_account: &sso_account,
+        claims,
+        claims_email_address: &claims_email_address,
+        mysql_connection: &mut mysql_connection,
+      }).await?;
 
       maybe_user_token = Some(user_token);
       maybe_user_display_name = sso_account.maybe_user_display_name.clone();
     },
     None => {
-      let result = handle_new_sso_account(
-        &http_request,
-        &subject,
-        &mut mysql_connection,
+      let result = handle_new_sso_account(NewSsoArgs {
+        http_request: &http_request,
         claims,
-      ).await?;
+        claims_subject: &claims_subject,
+        claims_email_address: &claims_email_address,
+        mysql_connection: &mut mysql_connection,
+      }).await?;
 
       maybe_user_token = Some(result.user_token);
       maybe_user_display_name = Some(result.user_display_name);

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/handle_creating_user_account.rs

@@ -21,7 +21,7 @@ pub struct CreateArgs<'a> {
   pub http_request: &'a HttpRequest,
   pub claims: Claims,
   pub claims_subject: &'a str,
-  pub claims_email: &'a str,
+  pub claims_email_address: &'a str,
   pub user_email_address: &'a str,
   pub mysql_connection: &'a mut PoolConnection<MySql>,
 }
@@ -98,7 +98,7 @@ pub async fn handle_creating_user_account(
   let _token = insert_google_sign_in_account(InsertGoogleSignInArgs {
     subject: args.claims_subject,
     maybe_user_token: Some(&user_token),
-    email_address: &args.claims_email, // NB: The one from the Google claims, not our canonicalized one.
+    email_address: &args.claims_email_address, // NB: The one from the Google claims, not our canonicalized one.
     is_email_verified: args.claims.email_verified(),
     maybe_locale: args.claims.locale(),
     maybe_name: args.claims.name(),

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/handle_existing_sso_link.rs

@@ -0,0 +1,67 @@
+use crate::http_server::endpoints::users::google_sso::google_sso_handler::GoogleCreateAccountErrorResponse;
+use crate::http_server::endpoints::users::google_sso::handle_new_sso_account::NewSsoAccountInfo;
+use actix_web::HttpRequest;
+use google_sign_in::claims::claims::Claims;
+use http_server_common::request::get_request_ip::get_request_ip;
+use log::warn;
+use mysql_queries::queries::google_sign_in_accounts::get_google_sign_in_account_by_subject::GoogleSignInAccount;
+use mysql_queries::queries::google_sign_in_accounts::update_google_sign_in_account::{update_google_sign_in_account, UpdateGoogleSignInArgs};
+use mysql_queries::utils::transactor::Transactor;
+use sqlx::pool::PoolConnection;
+use sqlx::MySql;
+use tokens::tokens::users::UserToken;
+
+pub struct ExistingLinkArgs<'a> {
+  pub http_request: &'a HttpRequest,
+  pub sso_account: &'a GoogleSignInAccount,
+  pub claims: Claims,
+  pub claims_email_address: &'a str,
+  pub mysql_connection: &'a mut PoolConnection<MySql>,
+}
+
+pub async fn handle_existing_sso_link(
+  args: ExistingLinkArgs<'_>
+)
+  -> Result<UserToken, GoogleCreateAccountErrorResponse>
+{
+  let user_token = match &args.sso_account.maybe_user_token {
+    Some(token) => token.clone(),
+    None => {
+      // NB: If accounts get into this state (e.g. if we support de-linking), we'll need to
+      // consider how to migrate accounts and handle all the various account states.
+      // For now, we'll just deny this possibility.
+      warn!("no user token for existing google sign in account!");
+      return Err(GoogleCreateAccountErrorResponse::server_error());
+    },
+  };
+
+  if should_update_sso_claims(args.sso_account, &args.claims) {
+    let ip_address = get_request_ip(args.http_request);
+
+    update_google_sign_in_account(UpdateGoogleSignInArgs {
+      subject: &args.sso_account.subject,
+      email_address: args.claims_email_address,
+      is_email_verified: args.claims.email_verified(),
+      maybe_locale: args.claims.locale(),
+      maybe_name: args.claims.name(),
+      maybe_given_name: args.claims.given_name(),
+      maybe_family_name: args.claims.family_name(),
+      creator_ip_address: &ip_address,
+      transactor: Transactor::for_connection(args.mysql_connection),
+    }).await.map_err(|err| {
+      warn!("error updating google sign in account: {:?}", err);
+      GoogleCreateAccountErrorResponse::server_error()
+    })?;
+  }
+
+  Ok(user_token)
+}
+
+fn should_update_sso_claims(sso_account: &GoogleSignInAccount, claims: &Claims) -> bool {
+  sso_account.email_address.as_deref() != claims.email()
+      || sso_account.is_email_verified != claims.email_verified()
+      || sso_account.maybe_locale.as_deref() != claims.locale()
+      || sso_account.maybe_name.as_deref() != claims.name()
+      || sso_account.maybe_given_name.as_deref() != claims.given_name()
+      || sso_account.maybe_family_name.as_deref() != claims.family_name()
+}

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/handle_linking_existing_account.rs

@@ -13,6 +13,7 @@ pub struct LinkArgs<'a> {
   pub http_request: &'a HttpRequest,
   pub claims: Claims,
   pub claims_subject: &'a str,
+  pub claims_email_address: &'a str,
   pub user_account: UserRecordForLogin,
   pub mysql_connection: &'a mut PoolConnection<MySql>,
 }
@@ -21,8 +22,6 @@ pub async fn handle_linking_existing_account(
 )
   -> Result<NewSsoAccountInfo, GoogleCreateAccountErrorResponse>
 {
-  // TODO: Double check email address in the claims before linking!
-
   let mut transaction = args.mysql_connection.begin()
       .await
       .map_err(|e| {
@@ -35,7 +34,7 @@ pub async fn handle_linking_existing_account(
   let _token = insert_google_sign_in_account(InsertGoogleSignInArgs {
     subject: args.claims_subject,
     maybe_user_token: Some(&args.user_account.token),
-    email_address: "todo@todo.com", // TODO // NB: The one from the Google claims, not our canonicalized one.
+    email_address: args.claims_email_address,
     is_email_verified: args.claims.email_verified(),
     maybe_locale: args.claims.locale(),
     maybe_name: args.claims.name(),

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/handle_new_sso_account.rs

@@ -21,6 +21,14 @@ use sqlx::{Acquire, MySql};
 use tokens::tokens::user_sessions::UserSessionToken;
 use tokens::tokens::users::UserToken;
 
+pub struct NewSsoArgs<'a> {
+  pub http_request: &'a HttpRequest,
+  pub claims: Claims,
+  pub claims_subject: &'a str,
+  pub claims_email_address: &'a str,
+  pub mysql_connection: &'a mut PoolConnection<MySql>,
+}
+
 pub struct NewSsoAccountInfo {
   pub user_token: UserToken,
   pub user_display_name: String,
@@ -28,28 +36,18 @@ pub struct NewSsoAccountInfo {
 }
 
 pub async fn handle_new_sso_account(
-  http_request: &HttpRequest,
-  claims_subject: &str,
-  mysql_connection: &mut PoolConnection<MySql>,
-  claims: Claims,
+  args: NewSsoArgs<'_>
 )
   -> Result<NewSsoAccountInfo, GoogleCreateAccountErrorResponse>
 {
-  let claims_email_address = claims.email()
-      .map(|email| email.to_string())
-      .ok_or_else(|| {
-        warn!("no email address in google claims");
-        GoogleCreateAccountErrorResponse::bad_request()
-      })?;
-
   // NB: We use this routine to "normalize" email addresses in the user table.
   // It won't necessarily match the email address in the Google claims.
   // A better normalization function in the future may handle dots and plus
   // signs in Gmail addresses, for instance.
-  let user_email_address = canonicalize_email_for_users_table(&claims_email_address);
+  let user_email_address = canonicalize_email_for_users_table(&args.claims_email_address);
 
   let maybe_user_account =
-      lookup_user_for_login_by_email_with_transactor(&user_email_address, Transactor::for_connection(mysql_connection))
+      lookup_user_for_login_by_email_with_transactor(&user_email_address, Transactor::for_connection(args.mysql_connection))
           .await
           .map_err(|err| {
             warn!("error looking up user by email: {:?}", err);
@@ -59,21 +57,22 @@ pub async fn handle_new_sso_account(
   match maybe_user_account {
     Some(user_account) => {
       handle_linking_existing_account(LinkArgs {
-        http_request,
-        claims,
-        claims_subject,
+        http_request: args.http_request,
+        claims: args.claims,
+        claims_subject: args.claims_subject,
+        claims_email_address: &args.claims_email_address,
         user_account,
-        mysql_connection,
+        mysql_connection: args.mysql_connection,
       }).await
     },
     None => {
       handle_creating_user_account(CreateArgs {
-        http_request,
-        claims,
-        claims_subject,
-        claims_email: &claims_email_address,
+        http_request: args.http_request,
+        claims: args.claims,
+        claims_subject: args.claims_subject,
+        claims_email_address: &args.claims_email_address,
         user_email_address: &user_email_address,
-        mysql_connection,
+        mysql_connection: args.mysql_connection,
       }).await
     },
   }

crates/service/web/storyteller_web/src/http_server/endpoints/users/google_sso/mod.rs

@@ -1,5 +1,6 @@
 pub mod check_claims;
 pub mod google_sso_handler;
+pub mod handle_creating_user_account;
+pub mod handle_existing_sso_link;
 pub mod handle_linking_existing_account;
-pub mod handle_new_sso_account;
-mod handle_creating_user_account;
+pub mod handle_new_sso_account;