Skip to content

Add token scope tests for Execution API routes.#68918

Merged
ferruzzi merged 2 commits into
apache:mainfrom
aws-mwaa:ferruzzi/api-tokens/scope-tests
Jun 23, 2026
Merged

Add token scope tests for Execution API routes.#68918
ferruzzi merged 2 commits into
apache:mainfrom
aws-mwaa:ferruzzi/api-tokens/scope-tests

Conversation

@ferruzzi

@ferruzzi ferruzzi commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Response to #66608 (tagging @seanghaeli ), these tests would assert that no API token scopes are modified unintentionally.

I ran these tests on main before the revert and they failed on the token-scope creep that @ashb found, then again after rebasing onto the reverted main and the tests all pass.

There is a section in the module docstring explaining how to maintain the test. The policy would be "any new API route which doesn't use ONLY token:execution needs to be added to the constant". So the vast majority of existing (and future??) routes are unaffected.

To be clear: Execution-only routes are the default by far, so their implementation is unaffected. Someone adding or removing an execution-only route does not have to worry about these tests at all.

Usage/Test Cases

Adding tokens:

  • A new token on an existing execution-only route (What happened here): test_all_default_routes_are_execution_only fails.
  • A new token to an existing multi-token route: test_non_default_route_matches_policy fails.

Removing tokens:

  • A token removed from an execution-only route: test_all_default_routes_are_execution_only fails [1]
  • A token removed from an existing multi-token route: test_non_default_route_matches_policy fails.

Adding Routes:

  • A new execution-only route is added: No action required; tests all pass
  • A new multi-token route is added (without adding it to the exception list): test_all_default_routes_are_execution_only fails.

Removing Routes:

  • An existing multi-token route is removed without updating the policy: test_non_default_route_still_registered fails.
  • An existing execution-only route is removed: No action required; tests all pass

[1]: Also of note, that leaves a route with no token which likely shouldn't happen... but it's covered.

@boring-cyborg boring-cyborg Bot added the area:API Airflow's REST/HTTP API label Jun 23, 2026
@ferruzzi ferruzzi requested review from ashb, kaxil and o-nikolas June 23, 2026 21:02
@ferruzzi ferruzzi force-pushed the ferruzzi/api-tokens/scope-tests branch from f202bd4 to de0d3f8 Compare June 23, 2026 21:04

@ashb ashb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ferruzzi - Great idea!

@o-nikolas o-nikolas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this is great to have, thanks @ferruzzi

@seanghaeli seanghaeli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much needed to programmatically verify token scope compliance. Approved pending CI

We only use APIRoute (for now?), but it looks like it could theoretically be a Mount or APIWebSocketRoute.  An `isinstance` with an escape hatch scopes it down and makes mypy happy
@ferruzzi ferruzzi merged commit ce72c02 into apache:main Jun 23, 2026
76 checks passed
@ferruzzi ferruzzi deleted the ferruzzi/api-tokens/scope-tests branch June 23, 2026 23:34
cetingokhan pushed a commit to cetingokhan/airflow that referenced this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants