Skip to content

Commit 76d7994

Browse files
authored
fix: handle clientContextParam collisions with builtin config keys (#1788)
1 parent 8b90f36 commit 76d7994

File tree

28 files changed

+756
-98
lines changed

28 files changed

+756
-98
lines changed

.changeset/stale-phones-behave.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@smithy/middleware-endpoint": minor
3+
---
4+
5+
handle clientContextParam collisions with builtin config keys
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { describe, expect, test as it } from "vitest";
2+
import { XYZService } from "xyz";
3+
4+
describe("client context parameters precedence integration test", () => {
5+
it("should handle conflicting vs non-conflicting parameter precedence correctly", async () => {
6+
// For non-conflicting params
7+
const clientWithNonConflicting = new XYZService({
8+
endpoint: "https://localhost",
9+
apiKey: async () => ({ apiKey: "test-key" }),
10+
customParam: "user-custom-value",
11+
clientContextParams: {
12+
apiKey: "test-key",
13+
customParam: "nested-custom-value",
14+
},
15+
});
16+
17+
// Verify that endpoint resolution uses the nested value over root value
18+
const resolvedConfig = clientWithNonConflicting.config;
19+
const effectiveCustomParam = resolvedConfig.clientContextParams?.customParam ?? resolvedConfig.customParam;
20+
expect(effectiveCustomParam).toBe("nested-custom-value");
21+
22+
// For conflicting parameters
23+
const clientWithConflicting = new XYZService({
24+
endpoint: "https://localhost",
25+
apiKey: async () => ({ apiKey: "auth-key" }),
26+
clientContextParams: {
27+
apiKey: "endpoint-key",
28+
},
29+
});
30+
31+
// Verify that both auth and endpoint contexts can coexist
32+
const resolvedConfigConflicting = clientWithConflicting.config;
33+
34+
// Verify endpoint context has the nested value
35+
expect(resolvedConfigConflicting.clientContextParams?.apiKey).toBe("endpoint-key");
36+
37+
// Verify auth context has the auth provider
38+
const authIdentity = await resolvedConfigConflicting.apiKey?.();
39+
expect(authIdentity?.apiKey).toBe("auth-key");
40+
});
41+
});

packages/eventstream-serde-universal/src/eventstream-cbor.integ.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ describe("local model integration test for cbor eventstreams", () => {
99
it("should read and write cbor event streams", async () => {
1010
const client = new XYZService({
1111
endpoint: "https://localhost",
12+
apiKey: async () => ({ apiKey: "test-api-key" }),
13+
clientContextParams: {
14+
apiKey: "test-api-key",
15+
},
1216
});
1317

1418
const body = cbor.serialize({

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,31 @@ describe(createConfigValueProvider.name, () => {
5858
expect(await createConfigValueProvider("v1", "endpoint", config)()).toEqual(sampleUrl);
5959
expect(await createConfigValueProvider("v2", "endpoint", config)()).toEqual(sampleUrl);
6060
});
61+
62+
it("should prioritize clientContextParams over direct properties", async () => {
63+
const config = {
64+
stage: "prod",
65+
clientContextParams: {
66+
stage: "beta",
67+
},
68+
};
69+
expect(await createConfigValueProvider("stage", "stage", config, true)()).toEqual("beta");
70+
});
71+
72+
it("should fall back to direct property when clientContextParams is not provided", async () => {
73+
const config = {
74+
customParam: "direct-value",
75+
};
76+
expect(await createConfigValueProvider("customParam", "customParam", config)()).toEqual("direct-value");
77+
});
78+
79+
it("should fall back to direct property when clientContextParams exists but param is not in it", async () => {
80+
const config = {
81+
customParam: "direct-value",
82+
clientContextParams: {
83+
otherParam: "other-value",
84+
},
85+
};
86+
expect(await createConfigValueProvider("customParam", "customParam", config)()).toEqual("direct-value");
87+
});
6188
});

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,29 @@ import type { Endpoint, EndpointV2 } from "@smithy/types";
99
* it will most likely not contain the config
1010
* value, but we use it as a fallback.
1111
* @param config - container of the config values.
12+
* @param isClientContextParam - whether this is a client context parameter.
1213
*
1314
* @returns async function that will resolve with the value.
1415
*/
1516
export const createConfigValueProvider = <Config extends Record<string, unknown>>(
1617
configKey: string,
1718
canonicalEndpointParamKey: string,
18-
config: Config
19+
config: Config,
20+
isClientContextParam = false
1921
) => {
2022
const configProvider = async () => {
21-
const configValue: unknown = config[configKey] ?? config[canonicalEndpointParamKey];
23+
let configValue: unknown;
24+
25+
if (isClientContextParam) {
26+
// For client context parameters, check clientContextParams first
27+
const clientContextParams = config.clientContextParams as Record<string, unknown> | undefined;
28+
const nestedValue: unknown = clientContextParams?.[configKey];
29+
configValue = nestedValue ?? config[configKey] ?? config[canonicalEndpointParamKey];
30+
} else {
31+
// For built-in parameters and other config properties
32+
configValue = config[configKey] ?? config[canonicalEndpointParamKey];
33+
}
34+
2235
if (typeof configValue === "function") {
2336
return configValue();
2437
}

packages/middleware-endpoint/src/adaptors/getEndpointFromInstructions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,12 @@ export const resolveParams = async <
9191
break;
9292
case "clientContextParams":
9393
case "builtInParams":
94-
endpointParams[name] = await createConfigValueProvider<Config>(instruction.name, name, clientConfig)();
94+
endpointParams[name] = await createConfigValueProvider<Config>(
95+
instruction.name,
96+
name,
97+
clientConfig,
98+
instruction.type !== "builtInParams"
99+
)();
95100
break;
96101
case "operationContextParams":
97102
endpointParams[name] = instruction.get(commandInput);

packages/util-retry/src/retries.integ.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ describe("retries", () => {
2020
it("should retry throttling and transient-error status codes", async () => {
2121
const client = new XYZService({
2222
endpoint: "https://localhost/nowhere",
23+
apiKey: { apiKey: "test-api-key" },
2324
});
2425

2526
requireRequestsFrom(client)
@@ -50,6 +51,7 @@ describe("retries", () => {
5051
it("should retry when a retryable trait is modeled", async () => {
5152
const client = new XYZService({
5253
endpoint: "https://localhost/nowhere",
54+
apiKey: { apiKey: "test-api-key" },
5355
});
5456

5557
requireRequestsFrom(client)
@@ -80,6 +82,7 @@ describe("retries", () => {
8082
it("should retry retryable trait with throttling", async () => {
8183
const client = new XYZService({
8284
endpoint: "https://localhost/nowhere",
85+
apiKey: { apiKey: "test-api-key" },
8386
});
8487

8588
requireRequestsFrom(client)
@@ -110,6 +113,7 @@ describe("retries", () => {
110113
it("should not retry if the error is not modeled with retryable trait and is not otherwise retryable", async () => {
111114
const client = new XYZService({
112115
endpoint: "https://localhost/nowhere",
116+
apiKey: { apiKey: "test-api-key" },
113117
});
114118

115119
requireRequestsFrom(client)

private/my-local-model-schema/src/XYZServiceClient.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@ import {
1313
import { getContentLengthPlugin } from "@smithy/middleware-content-length";
1414
import {
1515
type EndpointInputConfig,
16-
type EndpointRequiredInputConfig,
17-
type EndpointRequiredResolvedConfig,
1816
type EndpointResolvedConfig,
1917
resolveEndpointConfig,
20-
resolveEndpointRequiredConfig,
2118
} from "@smithy/middleware-endpoint";
2219
import {
2320
type RetryInputConfig,
@@ -190,7 +187,6 @@ export type XYZServiceClientConfigType = Partial<__SmithyConfiguration<__HttpHan
190187
ClientDefaults &
191188
RetryInputConfig &
192189
EndpointInputConfig<EndpointParameters> &
193-
EndpointRequiredInputConfig &
194190
EventStreamSerdeInputConfig &
195191
HttpAuthSchemeInputConfig &
196192
ClientInputEndpointParameters;
@@ -209,7 +205,6 @@ export type XYZServiceClientResolvedConfigType = __SmithyResolvedConfiguration<_
209205
RuntimeExtensionsConfig &
210206
RetryResolvedConfig &
211207
EndpointResolvedConfig<EndpointParameters> &
212-
EndpointRequiredResolvedConfig &
213208
EventStreamSerdeResolvedConfig &
214209
HttpAuthSchemeResolvedConfig &
215210
ClientResolvedEndpointParameters;
@@ -242,19 +237,20 @@ export class XYZServiceClient extends __Client<
242237
const _config_1 = resolveClientEndpointParameters(_config_0);
243238
const _config_2 = resolveRetryConfig(_config_1);
244239
const _config_3 = resolveEndpointConfig(_config_2);
245-
const _config_4 = resolveEndpointRequiredConfig(_config_3);
246-
const _config_5 = resolveEventStreamSerdeConfig(_config_4);
247-
const _config_6 = resolveHttpAuthSchemeConfig(_config_5);
248-
const _config_7 = resolveRuntimeExtensions(_config_6, configuration?.extensions || []);
249-
this.config = _config_7;
240+
const _config_4 = resolveEventStreamSerdeConfig(_config_3);
241+
const _config_5 = resolveHttpAuthSchemeConfig(_config_4);
242+
const _config_6 = resolveRuntimeExtensions(_config_5, configuration?.extensions || []);
243+
this.config = _config_6;
250244
this.middlewareStack.use(getSchemaSerdePlugin(this.config));
251245
this.middlewareStack.use(getRetryPlugin(this.config));
252246
this.middlewareStack.use(getContentLengthPlugin(this.config));
253247
this.middlewareStack.use(
254248
getHttpAuthSchemeEndpointRuleSetPlugin(this.config, {
255249
httpAuthSchemeParametersProvider: defaultXYZServiceHttpAuthSchemeParametersProvider,
256250
identityProviderConfigProvider: async (config: XYZServiceClientResolvedConfig) =>
257-
new DefaultIdentityProviderConfig({}),
251+
new DefaultIdentityProviderConfig({
252+
"smithy.api#httpApiKeyAuth": config.apiKey,
253+
}),
258254
})
259255
);
260256
this.middlewareStack.use(getHttpSigningPlugin(this.config));

private/my-local-model-schema/src/auth/httpAuthExtensionConfiguration.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// smithy-typescript generated code
2-
import type { HttpAuthScheme } from "@smithy/types";
2+
import type { ApiKeyIdentity, ApiKeyIdentityProvider, HttpAuthScheme } from "@smithy/types";
33

44
import type { XYZServiceHttpAuthSchemeProvider } from "./httpAuthSchemeProvider";
55

@@ -11,6 +11,8 @@ export interface HttpAuthExtensionConfiguration {
1111
httpAuthSchemes(): HttpAuthScheme[];
1212
setHttpAuthSchemeProvider(httpAuthSchemeProvider: XYZServiceHttpAuthSchemeProvider): void;
1313
httpAuthSchemeProvider(): XYZServiceHttpAuthSchemeProvider;
14+
setApiKey(apiKey: ApiKeyIdentity | ApiKeyIdentityProvider): void;
15+
apiKey(): ApiKeyIdentity | ApiKeyIdentityProvider | undefined;
1416
}
1517

1618
/**
@@ -19,6 +21,7 @@ export interface HttpAuthExtensionConfiguration {
1921
export type HttpAuthRuntimeConfig = Partial<{
2022
httpAuthSchemes: HttpAuthScheme[];
2123
httpAuthSchemeProvider: XYZServiceHttpAuthSchemeProvider;
24+
apiKey: ApiKeyIdentity | ApiKeyIdentityProvider;
2225
}>;
2326

2427
/**
@@ -29,6 +32,7 @@ export const getHttpAuthExtensionConfiguration = (
2932
): HttpAuthExtensionConfiguration => {
3033
const _httpAuthSchemes = runtimeConfig.httpAuthSchemes!;
3134
let _httpAuthSchemeProvider = runtimeConfig.httpAuthSchemeProvider!;
35+
let _apiKey = runtimeConfig.apiKey;
3236
return {
3337
setHttpAuthScheme(httpAuthScheme: HttpAuthScheme): void {
3438
const index = _httpAuthSchemes.findIndex((scheme) => scheme.schemeId === httpAuthScheme.schemeId);
@@ -47,6 +51,12 @@ export const getHttpAuthExtensionConfiguration = (
4751
httpAuthSchemeProvider(): XYZServiceHttpAuthSchemeProvider {
4852
return _httpAuthSchemeProvider;
4953
},
54+
setApiKey(apiKey: ApiKeyIdentity | ApiKeyIdentityProvider): void {
55+
_apiKey = apiKey;
56+
},
57+
apiKey(): ApiKeyIdentity | ApiKeyIdentityProvider | undefined {
58+
return _apiKey;
59+
},
5060
};
5161
};
5262

@@ -57,5 +67,6 @@ export const resolveHttpAuthRuntimeConfig = (config: HttpAuthExtensionConfigurat
5767
return {
5868
httpAuthSchemes: config.httpAuthSchemes(),
5969
httpAuthSchemeProvider: config.httpAuthSchemeProvider(),
70+
apiKey: config.apiKey(),
6071
};
6172
};

private/my-local-model-schema/src/auth/httpAuthSchemeProvider.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
// smithy-typescript generated code
2-
import type {
3-
HandlerExecutionContext,
4-
HttpAuthOption,
5-
HttpAuthScheme,
6-
HttpAuthSchemeParameters,
7-
HttpAuthSchemeParametersProvider,
8-
HttpAuthSchemeProvider,
9-
Provider,
2+
import { doesIdentityRequireRefresh, isIdentityExpired, memoizeIdentityProvider } from "@smithy/core";
3+
import {
4+
type ApiKeyIdentity,
5+
type ApiKeyIdentityProvider,
6+
type HandlerExecutionContext,
7+
type HttpAuthOption,
8+
type HttpAuthScheme,
9+
type HttpAuthSchemeParameters,
10+
type HttpAuthSchemeParametersProvider,
11+
type HttpAuthSchemeProvider,
12+
type Provider,
13+
HttpApiKeyAuthLocation,
1014
} from "@smithy/types";
1115
import { getSmithyContext, normalizeProvider } from "@smithy/util-middleware";
1216

@@ -41,9 +45,14 @@ export const defaultXYZServiceHttpAuthSchemeParametersProvider = async (
4145
};
4246
};
4347

44-
function createSmithyApiNoAuthHttpAuthOption(authParameters: XYZServiceHttpAuthSchemeParameters): HttpAuthOption {
48+
function createSmithyApiHttpApiKeyAuthHttpAuthOption(authParameters: XYZServiceHttpAuthSchemeParameters): HttpAuthOption {
4549
return {
46-
schemeId: "smithy.api#noAuth",
50+
schemeId: "smithy.api#httpApiKeyAuth",
51+
signingProperties: {
52+
name: "X-Api-Key",
53+
in: HttpApiKeyAuthLocation.HEADER,
54+
scheme: undefined,
55+
},
4756
};
4857
}
4958

@@ -59,7 +68,7 @@ export const defaultXYZServiceHttpAuthSchemeProvider: XYZServiceHttpAuthSchemePr
5968
const options: HttpAuthOption[] = [];
6069
switch (authParameters.operation) {
6170
default: {
62-
options.push(createSmithyApiNoAuthHttpAuthOption(authParameters));
71+
options.push(createSmithyApiHttpApiKeyAuthHttpAuthOption(authParameters));
6372
}
6473
}
6574
return options;
@@ -88,6 +97,10 @@ export interface HttpAuthSchemeInputConfig {
8897
* @internal
8998
*/
9099
httpAuthSchemeProvider?: XYZServiceHttpAuthSchemeProvider;
100+
/**
101+
* The API key to use when making requests.
102+
*/
103+
apiKey?: ApiKeyIdentity | ApiKeyIdentityProvider;
91104
}
92105

93106
/**
@@ -113,6 +126,10 @@ export interface HttpAuthSchemeResolvedConfig {
113126
* @internal
114127
*/
115128
readonly httpAuthSchemeProvider: XYZServiceHttpAuthSchemeProvider;
129+
/**
130+
* The API key to use when making requests.
131+
*/
132+
readonly apiKey?: ApiKeyIdentityProvider;
116133
}
117134

118135
/**
@@ -121,7 +138,9 @@ export interface HttpAuthSchemeResolvedConfig {
121138
export const resolveHttpAuthSchemeConfig = <T>(
122139
config: T & HttpAuthSchemeInputConfig
123140
): T & HttpAuthSchemeResolvedConfig => {
141+
const apiKey = memoizeIdentityProvider(config.apiKey, isIdentityExpired, doesIdentityRequireRefresh);
124142
return Object.assign(config, {
125143
authSchemePreference: normalizeProvider(config.authSchemePreference ?? []),
144+
apiKey,
126145
}) as T & HttpAuthSchemeResolvedConfig;
127146
};

0 commit comments

Comments
 (0)