mirror of
https://tangled.org/tranquil.farm/tranquil-pds
synced 2026-09-24 19:24:15 +00:00
fix: handle wildcard + aud in OAuth scopes
`rpc:*?aud=did:web:api.bsky.app#bsky_appview` fails in the permissions check even though it's a valid scope. This makes it resolve correctly. Also this commit fixes a bug where `rpc` as a bare scope is interpreted as all methods for all services, but it's really an invalid scope. Can drop it if it's a problem, but it just looks like a bug in the implementation rather than something intended? Note that the parser treats `rpc:*` as "allow everything for no services", but aud is actually required, so it's not a valid scope? Didn't mess with it though because delegation uses it and that part of the code treats it as allow everything for every service. We should maybe split the scope parsers so we have one for delegated scopes and one for regular? However, to avoid the same bug as bare `rpc`, I changed the enforcement step in "regular" OAuth to disallow `rpc:*` (this matches the existing behavior on main, no actual change, just a little acknowledgement).
This commit is contained in:
@@ -92,11 +92,7 @@ fn blob_covers(g: &BlobScope, r: &BlobScope) -> bool {
|
||||
}
|
||||
|
||||
fn rpc_covers(g: &RpcScope, r: &RpcScope) -> bool {
|
||||
let lxm_ok = match &g.lxm {
|
||||
None => true,
|
||||
Some(gl) if gl == "*" => true,
|
||||
Some(gl) => r.lxm.as_deref() == Some(gl.as_str()),
|
||||
};
|
||||
let lxm_ok = g.lxm == "*" || g.lxm == r.lxm;
|
||||
let aud_ok = match &g.aud {
|
||||
None => true,
|
||||
Some(ga) if ga == "*" => true,
|
||||
|
||||
@@ -93,7 +93,7 @@ impl BlobScope {
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RpcScope {
|
||||
pub lxm: Option<String>,
|
||||
pub lxm: String,
|
||||
pub aud: Option<String>,
|
||||
}
|
||||
|
||||
@@ -245,9 +245,10 @@ pub fn parse_scope(scope: &str) -> ParsedScope {
|
||||
let lxm = lxm_positional.or_else(|| params.get("lxm").and_then(|v| v.first().cloned()));
|
||||
let aud = params.get("aud").and_then(|v| v.first().cloned());
|
||||
|
||||
let is_lxm_wildcard = lxm.as_deref() == Some("*") || lxm.is_none();
|
||||
let is_aud_wildcard = aud.as_deref() == Some("*");
|
||||
if is_lxm_wildcard && is_aud_wildcard {
|
||||
let Some(lxm) = lxm else {
|
||||
return ParsedScope::Unknown(scope.to_string());
|
||||
};
|
||||
if lxm == "*" && aud.as_deref() == Some("*") {
|
||||
return ParsedScope::Unknown(scope.to_string());
|
||||
}
|
||||
|
||||
@@ -400,7 +401,7 @@ mod tests {
|
||||
let scope = parse_scope("rpc:app.bsky.feed.getTimeline?aud=did:web:api.bsky.app");
|
||||
match scope {
|
||||
ParsedScope::Rpc(r) => {
|
||||
assert_eq!(r.lxm, Some("app.bsky.feed.getTimeline".to_string()));
|
||||
assert_eq!(r.lxm, "app.bsky.feed.getTimeline");
|
||||
assert_eq!(r.aud, Some("did:web:api.bsky.app".to_string()));
|
||||
}
|
||||
_ => panic!("Expected Rpc scope"),
|
||||
@@ -510,6 +511,12 @@ mod tests {
|
||||
assert!(matches!(scope4, ParsedScope::Rpc(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rpc_lxm_required() {
|
||||
let bare = parse_scope("rpc");
|
||||
assert!(matches!(bare, ParsedScope::Unknown(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_url_encoded_aud_with_fragment() {
|
||||
let scope =
|
||||
@@ -527,10 +534,7 @@ mod tests {
|
||||
);
|
||||
match scope2 {
|
||||
ParsedScope::Rpc(r) => {
|
||||
assert_eq!(
|
||||
r.lxm,
|
||||
Some("com.atproto.moderation.createReport".to_string())
|
||||
);
|
||||
assert_eq!(r.lxm, "com.atproto.moderation.createReport");
|
||||
assert_eq!(r.aud, Some("did:web:api.bsky.app#bsky_appview".to_string()));
|
||||
}
|
||||
_ => panic!("Expected Rpc scope"),
|
||||
|
||||
@@ -49,10 +49,7 @@ impl ScopePermissions {
|
||||
pub fn superseded_by_transition_generic(scope: &ParsedScope) -> bool {
|
||||
match scope {
|
||||
ParsedScope::Repo(_) | ParsedScope::Blob(_) => true,
|
||||
ParsedScope::Rpc(rpc) => !rpc
|
||||
.lxm
|
||||
.as_deref()
|
||||
.is_some_and(|lxm| lxm == "*" || lxm.starts_with("chat.bsky.")),
|
||||
ParsedScope::Rpc(rpc) => !(rpc.lxm == "*" || rpc.lxm.starts_with("chat.bsky.")),
|
||||
ParsedScope::Account(_)
|
||||
| ParsedScope::Identity(_)
|
||||
| ParsedScope::TransitionEmail
|
||||
@@ -193,9 +190,9 @@ impl ScopePermissions {
|
||||
|
||||
let has_permission = self.find_rpc_scopes().any(|rpc_scope| {
|
||||
let lxm_matches = match &rpc_scope.lxm {
|
||||
None => true,
|
||||
Some(scope_lxm) if scope_lxm == lxm => true,
|
||||
Some(scope_lxm) if scope_lxm.ends_with(".*") => {
|
||||
scope_lxm if scope_lxm == "*" => true,
|
||||
scope_lxm if scope_lxm == lxm => true,
|
||||
scope_lxm if scope_lxm.ends_with(".*") => {
|
||||
let prefix = scope_lxm.strip_suffix(".*").unwrap();
|
||||
lxm.starts_with(prefix) && lxm.chars().nth(prefix.len()) == Some('.')
|
||||
}
|
||||
@@ -203,7 +200,7 @@ impl ScopePermissions {
|
||||
};
|
||||
|
||||
let aud_matches = match &rpc_scope.aud {
|
||||
None => true,
|
||||
None => false,
|
||||
Some(scope_aud) if scope_aud == "*" => true,
|
||||
Some(scope_aud) => scope_aud == aud,
|
||||
};
|
||||
@@ -474,6 +471,24 @@ mod tests {
|
||||
assert!(!perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getAuthorFeed")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rpc_wildcard_lxm() {
|
||||
let perms = ScopePermissions::from_scope_string(Some(
|
||||
"rpc:*?aud=did:web:api.bsky.app#bsky_appview",
|
||||
));
|
||||
let aud = "did:web:api.bsky.app#bsky_appview";
|
||||
let other = "did:web:other.app#bsky_appview";
|
||||
assert!(perms.allows_rpc(aud, &c("app.bsky.feed.getTimeline")));
|
||||
assert!(!perms.allows_rpc(other, &c("app.bsky.feed.getTimeline")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rpc_wildcard_lxm_without_aud() {
|
||||
let perms = ScopePermissions::from_scope_string(Some("rpc:*"));
|
||||
let aud = "did:web:api.bsky.app#bsky_appview";
|
||||
assert!(!perms.allows_rpc(aud, &c("app.bsky.feed.getTimeline")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_granular_account() {
|
||||
let perms = ScopePermissions::from_scope_string(Some("account:email?action=read"));
|
||||
|
||||
Reference in New Issue
Block a user