Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP INDEX IF EXISTS users_upstream_id_active_uniq_idx;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Prevent concurrent user creations (e.g. parallel TMC-server create-user requests during
-- password migration) from inserting duplicate users for the same TMC account. Partial so that
-- soft-deleted users don't block re-creating an account with the same upstream_id.
--
-- In production, create this index with CONCURRENTLY before applying this migration (a plain
-- CREATE INDEX blocks writes to users while it builds; CONCURRENTLY cannot run inside the
-- migration transaction):
-- CREATE UNIQUE INDEX CONCURRENTLY users_upstream_id_active_uniq_idx ON users (upstream_id)
-- WHERE upstream_id IS NOT NULL AND deleted_at IS NULL;
--
-- If this fails, duplicate active users with the same upstream_id already exist and must be
-- merged manually first:
-- SELECT upstream_id, array_agg(id) FROM users
-- WHERE upstream_id IS NOT NULL AND deleted_at IS NULL
-- GROUP BY upstream_id HAVING count(*) > 1;
CREATE UNIQUE INDEX IF NOT EXISTS users_upstream_id_active_uniq_idx ON users (upstream_id)
WHERE upstream_id IS NOT NULL
AND deleted_at IS NULL;
8 changes: 8 additions & 0 deletions services/headless-lms/models/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,14 @@ impl From<sqlx::Error> for ModelError {
err.to_string(),
Some(err.into()),
),
"users_upstream_id_active_uniq_idx" => ModelError::new(
ModelErrorType::DatabaseConstraint {
constraint: constraint.to_string(),
description: "A user with this upstream id already exists.",
},
err.to_string(),
Some(err.into()),
),
"unique_chatbot_names_within_course" => ModelError::new(
ModelErrorType::DatabaseConstraint {
constraint: constraint.to_string(),
Expand Down
42 changes: 29 additions & 13 deletions services/headless-lms/server/src/domain/authorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,24 +942,40 @@ pub async fn get_or_create_user_from_tmc_mooc_fi_response(
let user = match models::users::find_by_upstream_id(conn, upstream_id).await? {
Some(existing_user) => existing_user,
None => {
models::users::insert_with_upstream_id_and_moocfi_id(
let inserted = models::users::insert_with_upstream_id_and_moocfi_id(
conn,
&email,
// convert empty names to None
if user_field.first_name.trim().is_empty() {
None
} else {
Some(user_field.first_name.as_str())
},
if user_field.last_name.trim().is_empty() {
None
} else {
Some(user_field.last_name.as_str())
},
// convert missing/empty names to None
user_field
.first_name
.as_deref()
.filter(|s| !s.trim().is_empty()),
user_field
.last_name
.as_deref()
.filter(|s| !s.trim().is_empty()),
upstream_id,
id,
)
.await?
.await;
match inserted {
Ok(user) => user,
// A concurrent request can create the user between the find and the insert
// (the insert runs in a savepoint, so the connection stays usable). The unique
// index on upstream_id rejects the loser; return the winner's row instead.
Err(insert_error)
if matches!(
insert_error.error_type(),
models::ModelErrorType::DatabaseConstraint { constraint, .. }
if constraint == "users_upstream_id_active_uniq_idx"
) =>
{
models::users::find_by_upstream_id(conn, upstream_id)
.await?
.ok_or(insert_error)?
}
Err(insert_error) => return Err(insert_error.into()),
}
}
};
Ok(user)
Expand Down
16 changes: 12 additions & 4 deletions services/headless-lms/utils/src/services/tmc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,22 @@ pub struct TMCUser {
pub email: String,
pub administrator: bool,
pub courses_mooc_fi_user_id: Option<Uuid>,
#[serde(default)]
pub user_field: TMCUserField,
}

#[derive(Debug, Serialize, Deserialize)]
/// User fields are optional data on the TMC side: a user who never filled in their profile (or a
/// TMC instance without the field definitions) serializes them as null or omits them entirely, so
/// deserialization must not require them.
#[derive(Debug, Default, Serialize, Deserialize)]
pub struct TMCUserField {
pub first_name: String,
pub last_name: String,
pub organizational_id: String,
#[serde(default)]
pub first_name: Option<String>,
#[serde(default)]
pub last_name: Option<String>,
#[serde(default)]
pub organizational_id: Option<String>,
#[serde(default)]
pub course_announcements: bool,
}

Expand Down
Loading