developmentseed / developmentseed/multistore
sts: fail closed on empty trust fields, check token type, require exp, log successes
- Dominant language
- Rust
- Stars
- 19
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Description
Four small hardening items in `multistore-sts`. None is reachable from the Source Cooperative deployment today, because it serves a single hardcoded Role — and all four become reachable the moment Roles are user-authored.
**Today**
- An empty `required_audiences` accepts **any** audience (`jwks.rs`, `audience_allowed` returns true on an empty list).
- An empty `subject_conditions` skips the subject check entirely.
- Both carry `#[serde(default)]`, so a Role that omits either field silently trusts everything from its issuer. (`trusted_oidc_issuers` is also defaulted but correctly fails closed.)
- There is no token-type (`typ`) check.
- A token with no `exp` is accepted — the expiry check is inside `if let Some(exp)`. This matters for tokens from *third-party* issuers, where a missing expiry means an indefinitely replayable credential the issuer never intended.
- Successful exchanges are never logged; only errors are.
**Do**
- Make an empty audience list and an empty subject-condition list mean **deny**.
- Add a token-type check.
- Reject tokens with no `exp` **from issuers other than the proxy itself**. See the caveat below — a blanket rule would break a legitimate case.
- Log successful exchanges.
**Also worth fixing here**
`resolve_scopes` substitutes `{claim}` templates into scope prefixes, and a missing claim resolves to an empty string. For `bucket` that fails safe, but for `prefixes` an empty string matches every key — so a missing claim grants **full-bucket access**. The doc comment claims it fails safe. Treat an unresolved placeholder as a hard error at mint time.
> [!IMPORTANT]
> **Do not require `exp` unconditionally.** A host may legitimately mint its own long-lived tokens whose validity is tracked server-side rather than in the claim — Source Cooperative's API keys work this way, with a nullable `expires_at` on a revocation record the exchange already reads. A blanket rule rejects exactly those.
>
> Scope the requirement to tokens the host did not mint: require `exp` when `iss` is not the host's own issuer, or exempt a token carrying a host-recognised type claim. Whichever shape it takes, the exemption should be explicit rather than a side effect of configuration.
**Done when**
Released, and the dependency bumped in `source-cooperative/data.source.coop`.
---
Part of source-cooperative/source.coop#491.
Contributor guide
Research direction
Begin in jwks.rs, especially audience_allowed, and trace the token exchange and resolve_scopes entry points. Review the existing validation and logging tests, if present, before covering empty trust fields, token type, issuer-scoped expiry, unresolved scope claims, and successful exchanges. Done means the hardening behavior is tested, released, and the dependency is bumped in source-cooperative/data.source.coop.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- authentication, backend-api-design, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100