[rust-server] Generate the auth scheme precedence tests - #24690
Open
twistali wants to merge 2 commits into
Open
Conversation
The auth scheme precedence tests added in OpenAPITools#24607 were hand-written and duplicated across two samples. Because they are not emitted by the templates, they only cover the specs someone remembered to write them for, and they have to be maintained by hand as new samples are added. Emit them from a new `tests-auth-scheme-precedence.mustache` supporting file instead. `RustServerCodegen` publishes a small description of the spec's security schemes (which schemes are declared, the apiKey parameter names, and whether Basic/Bearer are dispatched before the apiKey block); the template selects the test cases and expected outcomes from those flags. This drops the hand-written files and covers four samples instead of two. Reintroducing the untyped `from_headers` bug that OpenAPITools#24607 fixed now fails in all four, where the hand-written tests caught it in two.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Address review feedback on the generated auth scheme precedence tests. The header and query apiKey blocks are shadowed independently: each matches a different part of the request, so a preceding block that fails to claim one may still claim the other. Tracking a single "precedes the first apiKey block" flag got this wrong for specs that declare a query apiKey before an HTTP scheme, e.g. `[queryApiKey, bearer, headerApiKey]`, where the generated test expected `ApiKey` but the runtime resolves `Bearer`. Track precedence separately for each location. Also add the query-side counterparts of the header precedence tests, so a query apiKey declared after Basic or Bearer is checked for reachability too, and make the sole-credential test names reflect what they assert when the scheme is not declared. This takes the suite from 19 to 21 tests.
Member
|
thanks for the pr cc @frol (2017/07) @farcaller (2017/08) @richardwhiuk (2019/07) @paladinzh (2020/05) @jacob-pro (2022/10) @dsteeley (2025/07) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The auth scheme precedence tests added in #24607 were hand-written and duplicated across two samples. Because they are not emitted by the templates, they only cover the specs someone remembered to write them for, and they have to be maintained by hand as new samples are added.
Emit them from a new
tests-auth-scheme-precedence.mustachesupporting file instead.RustServerCodegenpublishes a small description of the spec's security schemes (which schemes are declared, the apiKey parameter names, and whether Basic/Bearer are dispatched before the apiKey block); the template selects the test cases and expected outcomes from those flags.This drops the hand-written files and covers four samples instead of two. Reintroducing the untyped
from_headersbug that #24607 fixed now fails in all four, where the hand-written tests caught it in two.Follow-up to #24607.
#24607 fixed an authorization bypass in rust-server : after swagger-rs made swagger::auth::from_headers untyped, an unrestricted if let Some(auth) = from_headers(headers) in the Basic block would swallow a Bearer request and make every later block — including the in-header apiKey block — unreachable. The fix was to bind with a type-restricted pattern ( Some(auth @ AuthData::Basic(..)) , Some(bearer @ AuthData::Bearer(..)) ).
That PR also added tests, but they were hand-written and checked straight into two sample output directories. That has two problems:
This PR emits them from the templates instead.
What changed
• New supporting file rust-server/tests-auth-scheme-precedence.mustache , emitted as tests/auth_scheme_precedence.rs for any spec that declares a security scheme the generator dispatches on.
• RustServerCodegen gains ~60 lines that publish a small description of the spec's security schemes into the supporting-file bundle: which schemes are declared, the apiKey parameter names, and whether the Basic/Bearer blocks are emitted before the apiKey block. Test-case selection and expected outcomes live in the template, not in Java.
• Both hand-written tests/auth_scheme_precedence.rs files are deleted, replaced by generated ones.
The test drives the generated Api::from(&request) dispatch through a real hyper::Request , so it exercises the emitted code rather than asserting on template text.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Generate auth‑scheme precedence tests for
rust-serverfrom a template and track apiKey shadowing per location. Old: hand‑written tests in two samples and a single “precedes first apiKey” flag. New:RustServerCodegenemitstests/auth_scheme_precedence.rswith separate header/query precedence, expanded cases, and updated samples, without changing runtime behavior.RustServerCodegen.addAuthSchemeTestsToBundlepublishesauthTestHasBasic,authTestHasBearer, per‑location precedence flags (authTestBasicPrecedesHeaderApiKey,authTestBearerPrecedesHeaderApiKey,authTestBasicPrecedesQueryApiKey,authTestBearerPrecedesQueryApiKey), and apiKey names (authTestApiKeyHeaderlowercased;authTestApiKeyQueryas declared). Adds thetests-auth-scheme-precedence.mustachesupporting file only when any scheme exists. OAuth is treated as Bearer.tests-auth-scheme-precedence.mustachegenerates sole‑credential and reachability tests for header and query apiKeys. Test names reflect declared schemes.openapi-v3,overlapping-auth-schemes,petstore-with-fake-endpoints-models-for-testing, andping-bearer-auth. Reintroducing the untypedfrom_headersbug now fails across all four; total tests increase from 19 to 21.Written for commit fb098e2. Summary will update on new commits.