Skip to content

Commit 85bade2

Browse files
committed
fix: use information-hiding for cross-user provider resolution
Non-admin callers who try to resolve a provider name owned by another user (via suffix scan) now get NotFound rather than PermissionDenied. This prevents leaking resource existence to unauthorized callers and ensures consistent behavior when a user deletes their own provider while another user has the same name. The PermissionDenied path remains active for the shared/legacy fallback (Step 2) where the record's owner label is directly checked — this covers providers stored under raw names with explicit ownership. Admin callers continue to resolve any user's provider via suffix scan. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent 0884c53 commit 85bade2

2 files changed

Lines changed: 11 additions & 9 deletions

File tree

crates/openshell-server/src/auth/ownership.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,19 +291,18 @@ pub async fn resolve_scoped_name(
291291
return Ok(shared);
292292
}
293293

294-
// Step 3: Both missed — suffix scan for cross-user detection
294+
// Step 3: Both missed — suffix scan for cross-user detection.
295+
// Admin callers get access to any user's provider. Non-admin callers
296+
// get None (surfaced as NotFound) for information-hiding security.
295297
let candidates = store
296298
.find_by_name_suffix(object_type, name)
297299
.await
298300
.map_err(|e| Status::internal(format!("suffix search failed: {e}")))?;
299301

300-
if let Some(record) = candidates.into_iter().next() {
301-
if is_admin {
302+
if is_admin {
303+
if let Some(record) = candidates.into_iter().next() {
302304
return Ok(Some(record));
303305
}
304-
return Err(Status::permission_denied(
305-
"provider is owned by another user",
306-
));
307306
}
308307

309308
Ok(None)

crates/openshell-server/src/grpc/provider.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5078,11 +5078,12 @@ mod tests {
50785078
.await
50795079
.unwrap();
50805080

5081+
// Information-hiding: non-owner sees NotFound, not PermissionDenied
50815082
let err =
50825083
handle_get_provider(&state, get_request_with_principal("alice-provider", &bob))
50835084
.await
50845085
.unwrap_err();
5085-
assert_eq!(err.code(), Code::PermissionDenied);
5086+
assert_eq!(err.code(), Code::NotFound);
50865087
}
50875088

50885089
#[tokio::test]
@@ -5250,10 +5251,11 @@ mod tests {
52505251
credential_expires_at_ms: HashMap::new(),
52515252
};
52525253

5254+
// Information-hiding: non-owner sees NotFound, not PermissionDenied
52535255
let err = handle_update_provider(&state, update_request_with_principal(update, &bob))
52545256
.await
52555257
.unwrap_err();
5256-
assert_eq!(err.code(), Code::PermissionDenied);
5258+
assert_eq!(err.code(), Code::NotFound);
52575259
}
52585260

52595261
#[tokio::test]
@@ -5286,13 +5288,14 @@ mod tests {
52865288
.await
52875289
.unwrap();
52885290

5291+
// Information-hiding: non-owner sees NotFound, not PermissionDenied
52895292
let err = handle_delete_provider(
52905293
&state,
52915294
delete_request_with_principal("alice-provider", &bob),
52925295
)
52935296
.await
52945297
.unwrap_err();
5295-
assert_eq!(err.code(), Code::PermissionDenied);
5298+
assert_eq!(err.code(), Code::NotFound);
52965299
}
52975300

52985301
#[tokio::test]

0 commit comments

Comments
 (0)