Skip to content

Commit 6f8f150

Browse files
sugmanuesugmanue
authored
Set the signing clock property (#4341)
* Set the signing clock property * Rename HttpSignerCommon to ClockSignerProperty * Rename HttpSignerCommon to SharedSignerProperty * Improve the javadoc * Move the signing clock property into the HttpSigner interface * Address PR comments
1 parent bfa24e7 commit 6f8f150

File tree

8 files changed

+157
-17
lines changed

8 files changed

+157
-17
lines changed

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/AwsV4HttpSigner.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
package software.amazon.awssdk.http.auth.aws;
1717

18-
import java.time.Clock;
1918
import java.time.Duration;
2019
import software.amazon.awssdk.annotations.SdkPublicApi;
2120
import software.amazon.awssdk.http.auth.aws.internal.signer.DefaultAwsV4HttpSigner;
@@ -43,12 +42,6 @@ public interface AwsV4HttpSigner extends HttpSigner<AwsCredentialsIdentity> {
4342
SignerProperty<String> SERVICE_SIGNING_NAME =
4443
SignerProperty.create(String.class, "ServiceSigningName");
4544

46-
/**
47-
* A {@link Clock} to be used at the time of signing. This property defaults to the time at which signing occurs.
48-
*/
49-
SignerProperty<Clock> SIGNING_CLOCK =
50-
SignerProperty.create(Clock.class, "SigningClock");
51-
5245
/**
5346
* A boolean to indicate whether to double url-encode the resource path when constructing the canonical request. This property
5447
* defaults to true.

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/AwsV4aHttpSigner.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
package software.amazon.awssdk.http.auth.aws;
1717

18-
import java.time.Clock;
1918
import java.time.Duration;
2019
import software.amazon.awssdk.annotations.SdkPublicApi;
2120
import software.amazon.awssdk.http.auth.aws.internal.signer.SignerLoader;
@@ -45,12 +44,6 @@ public interface AwsV4aHttpSigner extends HttpSigner<AwsCredentialsIdentity> {
4544
SignerProperty<String> SERVICE_SIGNING_NAME =
4645
SignerProperty.create(String.class, "ServiceSigningName");
4746

48-
/**
49-
* A {@link Clock} to be used at the time of signing. This property defaults to the time at which signing occurs.
50-
*/
51-
SignerProperty<Clock> SIGNING_CLOCK =
52-
SignerProperty.create(Clock.class, "SigningClock");
53-
5447
/**
5548
* A boolean to indicate whether to double url-encode the resource path when constructing the canonical request. This property
5649
* defaults to true.

‎core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/TestUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import static software.amazon.awssdk.http.auth.aws.AwsV4HttpSigner.REGION_NAME;
44
import static software.amazon.awssdk.http.auth.aws.AwsV4HttpSigner.SERVICE_SIGNING_NAME;
5-
import static software.amazon.awssdk.http.auth.aws.AwsV4HttpSigner.SIGNING_CLOCK;
5+
import static software.amazon.awssdk.http.auth.spi.HttpSigner.SIGNING_CLOCK;
66

77
import java.io.ByteArrayInputStream;
88
import java.net.URI;

‎core/http-auth-spi/src/main/java/software/amazon/awssdk/http/auth/spi/HttpSigner.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.http.auth.spi;
1717

18+
import java.time.Clock;
1819
import java.util.concurrent.CompletableFuture;
1920
import java.util.function.Consumer;
2021
import software.amazon.awssdk.annotations.SdkPublicApi;
@@ -31,6 +32,13 @@
3132
@SdkPublicApi
3233
public interface HttpSigner<IdentityT extends Identity> {
3334

35+
/**
36+
* A {@link Clock} to be used to derive the signing time. This property defaults to the system clock.
37+
*
38+
* <p>Note, signing time may not be relevant to some signers.
39+
*/
40+
SignerProperty<Clock> SIGNING_CLOCK = SignerProperty.create(Clock.class, "SigningClock");
41+
3442
/**
3543
* Method that takes in inputs to sign a request with sync payload and returns a signed version of the request.
3644
*

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStage.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

1818
import java.nio.ByteBuffer;
19+
import java.time.Clock;
1920
import java.time.Duration;
2021
import java.util.Optional;
2122
import java.util.concurrent.CompletableFuture;
@@ -100,6 +101,7 @@ private <T extends Identity> CompletableFuture<SdkHttpFullRequest> doSraSign(Sdk
100101
if (context.requestProvider() == null) {
101102
SyncSignRequest.Builder<T> signRequestBuilder = SyncSignRequest
102103
.builder(identity)
104+
.putProperty(HttpSigner.SIGNING_CLOCK, signingClock())
103105
.request(request)
104106
.payload(request.contentStreamProvider().orElse(null));
105107
authSchemeOption.forEachSignerProperty(signRequestBuilder::putProperty);
@@ -110,6 +112,7 @@ private <T extends Identity> CompletableFuture<SdkHttpFullRequest> doSraSign(Sdk
110112

111113
AsyncSignRequest.Builder<T> signRequestBuilder = AsyncSignRequest
112114
.builder(identity)
115+
.putProperty(HttpSigner.SIGNING_CLOCK, signingClock())
113116
.request(request)
114117
.payload(context.requestProvider());
115118
authSchemeOption.forEachSignerProperty(signRequestBuilder::putProperty);
@@ -226,6 +229,14 @@ private boolean shouldDoSraSigning(RequestExecutionContext context) {
226229
&& context.executionAttributes().getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME) != null;
227230
}
228231

232+
/**
233+
* Returns the {@link Clock} used for signing that already accounts for clock skew when detected by the retryable stage.
234+
*/
235+
private Clock signingClock() {
236+
int offsetInSeconds = dependencies.timeOffset();
237+
return Clock.offset(Clock.systemUTC(), Duration.ofSeconds(-offsetInSeconds));
238+
}
239+
229240
/**
230241
* Always use the client level timeOffset.
231242
*/

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

18+
import java.time.Clock;
1819
import java.time.Duration;
1920
import java.util.concurrent.CompletableFuture;
2021
import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -95,6 +96,7 @@ private <T extends Identity> SdkHttpFullRequest doSraSign(SdkHttpFullRequest req
9596
T identity) {
9697
SyncSignRequest.Builder<T> signRequestBuilder = SyncSignRequest
9798
.builder(identity)
99+
.putProperty(HttpSigner.SIGNING_CLOCK, signingClock())
98100
.request(request)
99101
.payload(request.contentStreamProvider().orElse(null));
100102
AuthSchemeOption authSchemeOption = selectedAuthScheme.authSchemeOption();
@@ -185,6 +187,14 @@ private boolean shouldDoSraSigning(RequestExecutionContext context) {
185187
&& context.executionAttributes().getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME) != null;
186188
}
187189

190+
/**
191+
* Returns the {@link Clock} used for signing that already accounts for clock skew when detected by the retryable stage.
192+
*/
193+
private Clock signingClock() {
194+
int offsetInSeconds = dependencies.timeOffset();
195+
return Clock.offset(Clock.systemUTC(), Duration.ofSeconds(-offsetInSeconds));
196+
}
197+
188198
/**
189199
* Always use the client level timeOffset.
190200
*/

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStageTest.java

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.within;
1920
import static org.mockito.ArgumentMatchers.any;
2021
import static org.mockito.ArgumentMatchers.eq;
2122
import static org.mockito.Mockito.mock;
@@ -28,6 +29,8 @@
2829
import static software.amazon.awssdk.core.metrics.CoreMetric.SIGNING_DURATION;
2930

3031
import java.nio.ByteBuffer;
32+
import java.time.Instant;
33+
import java.time.temporal.ChronoUnit;
3134
import java.util.concurrent.CompletableFuture;
3235
import org.junit.Before;
3336
import org.junit.Test;
@@ -67,7 +70,7 @@
6770

6871
@RunWith(MockitoJUnitRunner.class)
6972
public class AsyncSigningStageTest {
70-
73+
private static final int TEST_TIME_OFFSET = 17;
7174
private static final SignerProperty<String> SIGNER_PROPERTY = SignerProperty.create(String.class, "key");
7275

7376
@Mock
@@ -134,6 +137,59 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception {
134137
assertThat(signRequest.request()).isSameAs(request);
135138
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
136139

140+
// Assert that the time offset set was zero
141+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
142+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
143+
.isCloseTo(Instant.now(), within(10, ChronoUnit.MILLIS));
144+
145+
// assert that metrics are collected
146+
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
147+
148+
verifyNoInteractions(oldSigner);
149+
}
150+
151+
152+
@Test
153+
public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception {
154+
// Set up a scheme with a signer property
155+
SelectedAuthScheme<Identity> selectedAuthScheme = new SelectedAuthScheme<>(
156+
CompletableFuture.completedFuture(identity),
157+
signer,
158+
AuthSchemeOption.builder()
159+
.schemeId("my.auth#myAuth")
160+
.putSignerProperty(SIGNER_PROPERTY, "value")
161+
.build());
162+
RequestExecutionContext context = createContext(selectedAuthScheme);
163+
164+
// Setup the timeoffset to test that the clock is setup properly.
165+
httpClientDependencies.updateTimeOffset(TEST_TIME_OFFSET);
166+
167+
SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build();
168+
when(signer.sign(Mockito.<SyncSignRequest<? extends Identity>>any()))
169+
.thenReturn(SyncSignedRequest.builder()
170+
.request(signedRequest)
171+
.build());
172+
173+
SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build();
174+
SdkHttpFullRequest result = stage.execute(request, context).join();
175+
176+
assertThat(result).isSameAs(signedRequest);
177+
// assert that interceptor context is updated with result
178+
assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result);
179+
180+
// assert that the input to the signer is as expected, including that signer properties are set
181+
verify(signer).sign(syncSignRequestCaptor.capture());
182+
SyncSignRequest<? extends Identity> signRequest = syncSignRequestCaptor.getValue();
183+
assertThat(signRequest.identity()).isSameAs(identity);
184+
assertThat(signRequest.request()).isSameAs(request);
185+
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
186+
187+
// Assert that the signing clock is setup properly
188+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
189+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
190+
.isCloseTo(Instant.now().minusSeconds(17)
191+
, within(10, ChronoUnit.MILLIS));
192+
137193
// assert that metrics are collected
138194
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
139195

@@ -177,8 +233,13 @@ public void execute_selectedAuthScheme_asyncRequestBody_doesSraSignAsync() throw
177233
AsyncSignRequest<? extends Identity> signRequest = asyncSignRequestCaptor.getValue();
178234
assertThat(signRequest.identity()).isSameAs(identity);
179235
assertThat(signRequest.request()).isSameAs(request);
180-
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
181236
assertThat(signRequest.payload().get()).isSameAs(asyncPayload);
237+
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
238+
239+
// Assert that the time offset set was zero
240+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
241+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
242+
.isCloseTo(Instant.now(), within(10, ChronoUnit.MILLIS));
182243

183244
// assert that metrics are collected
184245
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
@@ -225,6 +286,11 @@ public void execute_selectedNoAuthAuthScheme_doesSraSignAsync() throws Exception
225286
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
226287
assertThat(signRequest.payload().get()).isSameAs(asyncPayload);
227288

289+
// Assert that the time offset set was zero
290+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
291+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
292+
.isCloseTo(Instant.now(), within(10, ChronoUnit.MILLIS));
293+
228294
// assert that metrics are collected
229295
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
230296

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStageTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package software.amazon.awssdk.core.internal.http.pipeline.stages;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.within;
1920
import static org.mockito.ArgumentMatchers.any;
2021
import static org.mockito.ArgumentMatchers.eq;
2122
import static org.mockito.Mockito.verify;
@@ -26,6 +27,8 @@
2627
import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME;
2728
import static software.amazon.awssdk.core.metrics.CoreMetric.SIGNING_DURATION;
2829

30+
import java.time.Instant;
31+
import java.time.temporal.ChronoUnit;
2932
import java.util.concurrent.CompletableFuture;
3033
import org.junit.Before;
3134
import org.junit.Test;
@@ -60,6 +63,7 @@
6063
@RunWith(MockitoJUnitRunner.class)
6164
public class SigningStageTest {
6265

66+
private static final int TEST_TIME_OFFSET = 17;
6367
private static final SignerProperty<String> SIGNER_PROPERTY = SignerProperty.create(String.class, "key");
6468

6569
@Mock
@@ -122,6 +126,56 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception {
122126
assertThat(signRequest.identity()).isSameAs(identity);
123127
assertThat(signRequest.request()).isSameAs(request);
124128
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
129+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
130+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
131+
.isCloseTo(Instant.now(), within(10, ChronoUnit.MILLIS));
132+
133+
// assert that metrics are collected
134+
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
135+
136+
verifyNoInteractions(oldSigner);
137+
}
138+
139+
@Test
140+
public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception {
141+
// Set up a scheme with a signer property
142+
SelectedAuthScheme<Identity> selectedAuthScheme = new SelectedAuthScheme<>(
143+
CompletableFuture.completedFuture(identity),
144+
signer,
145+
AuthSchemeOption.builder()
146+
.schemeId("my.auth#myAuth")
147+
.putSignerProperty(SIGNER_PROPERTY, "value")
148+
.build());
149+
RequestExecutionContext context = createContext(selectedAuthScheme);
150+
151+
// Setup the timeoffset to test that the clock is setup properly.
152+
httpClientDependencies.updateTimeOffset(TEST_TIME_OFFSET);
153+
154+
SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build();
155+
when(signer.sign(Mockito.<SyncSignRequest<? extends Identity>>any()))
156+
.thenReturn(SyncSignedRequest.builder()
157+
.request(signedRequest)
158+
.build());
159+
160+
SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build();
161+
SdkHttpFullRequest result = stage.execute(request, context);
162+
163+
assertThat(result).isSameAs(signedRequest);
164+
// Assert that interceptor context is updated with result
165+
assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result);
166+
167+
// Assert that the input to the signer is as expected, including that signer properties are set
168+
verify(signer).sign(signRequestCaptor.capture());
169+
SyncSignRequest<? extends Identity> signRequest = signRequestCaptor.getValue();
170+
assertThat(signRequest.identity()).isSameAs(identity);
171+
assertThat(signRequest.request()).isSameAs(request);
172+
assertThat(signRequest.property(SIGNER_PROPERTY)).isEqualTo("value");
173+
174+
// Assert that the signing clock is setup properly
175+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
176+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
177+
.isCloseTo(Instant.now().minusSeconds(17)
178+
, within(10, ChronoUnit.MILLIS));
125179

126180
// assert that metrics are collected
127181
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
@@ -160,6 +214,11 @@ public void execute_selectedNoAuthAuthScheme_doesSraSign() throws Exception {
160214
assertThat(signRequest.request()).isSameAs(request);
161215
assertThat(signRequest.property(SIGNER_PROPERTY)).isNull();
162216

217+
// Assert that the time offset set was zero
218+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK)).isNotNull();
219+
assertThat(signRequest.property(HttpSigner.SIGNING_CLOCK).instant())
220+
.isCloseTo(Instant.now(), within(10, ChronoUnit.MILLIS));
221+
163222
// assert that metrics are collected
164223
verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any());
165224

0 commit comments

Comments
 (0)