OAuth Authz validation parity test enhancements

[sdewhitt]

Lands after #159

What this adds

A parity-pin test for the OAuth authz validation endpoint that captures the current upstream behavior around dotted additional_scopes_key values (rabbitmq-server 16947), plus a small comment correction on the existing over-trust caveat.

rabbitmq-server 16947 documents that the OAuth backend resolves additional_scopes_key via rabbit_auth_backend_oauth2:split_path/1, which splits the key on every .:

split_path(Path) -> binary:split(Path, <<".">>, [global, trim_all]).

A flat claim key that itself contains dots — e.g. an STS/OIDC key like https://sts.amazonaws.com/tags — is therefore split into a nested-map path (["https://sts", "amazonaws", "com/tags"]), the flat claim is never found, and its scopes are not extracted. The validation endpoint replays the broker's real scope-resolution code, so it faithfully reproduces this behavior.

Changes

• test/aws_auth_validate_oauth_tests.erl — adds authz_dotted_additional_scopes_key_parity_pin_test_:

  • Mints a token carrying scopes under a flat dotted claim key (https://sts.amazonaws.com/tags), points additional_scopes_key at that key, and runs it through the real upstream normalize_token_scope / get_expanded_scopes / resource_access path (not mocks).
  • Asserts the current behavior: {error, authz_unverified, _} with the "no scopes for this resource_server" message, because split_path/1 mis-splits the dotted key.
  • Gated by maybe_skip_authz/1, so it skips cleanly on a pre-floor broker series that lacks the arity-4 scope API rather than failing.
  • Pins upstream behavior: if split_path is later changed (or a dedicated flat-key accessor is added), this test flips and forces a coordinated broker-dependency bump — surfacing the drift as a test failure the moment the dependency changes.

• src/aws_auth_validate_oauth.erl — corrects the module's over-trust caveat comment to reflect that scope authorization can now be checked via the optional authz_check block (delegated to aws_auth_validate_oauth_authz), while noting the documented resource-server-field overlay parity gap.

Why pin rather than fix

Fixing the dotted-key handling locally in the validation endpoint would make it report a grant the live broker would still refuse — a false pass, and exactly the parity regression the endpoint exists to avoid. The correct behavior is to mirror the current upstream behavior and pin it, so the endpoint and the real broker always agree. The follow-on escaped-dot support (once rabbitmq-server 16985 lands upstream) is tracked separately in #161, which builds on this PR.

Verification

• gmake eunit t=aws_auth_validate_oauth_tests — all tests pass; the new pin test's assertions confirmed executing (not skipped) with the OAuth backend on the code path.
• erlfmt -c clean.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[lukebakken]

Reviewed 7e07cde. The doc-comment rewrite is accurate (authz_check and the four authz fields are real entries in allowed_fields/0, the "no scopes for this resource_server" reason string exists in aws_auth_validate_oauth_authz.erl, and the split_path/1 characterization matches upstream verbatim), and the new pin's mechanism is real where it runs.

The comments below are all about the pin test's efficacy as a drift detector, not a logic bug in shipped code (the change is test-only plus a comment). Three ways the pin can miss the drift it exists to catch. None are blocking; whether they matter is a judgment call for a deliberate parity pin, but they are worth weighing before merge.

[lukebakken]

This pin is unconditionally skipped on the branch it lives on, which undermines the comment's promise (lines 1054-1056) to "surface regressions as test failures the moment the dependency changes."

On this checkout the resolved rabbitmq_auth_backend_oauth2/include/oauth2.hrl has no scope_pattern_syntax field and rabbit_oauth2_scope:resource_access is arity-3, so the Makefile does not define HAVE_OAUTH2_RESOURCE_SERVER. aws_auth_validate_oauth_authz then compiles its -else branch where available/0 returns false, so maybe_skip_authz/1 returns [] and this body never runs.

Concrete drift that slips through: if upstream backports only the split_path/1 fix for #16947 to a v4.3.x-series oauth2 backend without also landing the arity-4 scope API, available/0 stays false, the pin stays skipped, and the regression it targets goes unnoticed here. The pin fires only on a broker series that ships the full arity-4 API. Worth a note in the comment that the pin is dormant until the arity-4 scope API is present, so a reader does not assume green means "still broken as expected."

[lukebakken]

These two assertions are byte-identical to authz_no_effective_scopes_after_prefix_test_ (line 994-995), which pins the same {error, authz_unverified, _} + "no scopes for this resource_server" outcome for a completely different cause (a token whose scopes all have the wrong prefix).

Because any additional_scopes_key that fails to resolve for any reason (key renamed, feature removed, value silently ignored) yields the same category and message, this pin cannot distinguish the dotted-key split_path mis-split from unrelated no-scopes outcomes. It only detects a change once effective scopes become non-empty, not when the dotted-key path breaks in some other way. If the goal is to isolate the mis-split specifically, consider asserting on something that is unique to this path (for example, that a control token carrying the same scopes under a non-dotted key does reach ok, so the dotted vs non-dotted key is the only difference).

[lukebakken]

The flip-to-ok contract the comment describes (lines 1049-1052) depends on the exact shape of the eventual upstream fix, and a plausible alternative fix would leave this pin red after #16947 is resolved.

The value here is already rabbitmq.-prefixed. It flips to ok only if the fix returns this list verbatim before the scope_prefix "rabbitmq." strip. If upstream instead resolves additional_scopes_key values through the resource_server_id map-prefixing branch (prepending "rabbitmq."), the scopes become rabbitmq.rabbitmq.write:*/*, the strip leaves rabbitmq.write:*/*, resource_access is false, and the pin stays authz_unverified even though the drift was fixed. Using an unprefixed value here (e.g. write:*/*, letting the prefix logic add rabbitmq.) would make the flip robust to either fix shape.