JanssenProject / JanssenProject/jans

feat(jans-cedarling): report dropped tokens in the multi-issuer authorization result

Open
#15,121 0 comments 0 reactions 3 assignees Claimed by @haileyesus2433 View on GitHub
comp-jans-cedarling kind-feature
Dominant language
Java
Stars
647
Forks
173
Avg merge
1d 18h
Merged PRs (30d)
110

Description

# Report dropped tokens in the multi-issuer authorization result

## Summary

In multi-issuer authorization a token that fails is **dropped**, and the request
proceeds on whatever survived. The caller cannot tell: `MultiIssuerAuthorizeResult`
carries only `response`, `decision` and `request_id`
(`authz/authorize_result.rs:67-81`). An `ALLOW` produced from three tokens and an
`ALLOW` produced from two of them (because the third was thrown away) are
indistinguishable.

The only trace today is the log: a WARN/ERROR entry plus an error metric, correlated
by `request_id`. That means an application that wants to react has to keep the memory
logger on and parse log text.

This issue makes the drops part of the result, so the caller can decide for itself.

## Why now

The entity-type unification issue removes the custom issuer's `required` flag, which
today is the only way to make a failed token fail the request
(`CustomIssuerIndex::mapping_required`, `custom_token.rs:302`). After that, every
token is effectively optional and fail-closed for a mandatory token has to be
expressed either in policy (`forbid … unless { context has tokens.… }`) or by the
caller. The caller cannot do it without knowing what was dropped.

This is additive and independent — it does not need the format-versioning work.

## Where tokens are dropped

Two separate stages, and the second one is easy to miss:

**1. Validation / processing** (`JwtService::validate_multi_issuer_tokens`,
`jwt/mod.rs:395-470`):

- malformed input (empty mapping or payload) — `token.validate()` fails
(`jwt/mod.rs:413-421`);
- JWT validation failure (signature, expiry, missing validator, trusted-issuer
checks) (`jwt/mod.rs:648-657`);
- duplicate `(iss, mapping)` — the non-deterministic-token check
(`jwt/mod.rs:620-646`);
- custom token: no processor registered (`jwt/mod.rs:483-499`);
- custom token: processor error or timeout (non-`required` path).

**2. Entity building** (`build_multi_issuer_setup_entities`,
`build_multi_issuer_entity.rs:299-337`): a token that validated fine is still dropped
if its entity, or its entity key, cannot be built. Logged at ERROR, no effect on the
returned result.

If **nothing** survives either stage the request already errors
(`TokenValidationFailed`, `jwt/mod.rs:458-466`; `MultiIssuerEntityError::NoValidTokens`,
`build_multi_issuer_entity.rs:338-348`), so this issue is only about the partial case.

## Proposal

### 1. Carry the drops out of the pipeline

Follow the existing `DefaultEntitiesWithWarns` pattern (`default_entities.rs:89-111`):
both stages return their drops alongside their output instead of only logging them,
`multi_issuer_setup` (`authz/mod.rs:981-1015`) merges the two lists, and the
authorization path puts them into the result. Logging stays as it is.

### 2. Shape

- A `DroppedToken` value: the input `mapping`, the input `index` (mapping can repeat,
index is what identifies the entry the caller sent) and the reason.
- The reason is a `thiserror` enum whose variants carry their own data, so one type
gives both the machine-readable discriminant the caller branches on and the exact
message via `Display` — no parallel free-form detail field to keep in sync.
- The variant set is closed and small; the drop sites above are the list: invalid
input, JWT validation failed, duplicate token, no custom processor registered,
custom processing failed, custom processing timed out, entity build failed. Mark it
`#[non_exhaustive]` so a new drop site is not a breaking change.
- Duplicates are reported like any other drop. It is a caller mistake rather than a
failure, but silently ignoring one of two tokens is exactly the kind of thing the
caller should see.
- `Display` must not carry token payload or claim values — it describes the failure,
not the token.
- Binding surfaces cannot express a data-carrying Rust enum directly, so each one maps
it to a stable variant tag plus the rendered message. The tag is API: it is what
callers match on, so it is named deliberately and does not change.

### 3. Batch

Tokens come in once per batch, not per item (`BatchAuthorizeMultiIssuerRequest`,
`authz/request.rs:214-219`), and `multi_issuer_setup` runs once
(`authz/mod.rs:433-438`).

Nothing special is needed: the field goes on `MultiIssuerAuthorizeResult`, which is
already each item's result type, so in a batch the same list simply repeats per item.
That costs nothing and keeps one result type with one meaning — no separate
batch-level field on the generic `BatchAuthorizeResponse`.

The single-issuer `AuthorizeResult` is untouched.

### 4. Decision log

The decision-log entry should carry the same list, so an audit answers "which tokens
did this decision actually see" without joining against separate WARN entries.

### 5. Compatibility

- JSON gains a field — additive. Serialize as absent/empty when nothing was dropped.
- Rust: adding a public field technically breaks struct-literal construction. Callers
receive this struct rather than build it, so in practice it is fine; consider
`#[non_exhaustive]` so the next field is not a breaking change again.
- Bindings all mirror the result type and need the new field: uniffi
(`bindings/cedarling_uniffi/src/result.rs:73`), WASM
(`bindings/cedarling_wasm/src/lib.rs:36`), Python
(`MultiIssuerAuthorizeResult`, `BatchItemMultiIssuerResult`), the Java adapter, and
the OPA plugin types (`cedarling_opa/plugins/cedarling_opa/types.go`). A binding
that does not expose the field yet is not broken by it, so they can land
incrementally.

## Out of scope

- Changing any decision: dropping stays dropping, and nothing here turns a drop back
into a request-level error. The caller decides.
- Reintroducing a per-token `required` switch in the policy store.
- The single-issuer (`authorize`) path.

## Open questions

- [ ] How much of the underlying error to keep as structured variant data vs leave to
`Display` (e.g. is "expired" worth its own field?). Decide per variant while
implementing; the binding tag stays coarse either way.

## Tests

- [ ] Partial failure: N tokens in, one bad → decision still returned, drop list has
exactly that entry with the right index and reason.
- [ ] One case per reason variant, duplicates included.
- [ ] A token that validates but fails entity building appears in the list.
- [ ] All tokens bad → still an error, unchanged.
- [ ] Nothing dropped → field is empty/absent, JSON unchanged for existing consumers.
- [ ] Batch: every item carries the same drop list.
- [ ] Rendered reason messages contain no claim values.

## Docs

- [ ] `cedarling-multi-issuer.md`: the drop list, every reason variant with its tag,
and the recommended pattern — check the list when a missing token must not be
tolerated.
- [ ] Cross-link from the token-failure section added by the entity-type issue, and
from the `required` removal note in the upgrade guide.
- [ ] Note the decision-log field.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.