goblin: gate return-to-caller on an optional rt flag (QR scans stay in-wallet)
The Build 151 return-to-caller is right for same-device deep-links but wrong for a QR scanned from another screen: there is no caller, so the wallet bounced the user into whatever app was previous. The login/authorize/trust URIs now carry an optional rt flag: rt=0 disables the return (QR case), rt=1 or an absent flag keeps the return (backward compatible). Parsing is byte-exact and fails closed to the return-enabled default on empty, garbage, or duplicate values (first occurrence wins, like every other param). Security is unchanged: the flag is a single boolean gate on the existing best-effort app-switch and derives no target, so it can never introduce a redirect the URI could not already perform; cb-host==domain binding and every other check are untouched. - shared parse_return_flag helper in authuri, reused by login and trust - return_to_caller field on LoginUri, AuthorizeUri, TrustUri (default true) - all three GUI flows (login, authorize, trust) honour the flag uniformly - parser unit tests: absent/explicit/garbage/duplicate + the no-return decision
This commit is contained in:
@@ -6629,6 +6629,7 @@ impl GoblinWalletView {
|
||||
let st = self.login.as_mut().unwrap();
|
||||
st.pass.clear();
|
||||
st.wrong_pass = false;
|
||||
let return_to_caller = st.uri.return_to_caller;
|
||||
match crate::nostr::loginuri::build_login_event(
|
||||
&keys,
|
||||
&st.uri.challenge,
|
||||
@@ -6664,6 +6665,15 @@ impl GoblinWalletView {
|
||||
*slot.lock().unwrap() = Some(res);
|
||||
});
|
||||
Modal::close();
|
||||
// Signed and handed to the POST worker: return the user to
|
||||
// the app that deep-linked in (the polling browser tab),
|
||||
// without waiting on the HTTP response. Success path only, and
|
||||
// only for a same-device deep-link (the default); a QR-scanned
|
||||
// URI carries `rt=0`, so we stay in the wallet and the browser
|
||||
// picks the login up by polling.
|
||||
if return_to_caller {
|
||||
cb.return_to_caller();
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// Signing failed (never expected): consume the request and
|
||||
@@ -7050,6 +7060,7 @@ impl GoblinWalletView {
|
||||
let st = self.authorize.as_mut().unwrap();
|
||||
st.pass.clear();
|
||||
st.wrong_pass = false;
|
||||
let return_to_caller = st.uri.return_to_caller;
|
||||
match crate::nostr::authuri::build_authorize_event(&keys, &st.uri.template) {
|
||||
Ok(event) => {
|
||||
// Signed: the request is consumed from here on, whatever the
|
||||
@@ -7084,6 +7095,15 @@ impl GoblinWalletView {
|
||||
*slot.lock().unwrap() = Some(res);
|
||||
});
|
||||
Modal::close();
|
||||
// Signed and handed to the POST worker: return the user to
|
||||
// the app that deep-linked in (the polling browser tab),
|
||||
// without waiting on the HTTP response. Success path only, and
|
||||
// only for a same-device deep-link (the default); a QR-scanned
|
||||
// URI carries `rt=0`, so we stay in the wallet and the browser
|
||||
// picks the result up by polling.
|
||||
if return_to_caller {
|
||||
cb.return_to_caller();
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// Signing failed (never expected): consume the request and
|
||||
@@ -7320,6 +7340,7 @@ impl GoblinWalletView {
|
||||
let st = self.trust.as_mut().unwrap();
|
||||
st.pass.clear();
|
||||
st.wrong_pass = false;
|
||||
let return_to_caller = st.uri.return_to_caller;
|
||||
// Sign and POST the kind-22242 login event exactly as Build 150 does;
|
||||
// the session is created by the router once this POST succeeds.
|
||||
match crate::nostr::loginuri::build_login_event(
|
||||
@@ -7354,7 +7375,12 @@ impl GoblinWalletView {
|
||||
*slot.lock().unwrap() = Some(res);
|
||||
});
|
||||
Modal::close();
|
||||
cb.return_to_caller();
|
||||
// Same-device deep-link only (the default); a QR-scanned trust
|
||||
// URI carries `rt=0`, so we stay in the wallet and the browser
|
||||
// picks the session up by polling.
|
||||
if return_to_caller {
|
||||
cb.return_to_caller();
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
log::error!("trust login event signing failed: {e}");
|
||||
|
||||
@@ -99,6 +99,11 @@ pub struct AuthorizeUri {
|
||||
pub callback: String,
|
||||
/// The event template the site wants signed.
|
||||
pub template: Template,
|
||||
/// Whether the wallet should hand focus back to a same-device caller after
|
||||
/// the flow completes. `true` (the default when the `rt` flag is absent) is
|
||||
/// the Build 151 deep-link behavior; a QR-scanned URI mints `rt=0` so the
|
||||
/// wallet stays put and the browser picks the result up by polling.
|
||||
pub return_to_caller: bool,
|
||||
}
|
||||
|
||||
/// True when `scanned` carries a Goblin scheme with the `authorize` keyword,
|
||||
@@ -139,6 +144,7 @@ pub fn parse(scanned: &str) -> Option<AuthorizeUri> {
|
||||
let mut domain = None;
|
||||
let mut callback = None;
|
||||
let mut template = None;
|
||||
let mut return_flag = None;
|
||||
for pair in query.split('&') {
|
||||
let Some((key, val)) = pair.split_once('=') else {
|
||||
continue; // valueless / malformed segment, ignore
|
||||
@@ -150,6 +156,8 @@ pub fn parse(scanned: &str) -> Option<AuthorizeUri> {
|
||||
"d" if domain.is_none() => domain = Some(val),
|
||||
"cb" if callback.is_none() => callback = Some(val),
|
||||
"e" if template.is_none() => template = Some(val),
|
||||
// Optional return-to-caller gate; first occurrence wins like the rest.
|
||||
"rt" if return_flag.is_none() => return_flag = Some(val),
|
||||
// Unknown params are ignored for forward-compat.
|
||||
_ => {}
|
||||
}
|
||||
@@ -164,11 +172,13 @@ pub fn parse(scanned: &str) -> Option<AuthorizeUri> {
|
||||
return None;
|
||||
}
|
||||
let template = validate_template(template?)?;
|
||||
let return_to_caller = parse_return_flag(return_flag);
|
||||
Some(AuthorizeUri {
|
||||
challenge,
|
||||
domain,
|
||||
callback,
|
||||
template,
|
||||
return_to_caller,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -292,6 +302,27 @@ pub(crate) fn domain_bound(callback: &str, domain: &str) -> bool {
|
||||
host == domain || host.ends_with(&format!(".{domain}"))
|
||||
}
|
||||
|
||||
/// Parse the optional `rt` return-to-caller flag shared by the login, authorize,
|
||||
/// and trust URIs. This flag ONLY controls whether the wallet hands focus back to
|
||||
/// a same-device caller after the flow completes; it can NEVER introduce a
|
||||
/// redirect the URI could not already perform (no target is derived from it, it
|
||||
/// is a single boolean gate on the existing best-effort app-switch). Absent,
|
||||
/// empty, malformed, or any value other than an explicit `0` yields the default
|
||||
/// `true` (return enabled), so pre-flag URIs and any garbage keep the Build 151
|
||||
/// same-device behavior — the fail-closed default is the backward-compatible one.
|
||||
/// Only `rt=0` disables the return (the QR-scan case, where there is no caller and
|
||||
/// the browser picks the result up by polling); `rt=1` is the explicit default.
|
||||
/// The match is byte-exact on the (percent-decoded) value — no trimming — so only
|
||||
/// a clean `0` disables and anything padded or decorated stays the safe default.
|
||||
/// Duplicate `rt=` is resolved by each parser's first-occurrence-wins loop before
|
||||
/// this is called.
|
||||
pub(crate) fn parse_return_flag(raw: Option<&str>) -> bool {
|
||||
match raw {
|
||||
Some(v) => String::from_utf8_lossy(&percent_decode(v)) != "0",
|
||||
None => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Decode and validate the `e` template. Unpadded base64url only (any `=`
|
||||
/// padding or non-`A-Za-z0-9-_` char rejects), decoding to a UTF-8 JSON object
|
||||
/// with EXACTLY the three keys `kind`, `content`, `tags` and nothing else. The
|
||||
@@ -948,6 +979,87 @@ mod tests {
|
||||
assert_eq!(parse(&broken), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_absent_defaults_to_return() {
|
||||
// No `rt` param: backward-compatible default is return enabled.
|
||||
let out = parse(&valid_uri("goblin:")).expect("valid authorize URI");
|
||||
assert!(out.return_to_caller, "absent rt must default to return");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_explicit_values() {
|
||||
// rt=0 disables the return (QR-scan case); rt=1 is the explicit default.
|
||||
let no = format!(
|
||||
"goblin:authorize?e={}&d=magick.market&cb=https://magick.market/cb&c={C}&rt=0",
|
||||
valid_template()
|
||||
);
|
||||
assert!(
|
||||
!parse(&no).unwrap().return_to_caller,
|
||||
"rt=0 must disable return"
|
||||
);
|
||||
let yes = format!(
|
||||
"goblin:authorize?e={}&d=magick.market&cb=https://magick.market/cb&c={C}&rt=1",
|
||||
valid_template()
|
||||
);
|
||||
assert!(
|
||||
parse(&yes).unwrap().return_to_caller,
|
||||
"rt=1 must enable return"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_garbage_fails_closed_to_default() {
|
||||
// Anything that is not an explicit `0` keeps the default (return enabled),
|
||||
// so a malformed flag can never silently suppress the same-device return.
|
||||
for junk in [
|
||||
"", "2", "true", "false", "yes", "no", "00", " 0", "0x0", "%30",
|
||||
] {
|
||||
let uri = format!(
|
||||
"goblin:authorize?e={}&d=magick.market&cb=https://magick.market/cb&c={C}&rt={junk}",
|
||||
valid_template()
|
||||
);
|
||||
// %30 decodes to "0" -> disabled; every other junk value stays default.
|
||||
let expected = junk != "%30";
|
||||
assert_eq!(
|
||||
parse(&uri).unwrap().return_to_caller,
|
||||
expected,
|
||||
"rt={junk:?} must resolve to return={expected}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_duplicate_first_wins() {
|
||||
// First occurrence wins, matching every other param.
|
||||
let first_off = format!(
|
||||
"goblin:authorize?e={}&d=magick.market&cb=https://magick.market/cb&c={C}&rt=0&rt=1",
|
||||
valid_template()
|
||||
);
|
||||
assert!(!parse(&first_off).unwrap().return_to_caller);
|
||||
let first_on = format!(
|
||||
"goblin:authorize?e={}&d=magick.market&cb=https://magick.market/cb&c={C}&rt=1&rt=0",
|
||||
valid_template()
|
||||
);
|
||||
assert!(parse(&first_on).unwrap().return_to_caller);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_return_flag_unit() {
|
||||
assert!(parse_return_flag(None), "absent -> default return");
|
||||
assert!(parse_return_flag(Some("1")), "1 -> return");
|
||||
assert!(!parse_return_flag(Some("0")), "0 -> no return");
|
||||
assert!(parse_return_flag(Some("")), "empty -> default return");
|
||||
assert!(
|
||||
parse_return_flag(Some("garbage")),
|
||||
"garbage -> default return"
|
||||
);
|
||||
// Percent-encoded 0 decodes to a real disable.
|
||||
assert!(
|
||||
!parse_return_flag(Some("%30")),
|
||||
"%30 decodes to 0 -> no return"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorize_event_signed_by_the_chosen_identity() {
|
||||
// Two held identities; the user picks the NON-active one. The event must
|
||||
|
||||
@@ -62,6 +62,11 @@ pub struct LoginUri {
|
||||
/// The callback URL the signed event is delivered to: `https://...`, or
|
||||
/// `http://localhost[:port]...` for development.
|
||||
pub callback: String,
|
||||
/// Whether the wallet should hand focus back to a same-device caller after
|
||||
/// the flow completes. `true` (the default when the `rt` flag is absent) is
|
||||
/// the Build 151 deep-link behavior; a QR-scanned URI mints `rt=0` so the
|
||||
/// wallet stays put and the browser picks the login up by polling.
|
||||
pub return_to_caller: bool,
|
||||
}
|
||||
|
||||
/// True when `scanned` carries a Goblin scheme with the `login` keyword, i.e.
|
||||
@@ -99,6 +104,7 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
|
||||
let mut challenge = None;
|
||||
let mut domain = None;
|
||||
let mut callback = None;
|
||||
let mut return_flag = None;
|
||||
for pair in query.split('&') {
|
||||
let Some((key, val)) = pair.split_once('=') else {
|
||||
continue; // valueless / malformed segment, ignore
|
||||
@@ -109,6 +115,8 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
|
||||
"c" if challenge.is_none() => challenge = Some(val),
|
||||
"d" if domain.is_none() => domain = Some(val),
|
||||
"cb" if callback.is_none() => callback = Some(val),
|
||||
// Optional return-to-caller gate; first occurrence wins like the rest.
|
||||
"rt" if return_flag.is_none() => return_flag = Some(val),
|
||||
// Unknown params are ignored for forward-compat.
|
||||
_ => {}
|
||||
}
|
||||
@@ -116,6 +124,7 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
|
||||
let challenge = validate_challenge(challenge?)?;
|
||||
let domain = validate_domain(domain?)?;
|
||||
let callback = validate_callback(callback?)?;
|
||||
let return_to_caller = super::authuri::parse_return_flag(return_flag);
|
||||
// Domain binding: the callback host must belong to the displayed domain, so a
|
||||
// site cannot show `d=magick.market` while pointing `cb` at its own host and
|
||||
// harvest a login event for a domain it does not control. Same rule as
|
||||
@@ -128,6 +137,7 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
|
||||
challenge,
|
||||
domain,
|
||||
callback,
|
||||
return_to_caller,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -447,6 +457,47 @@ mod tests {
|
||||
assert!(!is_login_shaped(&huge));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_absent_defaults_to_return() {
|
||||
// No `rt`: backward-compatible default is return enabled.
|
||||
let out = parse(&valid_uri("goblin:")).expect("valid login URI");
|
||||
assert!(out.return_to_caller, "absent rt must default to return");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_explicit_and_garbage() {
|
||||
// rt=0 disables (QR-scan case), rt=1 is the explicit default.
|
||||
let no = format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb&rt=0");
|
||||
assert!(
|
||||
!parse(&no).unwrap().return_to_caller,
|
||||
"rt=0 must disable return"
|
||||
);
|
||||
let yes = format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb&rt=1");
|
||||
assert!(
|
||||
parse(&yes).unwrap().return_to_caller,
|
||||
"rt=1 must enable return"
|
||||
);
|
||||
// Garbage fails closed to the default (return enabled).
|
||||
for junk in ["", "2", "true", "no", "00"] {
|
||||
let uri =
|
||||
format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb&rt={junk}");
|
||||
assert!(
|
||||
parse(&uri).unwrap().return_to_caller,
|
||||
"rt={junk:?} must fail closed to return"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_duplicate_first_wins() {
|
||||
let first_off =
|
||||
format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb&rt=0&rt=1");
|
||||
assert!(!parse(&first_off).unwrap().return_to_caller);
|
||||
let first_on =
|
||||
format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb&rt=1&rt=0");
|
||||
assert!(parse(&first_on).unwrap().return_to_caller);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn login_event_signed_by_the_chosen_identity() {
|
||||
// Two held identities; the user picks the NON-active one. The event
|
||||
|
||||
@@ -78,6 +78,11 @@ pub struct TrustUri {
|
||||
/// renders the remainder as categories and shows caution lines for anything
|
||||
/// stripped. Guaranteed non-empty, and guaranteed non-empty after stripping.
|
||||
pub requested_kinds: Vec<u16>,
|
||||
/// Whether the wallet should hand focus back to a same-device caller after
|
||||
/// the flow completes. `true` (the default when the `rt` flag is absent) is
|
||||
/// the Build 151 deep-link behavior; a QR-scanned URI mints `rt=0` so the
|
||||
/// wallet stays put and the browser picks the result up by polling.
|
||||
pub return_to_caller: bool,
|
||||
}
|
||||
|
||||
/// True when `scanned` carries a Goblin scheme with the `trust` keyword, i.e. it
|
||||
@@ -121,6 +126,7 @@ pub fn parse(scanned: &str) -> Option<TrustUri> {
|
||||
let mut site_key = None;
|
||||
let mut relay = None;
|
||||
let mut kinds = None;
|
||||
let mut return_flag = None;
|
||||
for pair in query.split('&') {
|
||||
let Some((key, val)) = pair.split_once('=') else {
|
||||
continue; // valueless / malformed segment, ignore
|
||||
@@ -134,6 +140,8 @@ pub fn parse(scanned: &str) -> Option<TrustUri> {
|
||||
"sk" if site_key.is_none() => site_key = Some(val),
|
||||
"r" if relay.is_none() => relay = Some(val),
|
||||
"k" if kinds.is_none() => kinds = Some(val),
|
||||
// Optional return-to-caller gate; first occurrence wins like the rest.
|
||||
"rt" if return_flag.is_none() => return_flag = Some(val),
|
||||
// Unknown params are ignored for forward-compat.
|
||||
_ => {}
|
||||
}
|
||||
@@ -150,6 +158,7 @@ pub fn parse(scanned: &str) -> Option<TrustUri> {
|
||||
let site_session_pubkey = validate_x_only_hex(site_key?)?;
|
||||
let relay = validate_relay(relay?)?;
|
||||
let requested_kinds = validate_kinds(kinds?)?;
|
||||
let return_to_caller = super::authuri::parse_return_flag(return_flag);
|
||||
Some(TrustUri {
|
||||
challenge,
|
||||
domain,
|
||||
@@ -157,6 +166,7 @@ pub fn parse(scanned: &str) -> Option<TrustUri> {
|
||||
site_session_pubkey,
|
||||
relay,
|
||||
requested_kinds,
|
||||
return_to_caller,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -507,6 +517,51 @@ mod tests {
|
||||
assert!(!is_trust_shaped(&huge));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_absent_defaults_to_return() {
|
||||
// No `rt`: backward-compatible default is return enabled.
|
||||
let out = parse(&valid_uri("goblin:")).expect("valid trust URI");
|
||||
assert!(out.return_to_caller, "absent rt must default to return");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn return_flag_explicit_garbage_and_duplicate() {
|
||||
let base = format!(
|
||||
"c={C}&d=magick.market&cb=https://magick.market/cb&sk={SK}&r=wss://r.magick.market&k=1"
|
||||
);
|
||||
// rt=0 disables (QR-scan case), rt=1 is the explicit default.
|
||||
assert!(
|
||||
!parse(&format!("goblin:trust?{base}&rt=0"))
|
||||
.unwrap()
|
||||
.return_to_caller
|
||||
);
|
||||
assert!(
|
||||
parse(&format!("goblin:trust?{base}&rt=1"))
|
||||
.unwrap()
|
||||
.return_to_caller
|
||||
);
|
||||
// Garbage fails closed to the default (return enabled).
|
||||
for junk in ["", "2", "true", "no"] {
|
||||
assert!(
|
||||
parse(&format!("goblin:trust?{base}&rt={junk}"))
|
||||
.unwrap()
|
||||
.return_to_caller,
|
||||
"rt={junk:?} must fail closed to return"
|
||||
);
|
||||
}
|
||||
// First occurrence wins.
|
||||
assert!(
|
||||
!parse(&format!("goblin:trust?{base}&rt=0&rt=1"))
|
||||
.unwrap()
|
||||
.return_to_caller
|
||||
);
|
||||
assert!(
|
||||
parse(&format!("goblin:trust?{base}&rt=1&rt=0"))
|
||||
.unwrap()
|
||||
.return_to_caller
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trust_is_not_pay_login_or_authorize() {
|
||||
let trust = valid_uri("goblin:");
|
||||
|
||||
Reference in New Issue
Block a user