Skip to content

Commit 337efe5

Browse files
authored
fix: keep API keys within the request origin (#1021)
## Summary Keep `api-key` headers within the original request origin when following redirects. ## Changes - Remove `api-key` from redirected requests when the destination scheme, hostname, or port differs from the original request. - Preserve the header for same-origin requests and redirects. ## Compatibility This changes cross-origin redirect behavior for all `api-key` headers, including manually supplied headers; it is not limited to Azure authentication. A redirected destination that requires the forwarded key may now reject the request. Applications relying on this behavior should configure the authenticated destination directly or have their gateway proxy upstream instead of redirecting the client across origins. Same-origin redirects are unaffected. This change is intended for a minor release under the documented semantic-versioning exception for changes not expected to affect the vast majority of users. Co-authored-by: dpiet-oai <326640584+dpiet-oai@users.noreply.github.com>
1 parent 81f7f83 commit 337efe5

3 files changed

Lines changed: 122 additions & 3 deletions

File tree

‎.castiron.stats.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
schema_version: 1
2-
generation_id: 83660139-66f0-496d-b857-53e5c96b385e
2+
generation_id: e6bf81c1-895b-4eed-bda4-d3713d677592
33
openapi_spec_hash: 12fb4f3ab9b96e5acefc54dceacd92cc
44
openapi_transformed_spec_hash: 336279ed1cb704ff922280d0ae0b4470
55
config_hash: 6b3cc69eb8ac6b5a19e4f865bb7e93fa
6-
codegen_sha: edc29b00bf8f69227852ab87f3f8deb05ef3919c
6+
codegen_sha: 4e8acd224e751c0577180c9f4cae262f52643e17
77
codegen_hash: e4e4a0e129b97287ab79d7a523334642e9b12e96d1d48171103929b19aa0fbba
8-
public_codegen_sha: 1b62f8f4f8ad50a2a1b423f302b3f7d7d28baa0c
8+
public_codegen_sha: 770860394e8ddd3ef6786450011bfc909df3129f

‎openai-java-client-okhttp/src/main/kotlin/com/openai/client/okhttp/OkHttpClient.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,21 @@ internal constructor(@JvmSynthetic internal val okHttpClient: okhttp3.OkHttpClie
184184
.retryOnConnectionFailure(false)
185185
.followRedirects(followRedirects)
186186
.followSslRedirects(followRedirects)
187+
.addNetworkInterceptor { chain ->
188+
val request = chain.request()
189+
val originalUrl = chain.call().request().url
190+
val url = request.url
191+
val sameOrigin =
192+
originalUrl.scheme == url.scheme &&
193+
originalUrl.host == url.host &&
194+
originalUrl.port == url.port
195+
// OkHttp strips Authorization on cross-origin redirects, but does not
196+
// recognize the Azure API-key header as a credential.
197+
chain.proceed(
198+
if (sameOrigin) request
199+
else request.newBuilder().removeHeader("api-key").build()
200+
)
201+
}
187202
.connectTimeout(timeout.connect())
188203
.readTimeout(timeout.read())
189204
.writeTimeout(timeout.write())
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package com.openai.client.okhttp
2+
3+
import com.openai.core.RequestOptions
4+
import com.openai.core.http.HttpMethod
5+
import com.openai.core.http.HttpRequest
6+
import java.util.concurrent.TimeUnit
7+
import okhttp3.mockwebserver.MockResponse
8+
import okhttp3.mockwebserver.MockWebServer
9+
import org.assertj.core.api.Assertions.assertThat
10+
import org.junit.jupiter.api.Test
11+
12+
internal class RedirectCredentialBoundaryTest {
13+
14+
@Test fun crossOriginRedirectDoesNotForwardAzureApiKey() = checkCrossOriginRedirect(false)
15+
16+
@Test fun asyncCrossOriginRedirectDoesNotForwardAzureApiKey() = checkCrossOriginRedirect(true)
17+
18+
private fun checkCrossOriginRedirect(async: Boolean) {
19+
MockWebServer().use { origin ->
20+
MockWebServer().use { destination ->
21+
origin.enqueue(
22+
MockResponse()
23+
.setResponseCode(302)
24+
.setHeader("Location", destination.url("/redirected"))
25+
)
26+
destination.enqueue(MockResponse().setBody("ok"))
27+
val request = request(origin)
28+
29+
OkHttpClient.builder().build().use { client ->
30+
val response =
31+
if (async) {
32+
client
33+
.executeAsync(request, RequestOptions.none())
34+
.get(5, TimeUnit.SECONDS)
35+
} else {
36+
client.execute(request, RequestOptions.none())
37+
}
38+
response.use { assertThat(it.statusCode()).isEqualTo(200) }
39+
}
40+
41+
assertThat(origin.takeRequest(5, TimeUnit.SECONDS)?.getHeader("api-key"))
42+
.isEqualTo("test-azure-key")
43+
val redirected = destination.takeRequest(5, TimeUnit.SECONDS)
44+
assertThat(redirected).isNotNull()
45+
assertThat(redirected?.getHeader("api-key")).isNull()
46+
assertThat(redirected?.getHeader("X-Test")).isEqualTo("retained")
47+
}
48+
}
49+
}
50+
51+
@Test
52+
fun sameOriginRedirectKeepsAzureApiKey() {
53+
MockWebServer().use { origin ->
54+
origin.enqueue(MockResponse().setResponseCode(302).setHeader("Location", "/redirected"))
55+
origin.enqueue(MockResponse().setBody("ok"))
56+
57+
OkHttpClient.builder().build().use { client ->
58+
client.execute(request(origin), RequestOptions.none()).close()
59+
}
60+
61+
assertThat(origin.takeRequest(5, TimeUnit.SECONDS)?.getHeader("api-key"))
62+
.isEqualTo("test-azure-key")
63+
assertThat(origin.takeRequest(5, TimeUnit.SECONDS)?.getHeader("api-key"))
64+
.isEqualTo("test-azure-key")
65+
}
66+
}
67+
68+
@Test
69+
fun redirectChainKeepsCredentialsWithinOriginalOrigin() {
70+
MockWebServer().use { origin ->
71+
MockWebServer().use { destination ->
72+
origin.enqueue(
73+
MockResponse()
74+
.setResponseCode(302)
75+
.setHeader("Location", destination.url("/redirected"))
76+
)
77+
destination.enqueue(
78+
MockResponse()
79+
.setResponseCode(302)
80+
.setHeader("Location", origin.url("/returned"))
81+
)
82+
origin.enqueue(MockResponse().setBody("ok"))
83+
84+
OkHttpClient.builder().build().use { client ->
85+
client.execute(request(origin), RequestOptions.none()).close()
86+
}
87+
88+
origin.takeRequest(5, TimeUnit.SECONDS)
89+
assertThat(destination.takeRequest(5, TimeUnit.SECONDS)?.getHeader("api-key"))
90+
.isNull()
91+
assertThat(origin.takeRequest(5, TimeUnit.SECONDS)?.getHeader("api-key"))
92+
.isEqualTo("test-azure-key")
93+
}
94+
}
95+
}
96+
97+
private fun request(origin: MockWebServer): HttpRequest =
98+
HttpRequest.builder()
99+
.method(HttpMethod.GET)
100+
.baseUrl(origin.url("/original").toString())
101+
.putHeader("Api-Key", "test-azure-key")
102+
.putHeader("X-Test", "retained")
103+
.build()
104+
}

0 commit comments

Comments
 (0)