Skip to content

Commit b1f7ec0

Browse files
committed
🐛 fix(http): honor If-Match conditions
Management mutations accepted one numeric tag, treated weak tags as strong matches, and reported repository mismatches as conflicts instead of failed preconditions. Parse the RFC field once and carry its typed condition into the write transaction. This keeps existence checks, strong comparison, and mutation atomic while preserving authorization concealment.
1 parent b52139f commit b1f7ec0

16 files changed

Lines changed: 506 additions & 177 deletions

File tree

crates/peryx-driver/src/authz.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
1212
use peryx_events::security::{AuthorizationDenial, authorization_denied};
1313
use peryx_identity::{GrantScope, Resource, Role, RoleGrant, Scope, UserId, grants_permit};
14-
use peryx_storage::meta::{CreateGrantOutcome, MetaError, MetaStore, RoleGrantPage};
14+
use peryx_storage::meta::{CreateGrantOutcome, MetaError, MetaStore, RoleGrantPage, VersionPrecondition};
1515
pub use peryx_storage::meta::{
1616
DeleteGrantOutcome, RoleGrantFilter, RoleGrantOrigin, RoleGrantQuery, RoleGrantQueryError, RoleGrantStoreError,
1717
StoredRoleGrant, role_grant_reach,
@@ -109,8 +109,12 @@ impl AuthorizationService {
109109

110110
/// # Errors
111111
/// Returns a store error when the transaction cannot commit.
112-
pub fn delete_managed_grant(&self, id: &str, expected_version: u64) -> Result<DeleteGrantOutcome, MetaError> {
113-
self.store.delete_managed_grant(id, expected_version)
112+
pub fn delete_managed_grant(
113+
&self,
114+
id: &str,
115+
precondition: impl Into<VersionPrecondition>,
116+
) -> Result<DeleteGrantOutcome, MetaError> {
117+
self.store.delete_managed_grant(id, precondition)
114118
}
115119

116120
/// # Errors

crates/peryx-driver/src/http_services.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub use peryx_storage::meta::{
2020
CreateRepositoryError, NewRepository, PolicyDecisionItem, PolicyDecisionPage, PolicyDecisionQuery,
2121
PolicyDecisionQueryError, PolicyInputGeneration, RepositoryFieldError, RepositoryId, RepositoryPage,
2222
RepositoryQuery, RepositoryQueryError, RepositoryRecord, RepositoryState, RepositoryStateError, RepositoryUpdate,
23-
UpdateRepositoryError,
23+
UpdateRepositoryError, VersionPrecondition,
2424
};
2525

2626
pub trait RepositoryService: Send + Sync {
@@ -45,7 +45,7 @@ pub trait RepositoryService: Send + Sync {
4545
fn update(
4646
&self,
4747
id: &RepositoryId,
48-
expected: u64,
48+
precondition: VersionPrecondition,
4949
update: RepositoryUpdate,
5050
actor: &peryx_identity::UserId,
5151
now: i64,
@@ -57,7 +57,7 @@ pub trait RepositoryService: Send + Sync {
5757
fn set_enabled(
5858
&self,
5959
id: &RepositoryId,
60-
expected: u64,
60+
precondition: VersionPrecondition,
6161
enabled: bool,
6262
actor: &peryx_identity::UserId,
6363
now: i64,
@@ -282,23 +282,23 @@ impl RepositoryService for StoreServices {
282282
fn update(
283283
&self,
284284
id: &RepositoryId,
285-
expected: u64,
285+
precondition: VersionPrecondition,
286286
update: RepositoryUpdate,
287287
actor: &peryx_identity::UserId,
288288
now: i64,
289289
) -> Result<RepositoryRecord, UpdateRepositoryError> {
290-
self.meta.update_repository(id, expected, update, actor, now)
290+
self.meta.update_repository(id, precondition, update, actor, now)
291291
}
292292

293293
fn set_enabled(
294294
&self,
295295
id: &RepositoryId,
296-
expected: u64,
296+
precondition: VersionPrecondition,
297297
enabled: bool,
298298
actor: &peryx_identity::UserId,
299299
now: i64,
300300
) -> Result<RepositoryRecord, RepositoryStateError> {
301-
self.meta.set_repository_enabled(id, expected, enabled, actor, now)
301+
self.meta.set_repository_enabled(id, precondition, enabled, actor, now)
302302
}
303303
}
304304

crates/peryx-driver/tests/http_services_contract.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::time::Duration;
55
use peryx_core::Ecosystem;
66
use peryx_driver::http_services::{
77
HttpDomainServices, NewRepository, PolicyDecisionQuery, RepositoryQuery, RepositoryService, RepositoryState,
8-
RepositoryStateError, RepositoryUpdate, StoreServices,
8+
RepositoryStateError, RepositoryUpdate, StoreServices, VersionPrecondition,
99
};
1010
use peryx_driver::retention::{RetentionExport, RetentionQuery};
1111
use peryx_driver::serving::RetentionDriver;
@@ -104,7 +104,7 @@ fn repository_service_owns_the_complete_store_lifecycle() {
104104
let updated = service
105105
.update(
106106
&created.id,
107-
1,
107+
VersionPrecondition::exact(1),
108108
RepositoryUpdate {
109109
display_name: "Renamed".to_owned(),
110110
definition: serde_json::json!({"visible": true}),
@@ -113,15 +113,17 @@ fn repository_service_owns_the_complete_store_lifecycle() {
113113
2,
114114
)
115115
.unwrap();
116-
let disabled = service.set_enabled(&created.id, 2, false, &actor, 3).unwrap();
116+
let disabled = service
117+
.set_enabled(&created.id, VersionPrecondition::exact(2), false, &actor, 3)
118+
.unwrap();
117119

118120
assert_eq!(listed.repositories, vec![created.clone()]);
119121
assert_eq!(inspected, created);
120122
assert_eq!((updated.display_name.as_str(), updated.version), ("Renamed", 2));
121123
assert_eq!((disabled.state, disabled.version), (RepositoryState::Disabled, 3));
122124
assert!(matches!(
123-
service.set_enabled(&disabled.id, 2, true, &actor, 4),
124-
Err(RepositoryStateError::VersionConflict { current: 3 })
125+
service.set_enabled(&disabled.id, VersionPrecondition::exact(2), true, &actor, 4),
126+
Err(RepositoryStateError::PreconditionFailed { current: Some(3) })
125127
));
126128
}
127129

crates/peryx-driver/tests/unit/tests/authz_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ fn test_managed_grant_lifecycle_uses_versions() {
143143
assert_eq!(
144144
service.delete_managed_grant(&id, created.record.version + 1).unwrap(),
145145
DeleteGrantOutcome::PreconditionFailed {
146-
current: created.record.version
146+
current: Some(created.record.version)
147147
}
148148
);
149149
assert!(matches!(

crates/peryx-http/src/handlers/grants.rs

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use peryx_identity::{GrantScope, Role, RoleGrant, UserId, can_manage_grants, par
1717

1818
use crate::response_security::ProtectedCachePolicy;
1919

20+
use super::IfMatchError;
21+
2022
const MAX_BODY: usize = 4 * 1024;
2123
const DEFAULT_LIMIT: usize = 25;
2224

@@ -154,37 +156,37 @@ pub async fn revoke_grant(State(state): State<Arc<AppState>>, headers: HeaderMap
154156
Ok(caller) => caller,
155157
Err(response) => return response,
156158
};
157-
let expected = match headers.get(header::IF_MATCH) {
158-
None => {
159+
if !administers(&id, &grants) {
160+
return StatusCode::NOT_FOUND.into_response();
161+
}
162+
let precondition = match super::if_match(&headers) {
163+
Ok(precondition) => precondition,
164+
Err(IfMatchError::Missing) => {
159165
return problem(
160166
StatusCode::PRECONDITION_REQUIRED,
161167
"revocation requires an If-Match precondition",
162168
);
163169
}
164-
Some(value) => match value.to_str().ok().and_then(parse_etag) {
165-
Some(version) => version,
166-
None => return problem(StatusCode::BAD_REQUEST, "invalid If-Match precondition"),
167-
},
170+
Err(IfMatchError::Malformed) => {
171+
return problem(StatusCode::BAD_REQUEST, "invalid If-Match precondition");
172+
}
168173
};
169-
if !administers(&id, &grants) {
170-
return StatusCode::NOT_FOUND.into_response();
171-
}
172-
match state.serving.authorization.delete_managed_grant(&id, expected) {
174+
match state.serving.authorization.delete_managed_grant(&id, precondition) {
173175
Ok(DeleteGrantOutcome::Removed(stored)) => {
174176
record(&actor, RoleGrantChange::Revoke, &stored.grant, "allowed", "removed");
175177
StatusCode::NO_CONTENT.into_response()
176178
}
177-
Ok(DeleteGrantOutcome::PreconditionFailed { current }) => (
178-
StatusCode::PRECONDITION_FAILED,
179-
[(header::ETAG, etag(current))],
180-
axum::Json(serde_json::json!({"error": "grant version precondition failed"})),
181-
)
182-
.into_response(),
179+
Ok(DeleteGrantOutcome::PreconditionFailed { current }) => {
180+
let mut response = problem(StatusCode::PRECONDITION_FAILED, "grant version precondition failed");
181+
if let Some(current) = current {
182+
response.headers_mut().insert(header::ETAG, etag(current));
183+
}
184+
response
185+
}
183186
Ok(DeleteGrantOutcome::ExternallyManaged { link_id }) => problem(
184187
StatusCode::CONFLICT,
185188
&format!("grant is managed by external identity link {link_id}"),
186189
),
187-
Ok(DeleteGrantOutcome::NotFound) => StatusCode::NOT_FOUND.into_response(),
188190
Err(_) => unavailable(),
189191
}
190192
}
@@ -268,10 +270,6 @@ fn reach_label(scope: &GrantScope) -> String {
268270
}
269271
}
270272

271-
fn parse_etag(value: &str) -> Option<u64> {
272-
value.trim().trim_matches('"').parse().ok()
273-
}
274-
275273
fn etag(version: u64) -> HeaderValue {
276274
HeaderValue::from_str(&format!("\"{version}\"")).expect("a version etag is a valid header value")
277275
}

crates/peryx-http/src/handlers/mod.rs

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! Longest-prefix routing dispatches repository traffic without encoding ecosystem paths here.
22
3+
use std::collections::BTreeSet;
4+
35
mod acl;
46
mod analytics;
57
mod discover;
@@ -22,6 +24,7 @@ mod usage;
2224
use axum::http::{HeaderMap, StatusCode, header};
2325
use axum::response::{IntoResponse, Response};
2426
use mediatype::{MediaType, names};
27+
use peryx_driver::http_services::VersionPrecondition;
2528
use peryx_driver::state::{AppState, Index};
2629
use peryx_identity::{Action, Denial};
2730

@@ -77,6 +80,96 @@ fn is_json(headers: &HeaderMap) -> bool {
7780
.is_some_and(|media_type| media_type.ty == names::APPLICATION && media_type.subty == names::JSON)
7881
}
7982

83+
#[derive(Clone, Copy)]
84+
enum IfMatchError {
85+
Missing,
86+
Malformed,
87+
}
88+
89+
fn if_match(headers: &HeaderMap) -> Result<VersionPrecondition, IfMatchError> {
90+
let fields = headers.get_all(header::IF_MATCH);
91+
let mut values = fields.iter();
92+
let first = values.next().ok_or(IfMatchError::Missing)?;
93+
let mut wildcard_count = 0;
94+
let mut saw_tag = false;
95+
let mut versions = BTreeSet::new();
96+
for value in std::iter::once(first).chain(values) {
97+
parse_if_match_field(value.as_bytes(), &mut wildcard_count, &mut saw_tag, &mut versions)?;
98+
}
99+
if wildcard_count > 1 || wildcard_count == 1 && saw_tag {
100+
return Err(IfMatchError::Malformed);
101+
}
102+
Ok(if wildcard_count == 1 {
103+
VersionPrecondition::Exists
104+
} else {
105+
VersionPrecondition::Versions(versions)
106+
})
107+
}
108+
109+
// Commas are valid inside opaque tags: https://www.rfc-editor.org/rfc/rfc9110.html#section-8.8.3
110+
fn parse_if_match_field(
111+
field: &[u8],
112+
wildcard_count: &mut usize,
113+
saw_tag: &mut bool,
114+
versions: &mut BTreeSet<u64>,
115+
) -> Result<(), IfMatchError> {
116+
let mut index = 0;
117+
while index < field.len() {
118+
while field.get(index).is_some_and(|byte| matches!(byte, b' ' | b'\t')) {
119+
index += 1;
120+
}
121+
if index == field.len() {
122+
break;
123+
}
124+
if field[index] == b',' {
125+
index += 1;
126+
continue;
127+
}
128+
if field[index] == b'*' {
129+
*wildcard_count += 1;
130+
} else {
131+
let weak = field[index..].starts_with(b"W/");
132+
if weak {
133+
index += 2;
134+
}
135+
if field.get(index) != Some(&b'"') {
136+
return Err(IfMatchError::Malformed);
137+
}
138+
index += 1;
139+
let start = index;
140+
while field.get(index).is_some_and(|byte| *byte != b'"') {
141+
if !matches!(field[index], b'!' | b'#'..=b'~' | b'\x80'..=b'\xff') {
142+
return Err(IfMatchError::Malformed);
143+
}
144+
index += 1;
145+
}
146+
if field.get(index) != Some(&b'"') {
147+
return Err(IfMatchError::Malformed);
148+
}
149+
*saw_tag = true;
150+
if !weak
151+
&& let Ok(value) = std::str::from_utf8(&field[start..index])
152+
&& let Ok(version) = value.parse::<u64>()
153+
&& version.to_string() == value
154+
{
155+
versions.insert(version);
156+
}
157+
}
158+
index += 1;
159+
while field.get(index).is_some_and(|byte| matches!(byte, b' ' | b'\t')) {
160+
index += 1;
161+
}
162+
if index == field.len() {
163+
break;
164+
}
165+
if field[index] != b',' {
166+
return Err(IfMatchError::Malformed);
167+
}
168+
index += 1;
169+
}
170+
Ok(())
171+
}
172+
80173
/// HTTP handlers distinguish an authenticated denial from a missing or invalid credential.
81174
enum EcosystemCredentialDenied {
82175
Forbidden,

0 commit comments

Comments
 (0)