Skip to content

auth: support multiple AUTHORIZATION_TOKEN in single message - #552

Open
michalhosna wants to merge 1 commit into
mainfrom
mh/multi-auth-token
Open

auth: support multiple AUTHORIZATION_TOKEN in single message#552
michalhosna wants to merge 1 commit into
mainfrom
mh/multi-auth-token

Conversation

@michalhosna

@michalhosna michalhosna commented Aug 7, 2026

Copy link
Copy Markdown
Member

A token that fails to verify is dropped as a non-viable candidate.
The most specific verification error surfaces when nothing in the pool works.

Related:
- moq-wg/moq-transport#1838
    - Semantics of multiple AUTHORIZATION TOKEN parameters in one message
- moq-wg/moq-transport#1724
    - Which token caused the error
@michalhosna
michalhosna force-pushed the mh/multi-auth-token branch from 0920918 to 138350f Compare August 7, 2026 21:42
@mondain

mondain commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Reviewable

I found two blocking authorization-contract issues.

[IMPORTANT] Invalid tokens must not be dropped when another credential permits the operation. authenticateSetup() and authorize() currently continue after every verification error and accept the message if another token or the session grants authorize it. Section 9.2.2.1 of the checked-in draft-16 says a message containing a well-formed Token structure but otherwise invalid AUTHORIZATION TOKEN parameter MUST be rejected with MALFORMED_AUTH_TOKEN. The existing AuthorizeGarbageRequestTokenDoesNotBlockSessionGrants and AuthenticateSetupAcceptsAnyOfMultipleSetupTokens tests codify the opposite behavior. I added a temporary diagnostic with a bad-signature request token followed by a valid token; the expected rejection failed because authorize() returned success. The linked WG issue #1838 is still open and explicitly identifies this conflict, so the current draft behavior should remain authoritative until that discussion changes it. Please fail on a recognized token's verification error and add negative request/setup coverage; union grants only across tokens that are all valid.

[IMPORTANT] A request token no longer overrides the session grants. allow_request_token_override is documented in docs/config.md, config.example.yaml, and the generated schema as replacing the setup grants for that request. The new requestGrants || sessionGrants check makes the sources additive instead. That means a request token intended to narrow authority cannot do so: a broad session grant still authorizes the operation. A temporary diagnostic with a valid Publish-only request token and a Subscribe session grant expected Forbidden, but returned success. When override is enabled and at least one request token is present, please evaluate the request-token pool instead of the session pool; use session grants only when the request carries no matching token. If additive behavior is intentional, the setting and all three operator-facing descriptions need an explicit contract change.

For verification, exact head 138350f6 built successfully, all 36 focused auth/filter/context tests passed before the diagnostics, and git diff --check is clean.

@michalhosna

Copy link
Copy Markdown
Member Author

@mondain I am lost at what reaction/action do you expect from me?

Both of those findings work as I intended, do you disagree with my intention or are you raising a question if this was the intention?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants