diff --git a/packages/electoral-log/src/messages/message.rs b/packages/electoral-log/src/messages/message.rs index 612bdfba1be..a204d2d9aac 100644 --- a/packages/electoral-log/src/messages/message.rs +++ b/packages/electoral-log/src/messages/message.rs @@ -91,10 +91,6 @@ impl Message { /// `StatementBody::ExternalReconciliation`. Named for the general /// capability, not the specific integration (Datafix) that first needed /// it. - /// Unlike most `Message::*_message` constructors, this calls [`Self::sign`] - /// directly instead of [`Self::from_body`] so `artifact` (the JSON of - /// old/new values applied, for a `ChangesApplied` entry) can be carried — - /// `from_body` always signs with `artifact: None`. #[instrument(skip_all, err)] pub fn external_reconciliation_message( event_id: EventIdString, @@ -103,7 +99,6 @@ impl Message { generated_at: ExternalReconciliationGeneratedAtString, input_hash: ExternalReconciliationInputHashString, output_hash: ExternalReconciliationOutputHashString, - artifact: Option>, sd: &SigningData, user_id: Option, username: Option, @@ -116,21 +111,10 @@ impl Message { input_hash, output_hash, ); - let head = StatementHead::from_body(event_id, &body); - let statement = Statement::new(head, body); - Message::sign( - statement, - artifact, - &sd.sender_sk, - &sd.sender_name, - &sd.system_sk, - user_id, - username, - None, /* election_id: a reconciliation run is event-wide, not tied to one election */ - None, /* area_id */ - None, /* ballot_id */ - ) + // A reconciliation run is event-wide, so no election, area or ballot + // id is attached. + Message::from_body(event_id, body, sd, user_id, username, None, None, None) } pub fn cast_vote_message( diff --git a/packages/electoral-log/src/messages/statement.rs b/packages/electoral-log/src/messages/statement.rs index 6d579818c48..b7241df054a 100644 --- a/packages/electoral-log/src/messages/statement.rs +++ b/packages/electoral-log/src/messages/statement.rs @@ -425,10 +425,8 @@ pub enum StatementBody { /// integration (Datafix) that first needed it. Doesn't fit /// `ExternalApiRequest`'s shape: there is no HTTP call to the external /// system involved, since it is offline for the whole freeze period a - /// reconciliation run happens during. The JSON of every applied voter's - /// old/new values is carried in `Message.artifact` on the - /// `ChangesApplied` entry — there is exactly one entry per phase per run, - /// not one per voter. + /// reconciliation run happens during. There is exactly one entry per + /// phase per run, not one per voter. ExternalReconciliation( EventIdString, ExternalReconciliationKind, diff --git a/packages/windmill/src/services/electoral_log.rs b/packages/windmill/src/services/electoral_log.rs index 43e08c7654b..b408b70e145 100644 --- a/packages/windmill/src/services/electoral_log.rs +++ b/packages/windmill/src/services/electoral_log.rs @@ -738,10 +738,8 @@ impl ElectoralLog { /// generation or applying the Sequent-side diff) — see /// `windmill::services::external::reconciliation`. Named for the general /// capability, not the specific integration (Datafix) that first needed - /// it. `artifact` carries the JSON of old/new values applied, for a - /// `ChangesApplied` entry (`None` for `PatchGenerated`, which has nothing - /// to apply yet). - #[instrument(skip(self, artifact), fields(kind = %kind), err)] + /// it. + #[instrument(skip(self), fields(kind = %kind), err)] pub async fn post_external_reconciliation( &self, event_id: String, @@ -750,7 +748,6 @@ impl ElectoralLog { generated_at: i64, input_sha256: String, output_sha256: Option, - artifact: Option>, user_id: Option, username: Option, ) -> Result<()> { @@ -763,7 +760,6 @@ impl ElectoralLog { ExternalReconciliationGeneratedAtString(generated_at.to_string()), ExternalReconciliationInputHashString(input_sha256), ExternalReconciliationOutputHashString(output_sha256), - artifact, &self.sd, user_id, username, diff --git a/packages/windmill/src/services/external/api_datafix.rs b/packages/windmill/src/services/external/api_datafix.rs index 582972a1d40..8a4a314968e 100644 --- a/packages/windmill/src/services/external/api_datafix.rs +++ b/packages/windmill/src/services/external/api_datafix.rs @@ -373,6 +373,13 @@ pub async fn replace_voter_pin( .generate_password(username); let password = Some(pin.clone()); + // edit_user defaults a missing `temporary` to `true`; Datafix-issued PINs + // should default to `false` unless the annotation says otherwise. + let temporary = match datafix_annotations.password_policy.temporary { + Some(temporary) => Some(temporary), + None => Some(false), + }; + let client = KeycloakAdminClient::new().await.map_err(|e| { error!("Error getting KeycloakAdminClient: {e:?}"); DatafixResponse::error(DatafixErrorCode::InternalError) @@ -382,7 +389,7 @@ pub async fn replace_voter_pin( .edit_user( realm, &user_id, None, // Enable/disable None, // attributes - None, None, None, None, password, None, + None, None, None, None, password, temporary, ) .await .map_err(|e| { diff --git a/packages/windmill/src/services/external/datafix_types.rs b/packages/windmill/src/services/external/datafix_types.rs index 01f8a417436..a5ae84881f9 100644 --- a/packages/windmill/src/services/external/datafix_types.rs +++ b/packages/windmill/src/services/external/datafix_types.rs @@ -181,6 +181,10 @@ pub struct PasswordPolicy { base: BasePolicy, size: usize, characters: CharactersPolicy, + /// Whether the generated PIN is a Keycloak temporary credential (forcing + /// a change on next login). `None` when the annotation omits it; callers + /// decide their own default rather than relying on Keycloak's. + pub temporary: Option, } impl PasswordPolicy { diff --git a/packages/windmill/src/services/external/reconciliation/bulk_create.rs b/packages/windmill/src/services/external/reconciliation/bulk_create.rs index 9f7c34f8e02..446c0b7265b 100644 --- a/packages/windmill/src/services/external/reconciliation/bulk_create.rs +++ b/packages/windmill/src/services/external/reconciliation/bulk_create.rs @@ -161,9 +161,9 @@ async fn resolve_area_ids_bulk( } /// Bulk-inserts every `VOTER_ADDED` voter in `voters` directly into -/// Keycloak's tables. Returns the same `(applied_items, row_failures)` shape -/// `run_apply_reconciliation_patch` already collects from the sequential -/// Admin-API path, so the two paths merge into one report. +/// Keycloak's tables. Returns the same `(voter_username, reason)` row +/// failures `run_apply_reconciliation_patch` already collects from the +/// sequential Admin-API path, so the two paths merge into one report. #[instrument(skip_all, fields(voter_count = voters.len()), err)] pub async fn apply_voters_added_bulk( hasura_transaction: &Transaction<'_>, @@ -173,12 +173,11 @@ pub async fn apply_voters_added_bulk( realm: &str, voter_group_name: &str, voters: &HashMap>, -) -> Result<(Vec, Vec<(String, String)>)> { - let mut applied_items = Vec::new(); +) -> Result> { let mut row_failures: Vec<(String, String)> = Vec::new(); if voters.is_empty() { - return Ok((applied_items, row_failures)); + return Ok(row_failures); } let mut pending = Vec::with_capacity(voters.len()); @@ -245,7 +244,7 @@ pub async fn apply_voters_added_bulk( } if ready.is_empty() { - return Ok((applied_items, row_failures)); + return Ok(row_failures); } let realm_id = get_realm_id(keycloak_transaction, realm.to_string()) @@ -258,11 +257,7 @@ pub async fn apply_voters_added_bulk( { Ok(inserted_usernames) => { for (candidate, _area_id) in batch { - if inserted_usernames.contains(&candidate.voter_username) { - if let Some(items) = voters.get(&candidate.voter_username) { - applied_items.extend(items.clone()); - } - } else { + if !inserted_usernames.contains(&candidate.voter_username) { row_failures.push(( candidate.voter_username.clone(), "Voter already existed in Keycloak (skipped by ON CONFLICT)" @@ -283,7 +278,7 @@ pub async fn apply_voters_added_bulk( } } - Ok((applied_items, row_failures)) + Ok(row_failures) } #[instrument(skip(keycloak_transaction), err)] diff --git a/packages/windmill/src/tasks/apply_reconciliation_patch.rs b/packages/windmill/src/tasks/apply_reconciliation_patch.rs index d6ebc9b9e92..bd3350e2749 100644 --- a/packages/windmill/src/tasks/apply_reconciliation_patch.rs +++ b/packages/windmill/src/tasks/apply_reconciliation_patch.rs @@ -21,7 +21,6 @@ use crate::services::electoral_log::ElectoralLog; use crate::services::external::reconciliation::apply::{apply_voter_changes, VoterApplyOutcome}; use crate::services::external::reconciliation::bulk_create::apply_voters_added_bulk; use crate::services::external::reconciliation::diff::{DiffItem, ReconciliationApplyEnvelope}; -use crate::services::external::reconciliation::patch::DiffItemArrayWriter; use crate::services::external::types::{ReconciliationChangeCategory, ReconciliationPatchSource}; use crate::services::external::utils::set_datafix_reconciliation_state; use crate::services::protocol_manager::get_event_board; @@ -36,7 +35,7 @@ use sequent_core::types::hasura::extra::TasksExecutionStatus; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fs::File; -use std::io::{BufReader, BufWriter, Write}; +use std::io::BufReader; use tracing::{info, instrument}; const VOTER_ADD_APPLY_BATCH_SIZE: usize = 5_000; @@ -348,17 +347,6 @@ async fn run_apply_reconciliation_patch( let mut applied_voters_count: usize = 0; let mut pending_voters_added: HashMap> = HashMap::new(); - // Applied old/new values are also streamed to disk. The electoral-log - // API ultimately needs one byte artifact, but no second Vec is - // retained while the apply itself runs. - let audit_temp = tempfile::NamedTempFile::new() - .map_err(|err| format!("Error creating reconciliation audit artifact: {err}"))?; - let audit_file = audit_temp - .reopen() - .map_err(|err| format!("Error opening reconciliation audit artifact: {err}"))?; - let mut audit_writer = DiffItemArrayWriter::start(BufWriter::new(audit_file)) - .map_err(|err| format!("Error starting reconciliation audit artifact: {err}"))?; - // Consume one contiguous voter group at a time. `VoterGroupTracker` // rejects any voter that reappears after its first group, making the // generator/apply ordering contract self-enforcing. @@ -398,7 +386,6 @@ async fn run_apply_reconciliation_patch( voter_username, current_items, &mut pending_voters_added, - &mut audit_writer, &mut applied_voters_count, &mut row_failures, ) @@ -411,23 +398,13 @@ async fn run_apply_reconciliation_patch( &realm, &voter_group_name, &mut pending_voters_added, - &mut audit_writer, &mut applied_voters_count, &mut row_failures, ) .await?; // Electoral log: every apply attempt gets a run-level entry, including a - // run where every row failed. The artifact contains only old/new items - // that were actually applied. - let mut audit_file = audit_writer - .finish() - .map_err(|err| format!("Error finishing reconciliation audit artifact: {err}"))?; - audit_file - .flush() - .map_err(|err| format!("Error flushing reconciliation audit artifact: {err}"))?; - let artifact = std::fs::read(audit_temp.path()) - .map_err(|err| format!("Error reading reconciliation audit artifact: {err}"))?; + // run where every row failed. let slug = std::env::var("ENV_SLUG").map_err(|err| format!("Missing ENV_SLUG: {err}"))?; let board_name = get_event_board(&body.tenant_id, &body.election_event_id, &slug); let electoral_log = ElectoralLog::new( @@ -446,7 +423,6 @@ async fn run_apply_reconciliation_patch( envelope.generated_at, envelope.source_sha256.clone(), None, - Some(artifact), Some(body.applied_by_user_id.clone()), body.applied_by_username.clone(), ) @@ -479,7 +455,7 @@ async fn run_apply_reconciliation_patch( } #[allow(clippy::too_many_arguments)] -async fn process_voter_group( +async fn process_voter_group( hasura_transaction: &deadpool_postgres::Transaction<'_>, keycloak_transaction: &deadpool_postgres::Transaction<'_>, body: &ApplyReconciliationPatchBody, @@ -488,7 +464,6 @@ async fn process_voter_group( voter_username: String, voter_items: Vec, pending_voters_added: &mut HashMap>, - audit_writer: &mut DiffItemArrayWriter, applied_voters_count: &mut usize, row_failures: &mut RowFailureSummary, ) -> std::result::Result<(), String> { @@ -523,7 +498,6 @@ async fn process_voter_group( realm, voter_group_name, pending_voters_added, - audit_writer, applied_voters_count, row_failures, ) @@ -542,12 +516,7 @@ async fn process_voter_group( ) .await { - Ok(VoterApplyOutcome::Applied) => { - *applied_voters_count += 1; - audit_writer - .write_batch(voter_items.iter()) - .map_err(|err| format!("Error writing reconciliation audit artifact: {err}"))?; - } + Ok(VoterApplyOutcome::Applied) => *applied_voters_count += 1, Ok(VoterApplyOutcome::Failed { reason }) => { row_failures.record(voter_username, reason); } @@ -557,14 +526,13 @@ async fn process_voter_group( } #[allow(clippy::too_many_arguments)] -async fn flush_voters_added( +async fn flush_voters_added( hasura_transaction: &deadpool_postgres::Transaction<'_>, keycloak_transaction: &deadpool_postgres::Transaction<'_>, body: &ApplyReconciliationPatchBody, realm: &str, voter_group_name: &str, pending_voters_added: &mut HashMap>, - audit_writer: &mut DiffItemArrayWriter, applied_voters_count: &mut usize, row_failures: &mut RowFailureSummary, ) -> std::result::Result<(), String> { @@ -573,7 +541,7 @@ async fn flush_voters_added( } let voters_added = std::mem::take(pending_voters_added); let voters_added_count = voters_added.len(); - let (bulk_applied, bulk_failures) = apply_voters_added_bulk( + let bulk_failures = apply_voters_added_bulk( hasura_transaction, keycloak_transaction, &body.tenant_id, @@ -585,9 +553,6 @@ async fn flush_voters_added( .await .map_err(|err| format!("Error bulk-creating added voters: {err:?}"))?; *applied_voters_count += voters_added_count - bulk_failures.len(); - audit_writer - .write_batch(bulk_applied.iter()) - .map_err(|err| format!("Error writing reconciliation audit artifact: {err}"))?; row_failures.extend(bulk_failures); Ok(()) } diff --git a/packages/windmill/src/tasks/generate_reconciliation_patches.rs b/packages/windmill/src/tasks/generate_reconciliation_patches.rs index 83d34b42c98..4804a6bc4da 100644 --- a/packages/windmill/src/tasks/generate_reconciliation_patches.rs +++ b/packages/windmill/src/tasks/generate_reconciliation_patches.rs @@ -490,7 +490,6 @@ async fn run_generate_reconciliation_patches( meta.generated_at, source_sha256.clone(), external_patch_sha256.clone(), - None, Some(body.requested_by_user_id.clone()), body.requested_by_username.clone(), )