auth: support multiple AUTHORIZATION_TOKEN in single message

[michalhosna]

Preparation for #530

Related:

• Semantics of multiple AUTHORIZATION TOKEN parameters in one message moq-wg/moq-transport#1838

---

This change is 

[mondain]

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]

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

Both of those findings work as I intended, at least what I can parse from your text, do you disagree with my intention or are you raising a question if this was the intention?

[afrind]

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.

eh, I think we have operational flexibility here, especially if we think it's going to trend towards this interpretation.

A request token no longer overrides the session grants.

As long as the docs are updated to match, I think this is also within our power to define. I think "SETUP tokens are the base set, requests extend privileges" makes sense. Obviously the caller has the SETUP token so they could have included it in the request if they wanted to.

@afrind reviewed 6 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on michalhosna).

---

src/auth/Auth.h line 131 at r1 (raw file):

// Verifies the setup AUTHORIZATION_TOKEN(s). Returns a null pointer when auth
// is disabled; otherwise a shared vector of every successfully-verified

why shared and not unique_ptr?

---

src/auth/Auth.cpp line 182 at r1 (raw file):

}

folly::Expected<std::shared_ptr<const std::vector<Grants>>, AuthError>

I guess I missed before that this is shared_ptr - can it be unique?

[michalhosna]

@michalhosna made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on afrind).

---

src/auth/Auth.h line 131 at r1 (raw file):

Previously, afrind wrote…

why shared and not unique_ptr?

It's passed down to AuthPublisherFilter and AuthSubscriberFilter.

AFAIK the Filter's lifetime is decoupled from Sessions's lifetime, so making it owned by the session is not safe as the filter may outlive it in coroutine.

And I don't feel like single shared_ptr copy at session initialization is worth optimizing.

[afrind]

@afrind made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on michalhosna).

---

src/auth/Auth.h line 131 at r1 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

It's passed down to AuthPublisherFilter and AuthSubscriberFilter.

AFAIK the Filter's lifetime is decoupled from Sessions's lifetime, so making it owned by the session is not safe as the filter may outlive it in coroutine.

And I don't feel like single shared_ptr copy at session initialization is worth optimizing.

I think it's less about optimization and more about making it easier to reason about things that have only a single owner. But I won't lie down in the road here.

[afrind]

@afrind resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on michalhosna).