diff --git a/modules/openapi-generator/src/main/resources/rust-server/context.mustache b/modules/openapi-generator/src/main/resources/rust-server/context.mustache index b9ac507eb5e2..f09420da8b8f 100644 --- a/modules/openapi-generator/src/main/resources/rust-server/context.mustache +++ b/modules/openapi-generator/src/main/resources/rust-server/context.mustache @@ -107,7 +107,7 @@ impl Service> for AddContext {{#isBasicBasic}} { use std::ops::Deref; - if let Some(auth) = swagger::auth::from_headers(headers) { + if let Some(auth @ AuthData::Basic(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(auth)); return self.inner.call((request, context)) @@ -118,7 +118,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context)) @@ -130,7 +130,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context)) diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustServerCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustServerCodegenTest.java index 5f73054cbe1b..229cfa57ad2c 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustServerCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustServerCodegenTest.java @@ -3,6 +3,7 @@ import org.openapitools.codegen.DefaultGenerator; import org.openapitools.codegen.TestUtils; import org.openapitools.codegen.config.CodegenConfigurator; +import org.testng.Assert; import org.testng.annotations.Test; import java.io.File; @@ -214,4 +215,105 @@ public void testBinaryRequestBodyNotCoercedToUtf8() throws IOException { // Clean up target.toFile().deleteOnExit(); } + + /** + * Test that each generated security scheme block in context.rs only matches the auth + * scheme it was generated for (see issue #24095). + * + * Since swagger-rs 7, swagger::auth::from_headers is no longer scheme-typed: it returns + * Option and matches either a Basic or a Bearer Authorization header. Because + * each generated block returns early on a match, an unrestricted block captures requests + * belonging to a different scheme and makes every later security scheme block + * unreachable - including in-header apiKey blocks, which is an authorization bypass. + */ + @Test + public void testAuthSchemeBlocksOnlyMatchTheirOwnScheme() throws IOException { + Path target = Files.createTempDirectory("test"); + final CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName("rust-server") + .setInputSpec("src/test/resources/2_0/rust-server/petstore-with-fake-endpoints-models-for-testing.yaml") + .setSkipOverwrite(false) + .setOutputDir(target.toAbsolutePath().toString().replace("\\", "/")); + List files = new DefaultGenerator().opts(configurator.toClientOptInput()).generate(); + files.forEach(File::deleteOnExit); + + Path contextPath = Path.of(target.toString(), "/src/context.rs"); + TestUtils.assertFileExists(contextPath); + + // The oauth2 (petstore_auth) block must only accept a Bearer header. + TestUtils.assertFileContains(contextPath, + "if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) {"); + // The basic (http_basic_test) block must only accept a Basic header. + TestUtils.assertFileContains(contextPath, + "if let Some(auth @ AuthData::Basic(..)) = swagger::auth::from_headers(headers) {"); + // No block may accept any Authorization header regardless of scheme, which would + // short-circuit the api_key / api_key_query blocks that follow it. + TestUtils.assertFileNotContains(contextPath, + "if let Some(bearer) = swagger::auth::from_headers(headers) {"); + TestUtils.assertFileNotContains(contextPath, + "if let Some(auth) = swagger::auth::from_headers(headers) {"); + + // The in-header apiKey block must still be generated and reachable. + TestUtils.assertFileContains(contextPath, + "if let Some(header) = api_key_from_header(headers, \"api_key\") {"); + + // Clean up + target.toFile().deleteOnExit(); + } + + /** + * Companion to {@link #testAuthSchemeBlocksOnlyMatchTheirOwnScheme()} covering the + * scheme combinations the petstore fixture cannot express. + * + * The petstore fixture pairs `isOAuth` with `isBasicBasic`, and declares HTTP Basic + * last so its block is generated after the apiKey blocks and cannot shadow them. + * This spec instead declares HTTP Basic, then HTTP Bearer, then an in-header apiKey + * scheme. That covers the `isBasicBasic` / `isBasicBearer` pairing - two HTTP schemes + * that both read the Authorization header, and so are the pair most able to swallow + * each other - and puts both of them ahead of an apiKey block, which is the ordering + * that turns an unrestricted block into an authorization bypass: the block claims + * credentials for a scheme it does not handle, returns early, and the apiKey block + * below it never runs. + */ + @Test + public void testOverlappingAuthSchemeBlocksDoNotShadowEachOther() throws IOException { + Path target = Files.createTempDirectory("test"); + final CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName("rust-server") + .setInputSpec("src/test/resources/3_0/rust-server/overlapping-auth-schemes.yaml") + .setSkipOverwrite(false) + .setOutputDir(target.toAbsolutePath().toString().replace("\\", "/")); + List files = new DefaultGenerator().opts(configurator.toClientOptInput()).generate(); + files.forEach(File::deleteOnExit); + + Path contextPath = Path.of(target.toString(), "/src/context.rs"); + TestUtils.assertFileExists(contextPath); + + String context = Files.readString(contextPath); + + String basicBlock = "if let Some(auth @ AuthData::Basic(..)) = swagger::auth::from_headers(headers) {"; + String bearerBlock = "if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) {"; + String apiKeyBlock = "if let Some(header) = api_key_from_header(headers, \"x-api-key\") {"; + + // Each Authorization-based block must be restricted to its own scheme... + TestUtils.assertFileContains(contextPath, basicBlock); + TestUtils.assertFileContains(contextPath, bearerBlock); + TestUtils.assertFileNotContains(contextPath, + "if let Some(auth) = swagger::auth::from_headers(headers) {"); + TestUtils.assertFileNotContains(contextPath, + "if let Some(bearer) = swagger::auth::from_headers(headers) {"); + // ...and the apiKey block that follows them must still be generated. + TestUtils.assertFileContains(contextPath, apiKeyBlock); + + // Guard the premise of this test: if the generator ever emits these blocks in a + // different order then this spec no longer exercises the shadowing case, and the + // assertions above would silently stop proving anything. + Assert.assertTrue(context.indexOf(basicBlock) < context.indexOf(bearerBlock), + "expected the Basic auth block to be generated before the Bearer auth block"); + Assert.assertTrue(context.indexOf(bearerBlock) < context.indexOf(apiKeyBlock), + "expected the Bearer auth block to be generated before the apiKey block"); + + // Clean up + target.toFile().deleteOnExit(); + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/rust-server/overlapping-auth-schemes.yaml b/modules/openapi-generator/src/test/resources/3_0/rust-server/overlapping-auth-schemes.yaml new file mode 100644 index 000000000000..13acd2f6c195 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/rust-server/overlapping-auth-schemes.yaml @@ -0,0 +1,43 @@ +openapi: 3.0.1 +info: + title: overlapping auth schemes test + version: '1.0' +servers: + - url: 'http://localhost:8080/' +paths: + /ping: + get: + operationId: pingGet + responses: + '201': + description: OK +components: + # This spec exists to exercise the auth-scheme blocks generated into context.rs when + # several schemes compete for the same request. See issue #24095. + # + # Two properties matter, and no other rust-server fixture has both: + # + # * `basicAuth` and `bearerAuth` are HTTP schemes that both read the `Authorization` + # header, so an unrestricted block for either one also matches the other. This is + # the `isBasicBasic` / `isBasicBearer` pairing; the petstore fixture only covers + # `isBasicBasic` alongside `isOAuth`. + # * `apiKeyAuth` is declared last. Blocks are emitted in declaration order and each + # returns early, so an unrestricted Basic or Bearer block does not merely pick the + # wrong scheme - it makes the apiKey block below it unreachable, which is an + # authorization bypass rather than a mislabelling. + securitySchemes: + basicAuth: + scheme: basic + type: http + bearerAuth: + scheme: bearer + bearerFormat: token + type: http + apiKeyAuth: + type: apiKey + name: x-api-key + in: header +security: + - basicAuth: [] + - bearerAuth: [] + - apiKeyAuth: [] diff --git a/samples/server/petstore/rust-server/output/openapi-v3/src/context.rs b/samples/server/petstore/rust-server/output/openapi-v3/src/context.rs index a7ec3155349e..6b378ad1a578 100644 --- a/samples/server/petstore/rust-server/output/openapi-v3/src/context.rs +++ b/samples/server/petstore/rust-server/output/openapi-v3/src/context.rs @@ -105,7 +105,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context)) @@ -114,7 +114,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context)) diff --git a/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/src/context.rs b/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/src/context.rs index 937ef9edc340..2b2e2309a1df 100644 --- a/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/src/context.rs +++ b/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/src/context.rs @@ -105,7 +105,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context)) @@ -135,7 +135,7 @@ impl Service> for AddContext } { use std::ops::Deref; - if let Some(auth) = swagger::auth::from_headers(headers) { + if let Some(auth @ AuthData::Basic(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(auth)); return self.inner.call((request, context)) diff --git a/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/tests/auth_scheme_precedence.rs b/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/tests/auth_scheme_precedence.rs new file mode 100644 index 000000000000..0a0632abe110 --- /dev/null +++ b/samples/server/petstore/rust-server/output/petstore-with-fake-endpoints-models-for-testing/tests/auth_scheme_precedence.rs @@ -0,0 +1,116 @@ +//! Runtime regression tests for auth-scheme precedence in the generated `AddContext` middleware. +//! +//! `swagger::auth::from_headers` returns an *untyped* `AuthData`, matching an +//! `Authorization` header that carries either `Basic` or `Bearer` credentials. Every +//! generated auth block returns early once it matches, so a block that does not check +//! which variant it received will claim credentials belonging to a different scheme and +//! prevent every later block - including API-key blocks - from ever running. +//! +//! This spec generates the blocks in the following order, which is what makes the +//! behaviour observable from the outside: +//! +//! 1. `petstore_auth` - OAuth2, reads `Authorization: Bearer` +//! 2. `api_key` - API key, reads the `api_key` header +//! 3. `api_key_query` - API key, reads the `api_key_query` query parameter +//! 4. `http_basic_test` - HTTP Basic, reads `Authorization: Basic` +//! +//! Presenting Basic credentials alongside an API key therefore proves whether block 1 +//! stays in its lane: if it wrongly claims the Basic credentials it also swallows +//! blocks 2 and 3. + +#![cfg(feature = "server")] + +use std::sync::{Arc, Mutex}; + +use hyper::service::Service; +use hyper::{Request, Response}; +use petstore_with_fake_endpoints_models_for_testing::context::AddContext; +use swagger::auth::AuthData; +use swagger::{EmptyContext, Has}; + +/// Innermost service: records the `Option` that `AddContext` pushed onto the context. +#[derive(Clone, Default)] +struct CaptureAuthData(Arc>>); + +impl Service<(Request, C)> for CaptureAuthData +where + C: Has>, +{ + type Response = Response; + type Error = std::convert::Infallible; + type Future = std::future::Ready>; + + fn call(&self, (_request, context): (Request, C)) -> Self::Future { + let auth_data: &Option = context.get(); + *self.0.lock().expect("lock poisoned") = auth_data.clone(); + std::future::ready(Ok(Response::new(String::new()))) + } +} + +/// Drives a request through `AddContext` and returns the `AuthData` it resolved. +fn resolve_auth_data(uri: &str, headers: &[(&str, &str)]) -> Option { + let capture = CaptureAuthData::default(); + let service = AddContext::<_, EmptyContext>::new(capture.clone()); + + let mut builder = Request::get(uri); + for (name, value) in headers { + builder = builder.header(*name, *value); + } + let request = builder.body(()).expect("request should build"); + + futures::executor::block_on(service.call(request)).expect("service call should succeed"); + + let resolved = capture.0.lock().expect("lock poisoned").clone(); + resolved +} + +/// `dXNlcjpwYXNzd29yZA==` is `user:password`. +const BASIC_HEADER: &str = "Basic dXNlcjpwYXNzd29yZA=="; + +#[test] +fn bearer_block_does_not_claim_basic_credentials() { + // The OAuth2 (Bearer) block is generated first. It must ignore Basic credentials and + // let them fall through to the HTTP Basic block generated last. + assert_eq!( + resolve_auth_data("/", &[("authorization", BASIC_HEADER)]), + Some(AuthData::Basic("user".to_owned(), "password".to_owned())), + ); +} + +#[test] +fn basic_block_does_not_claim_bearer_credentials() { + assert_eq!( + resolve_auth_data("/", &[("authorization", "Bearer some-token")]), + Some(AuthData::Bearer("some-token".to_owned())), + ); +} + +#[test] +fn header_api_key_is_reachable_when_basic_credentials_are_also_present() { + // Regression test: an unguarded Bearer block matches the Basic credentials, returns + // early, and the `api_key` header block below it never runs. + assert_eq!( + resolve_auth_data( + "/", + &[("authorization", BASIC_HEADER), ("api_key", "header-key")], + ), + Some(AuthData::ApiKey("header-key".to_owned())), + ); +} + +#[test] +fn query_api_key_is_reachable_when_basic_credentials_are_also_present() { + // Same regression, for the query-parameter API-key block. + assert_eq!( + resolve_auth_data( + "/?api_key_query=query-key", + &[("authorization", BASIC_HEADER)], + ), + Some(AuthData::ApiKey("query-key".to_owned())), + ); +} + +#[test] +fn no_credentials_resolve_to_no_auth_data() { + assert_eq!(resolve_auth_data("/", &[]), None); +} diff --git a/samples/server/petstore/rust-server/output/ping-bearer-auth/src/context.rs b/samples/server/petstore/rust-server/output/ping-bearer-auth/src/context.rs index 7a52731cc064..7fc19a955278 100644 --- a/samples/server/petstore/rust-server/output/ping-bearer-auth/src/context.rs +++ b/samples/server/petstore/rust-server/output/ping-bearer-auth/src/context.rs @@ -105,7 +105,7 @@ impl Service> for AddContext { use headers::authorization::Bearer; use std::ops::Deref; - if let Some(bearer) = swagger::auth::from_headers(headers) { + if let Some(bearer @ AuthData::Bearer(..)) = swagger::auth::from_headers(headers) { let context = context.push(Some(bearer)); return self.inner.call((request, context))