Skip to content

Commit ef0b308

Browse files
VibeSWEcodex
andcommitted
fix(rbac): publish durable metadata despite staging failures
Remote metadata is authoritative at runtime and startup. Log a failed local staging refresh with tenant context and return success once the durable write commits, so callers continue their live map and session updates. Keep durable write errors fatal and route the standalone role and OIDC writers through the shared helper. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
1 parent 7bd2159 commit ef0b308

3 files changed

Lines changed: 26 additions & 25 deletions

File tree

‎src/handlers/http/modal/utils/rbac_utils.rs‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,35 @@ pub async fn get_metadata(
3535
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
3636
}
3737

38+
/// Persist authoritative metadata and refresh its best-effort local staging copy.
39+
/// Success lets callers publish the committed change to live RBAC state.
3840
pub async fn put_metadata(
3941
metadata: &StorageMetadata,
4042
tenant_id: &Option<String>,
4143
) -> Result<(), ObjectStorageError> {
42-
put_metadata_with_writers(storage::put_remote_metadata(metadata, tenant_id), || {
43-
storage::put_staging_metadata(metadata, tenant_id)
44-
})
44+
put_metadata_with_writers(
45+
storage::put_remote_metadata(metadata, tenant_id),
46+
|| storage::put_staging_metadata(metadata, tenant_id),
47+
tenant_id,
48+
)
4549
.await
4650
}
4751

4852
async fn put_metadata_with_writers(
4953
remote_write: impl Future<Output = Result<(), ObjectStorageError>>,
5054
staging_write: impl FnOnce() -> io::Result<()>,
55+
tenant_id: &Option<String>,
5156
) -> Result<(), ObjectStorageError> {
5257
remote_write.await?;
53-
staging_write()?;
58+
// A staging failure cannot undo the durable commit. Returning an error here
59+
// would leave callers' live authorization state behind the source of truth.
60+
if let Err(error) = staging_write() {
61+
tracing::error!(
62+
error = %error,
63+
tenant_id = ?tenant_id,
64+
"Failed to refresh staging metadata after durable metadata write"
65+
);
66+
}
5467
Ok(())
5568
}
5669

@@ -80,6 +93,7 @@ mod tests {
8093
"staging is read-only",
8194
))
8295
},
96+
&None,
8397
)
8498
.await;
8599

@@ -95,6 +109,7 @@ mod tests {
95109
let result = put_metadata_with_writers(
96110
async { Err(ObjectStorageError::Custom("durable write failed".into())) },
97111
|| panic!("staging must not be changed after a failed durable write"),
112+
&None,
98113
)
99114
.await;
100115

@@ -117,6 +132,7 @@ mod tests {
117132
staging_written.set(true);
118133
Ok(())
119134
},
135+
&None,
120136
)
121137
.await;
122138

‎src/handlers/http/oidc.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ use crate::{
4747
handlers::{
4848
COOKIE_AGE_DAYS, SESSION_COOKIE_NAME, USER_COOKIE_NAME, USER_ID_COOKIE_NAME,
4949
http::{
50-
API_BASE_PATH, API_VERSION, cluster::sync_user_creation, modal::OIDC_CLIENT,
50+
API_BASE_PATH, API_VERSION,
51+
cluster::sync_user_creation,
52+
modal::{OIDC_CLIENT, utils::rbac_utils::put_metadata},
5153
rbac::UPDATE_LOCK,
5254
},
5355
},
@@ -58,7 +60,7 @@ use crate::{
5860
map::{DEFAULT_ROLE, SessionKey, mut_sessions, mut_users, roles, write_user_groups},
5961
user::{self, GroupUser, User, UserType},
6062
},
61-
storage::{self, ObjectStorageError, StorageMetadata},
63+
storage::{ObjectStorageError, StorageMetadata},
6264
utils::{
6365
actix::extract_session_key_from_req, get_tenant_id_from_key, get_tenant_id_from_request,
6466
},
@@ -825,15 +827,6 @@ async fn get_metadata(
825827
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
826828
}
827829

828-
async fn put_metadata(
829-
metadata: &StorageMetadata,
830-
tenant_id: &Option<String>,
831-
) -> Result<(), ObjectStorageError> {
832-
storage::put_remote_metadata(metadata, tenant_id).await?;
833-
storage::put_staging_metadata(metadata, tenant_id)?;
834-
Ok(())
835-
}
836-
837830
#[derive(Debug, thiserror::Error)]
838831
pub enum OIDCError {
839832
#[error("Failed to connect to storage: {0}")]

‎src/handlers/http/role.rs‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use actix_web::{
2525
web::{self, Json},
2626
};
2727

28+
use super::modal::utils::rbac_utils::put_metadata;
2829
use crate::rbac::map::roles;
2930
use crate::rbac::role::model::{Role, RoleType, RoleUI};
3031
use crate::{
@@ -34,7 +35,7 @@ use crate::{
3435
map::{DEFAULT_ROLE, mut_roles, mut_sessions, mut_users, read_user_groups, users},
3536
user::UserType,
3637
},
37-
storage::{self, ObjectStorageError, StorageMetadata},
38+
storage::{ObjectStorageError, StorageMetadata},
3839
utils::get_tenant_id_from_request,
3940
validator::{self, error::UsernameValidationError},
4041
};
@@ -302,15 +303,6 @@ async fn get_metadata(
302303
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
303304
}
304305

305-
async fn put_metadata(
306-
metadata: &StorageMetadata,
307-
tenant_id: &Option<String>,
308-
) -> Result<(), ObjectStorageError> {
309-
storage::put_remote_metadata(metadata, tenant_id).await?;
310-
storage::put_staging_metadata(metadata, tenant_id)?;
311-
Ok(())
312-
}
313-
314306
#[derive(Debug, thiserror::Error)]
315307
pub enum RoleError {
316308
#[error("Default OIDC role must name an existing role.")]

0 commit comments

Comments
 (0)