Skip to content

[Event Hubs Client] Track Two (Credential Fix) #8786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsquire merged 1 commit into from
Nov 14, 2019
Merged

[Event Hubs Client] Track Two (Credential Fix) #8786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 17 additions & 45 deletions sdk/eventhub/Azure.Messaging.EventHubs/src/Authorization/SharedAccessSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,19 @@ public SharedAccessSignature(string sharedAccessSignature) : this(sharedAccessSi
/// <param name="value">The shared access signature to be used for authorization.</param>
/// <param name="signatureExpiration">The date and time that the shared access signature expires, in UTC.</param>
///
/// <remarks>
/// This constructor is intended to support cloning of the signature and internal testing,
/// allowing for direct setting of the property values without validation or adjustment.
/// </remarks>
///
internal SharedAccessSignature(string eventHubResource,
string sharedAccessKeyName,
string sharedAccessKey,
string value,
DateTimeOffset signatureExpiration)
public SharedAccessSignature(string eventHubResource,
string sharedAccessKeyName,
string sharedAccessKey,
string value,
DateTimeOffset signatureExpiration)
{
Argument.AssertNotNullOrEmpty(eventHubResource, nameof(eventHubResource));
Argument.AssertNotNullOrEmpty(sharedAccessKeyName, nameof(sharedAccessKeyName));
Argument.AssertNotNullOrEmpty(sharedAccessKey, nameof(sharedAccessKey));

Argument.AssertNotTooLong(sharedAccessKeyName, MaximumKeyNameLength, nameof(sharedAccessKeyName));
Argument.AssertNotTooLong(sharedAccessKey, MaximumKeyLength, nameof(sharedAccessKey));

Resource = eventHubResource;
SharedAccessKeyName = sharedAccessKeyName;
SharedAccessKey = sharedAccessKey;
Expand All @@ -177,14 +179,14 @@ internal SharedAccessSignature(string eventHubResource,
}

/// <summary>
/// Extends the period for which the shared access signature is considered valid by adjusting the
/// calculated expiration time. Upon successful extension, the <see cref="Value" /> of the signature will
/// be updated with the new expiration.
/// Creates a new signature with the specified period for which the shared access signature is considered valid.
/// </summary>
///
/// <param name="signatureValidityDuration">The duration that the signature should be considered valid.</param>
///
public void ExtendExpiration(TimeSpan signatureValidityDuration)
/// <returns>A new <see cref="SharedAccessSignature" /> based on the same key, but with a new expiration time.</returns>
///
public SharedAccessSignature CloneWithNewExpiration(TimeSpan signatureValidityDuration)
{
Argument.AssertNotNegative(signatureValidityDuration, nameof(signatureValidityDuration));

Expand All @@ -195,30 +197,9 @@ public void ExtendExpiration(TimeSpan signatureValidityDuration)
throw new InvalidOperationException(Resources.SharedAccessKeyIsRequired);
}

SignatureExpiration = DateTimeOffset.UtcNow.Add(signatureValidityDuration);
Value = BuildSignature(Resource, SharedAccessKeyName, SharedAccessKey, SignatureExpiration);
return new SharedAccessSignature(Resource, SharedAccessKeyName, SharedAccessKey, signatureValidityDuration);
}

/// <summary>
/// Returns a hash code for this instance.
/// </summary>
///
/// <returns>A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.</returns>
///
[EditorBrowsable(EditorBrowsableState.Never)]
public override int GetHashCode() => base.GetHashCode();

/// <summary>
/// Determines whether the specified <see cref="System.Object" /> is equal to this instance.
/// </summary>
///
/// <param name="obj">The <see cref="System.Object" /> to compare with this instance.</param>
///
/// <returns><c>true</c> if the specified <see cref="System.Object" /> is equal to this instance; otherwise, <c>false</c>.</returns>
///
[EditorBrowsable(EditorBrowsableState.Never)]
public override bool Equals(object obj) => base.Equals(obj);

/// <summary>
/// Converts the instance to string representation.
/// </summary>
Expand All @@ -227,15 +208,6 @@ public void ExtendExpiration(TimeSpan signatureValidityDuration)
///
public override string ToString() => Value;

/// <summary>
/// Creates a new copy of the current <see cref="SharedAccessSignature" />, cloning its attributes into a new instance.
/// </summary>
///
/// <returns>A new copy of <see cref="SharedAccessSignature" />.</returns>
///
internal SharedAccessSignature Clone() =>
new SharedAccessSignature(Resource, SharedAccessKeyName, SharedAccessKey, Value, SignatureExpiration);

/// <summary>
/// Parses a shared access signature into its component parts.
/// </summary>
Expand Down
15 changes: 12 additions & 3 deletions sdk/eventhub/Azure.Messaging.EventHubs/src/Authorization/SharedAccessSignatureCredential.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,19 @@ namespace Azure.Messaging.EventHubs.Authorization
internal class SharedAccessSignatureCredential : TokenCredential
{
/// <summary>The buffer to apply when considering refreshing; signatures that expire less than this duration will be refreshed.</summary>
private static readonly TimeSpan SignatureRefreshBuffer = TimeSpan.FromMinutes(5);
private static readonly TimeSpan SignatureRefreshBuffer = TimeSpan.FromMinutes(10);

/// <summary>The length of time extend signature validity, if a token was requested.</summary>
private static readonly TimeSpan SignatureExtensionDuration = TimeSpan.FromMinutes(30);

/// <summary>Provides a target for synchronization to guard against concurrent token expirations.</summary>
private readonly object ExtensionSyncRoot = new object();

/// <summary>
/// The shared access signature that forms the basis of this security token.
/// </summary>
///
public SharedAccessSignature SharedAccessSignature { get; }
private SharedAccessSignature SharedAccessSignature { get; set; }

/// <summary>
/// Initializes a new instance of the <see cref="SharedAccessSignatureCredential"/> class.
Expand Down Expand Up @@ -57,7 +60,13 @@ public override AccessToken GetToken(TokenRequestContext requestContext,
{
if (SharedAccessSignature.SignatureExpiration <= DateTimeOffset.UtcNow.Add(SignatureRefreshBuffer))
{
SharedAccessSignature.ExtendExpiration(SignatureExtensionDuration);
lock (ExtensionSyncRoot)
{
if (SharedAccessSignature.SignatureExpiration <= DateTimeOffset.UtcNow.Add(SignatureRefreshBuffer))
{
SharedAccessSignature = SharedAccessSignature.CloneWithNewExpiration(SignatureExtensionDuration);
}
}
}

return new AccessToken(SharedAccessSignature.Value, SharedAccessSignature.SignatureExpiration);
Expand Down
2 changes: 1 addition & 1 deletion sdk/eventhub/Azure.Messaging.EventHubs/tests/Amqp/CbsTokenProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public async Task GetTokenAsyncPopulatesUsingTheCredentialAccessToken()
public async Task GetTokenAsyncSetsTheCorrectTypeForSharedAccessSignatureTokens()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.Parse("2015-10-27T00:00:00Z"));
var signature = new SharedAccessSignature("hub", "keyName", "key", value, DateTimeOffset.Parse("2015-10-27T00:00:00Z"));
var sasCredential = new SharedAccessSignatureCredential(signature);
var credential = new EventHubTokenCredential(sasCredential, "test");
var provider = new CbsTokenProvider(credential, default);
Expand Down
47 changes: 37 additions & 10 deletions ...venthub/Azure.Messaging.EventHubs/tests/Authorization/EventHubSharedKeyCredentialTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ public void ConstructorValidatesInitializesProperties()
var name = "KeyName";
var value = "KeyValue";
var credential = new EventHubSharedKeyCredential(name, value);

var initializedValue = typeof(EventHubSharedKeyCredential)
.GetProperty("SharedAccessKey", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(credential, null);
var initializedValue = GetSharedAccessKey(credential);

Assert.That(credential.SharedAccessKeyName, Is.EqualTo(name), "The shared key name should have been set.");
Assert.That(initializedValue, Is.EqualTo(value), "The shared key should have been set.");
Expand Down Expand Up @@ -93,14 +90,44 @@ public void CovertToSharedAccessSignatureCredentialProducesTheExpectedCredential
var validSpan = TimeSpan.FromHours(4);
var signature = new SharedAccessSignature(resource, keyName, keyValue, validSpan);
var keyCredential = new EventHubSharedKeyCredential(keyName, keyValue);
SharedAccessSignatureCredential sasCredential = keyCredential.ConvertToSharedAccessSignatureCredential(resource, validSpan);
var sasCredential = keyCredential.ConvertToSharedAccessSignatureCredential(resource, validSpan);

Assert.That(sasCredential, Is.Not.Null, "A shared access signature credential should have been created.");
Assert.That(sasCredential.SharedAccessSignature, Is.Not.Null, "The SAS credential should contain a shared access signature.");
Assert.That(sasCredential.SharedAccessSignature.Resource, Is.EqualTo(signature.Resource), "The resource should match.");
Assert.That(sasCredential.SharedAccessSignature.SharedAccessKeyName, Is.EqualTo(signature.SharedAccessKeyName), "The shared access key name should match.");
Assert.That(sasCredential.SharedAccessSignature.SharedAccessKey, Is.EqualTo(signature.SharedAccessKey), "The shared access key should match.");
Assert.That(sasCredential.SharedAccessSignature.SignatureExpiration, Is.EqualTo(signature.SignatureExpiration).Within(TimeSpan.FromSeconds(5)), "The expiration should match.");

var credentialSignature = GetSharedAccessSignature(sasCredential);
Assert.That(credentialSignature, Is.Not.Null, "The SAS credential should contain a shared access signature.");
Assert.That(credentialSignature.Resource, Is.EqualTo(signature.Resource), "The resource should match.");
Assert.That(credentialSignature.SharedAccessKeyName, Is.EqualTo(signature.SharedAccessKeyName), "The shared access key name should match.");
Assert.That(credentialSignature.SharedAccessKey, Is.EqualTo(signature.SharedAccessKey), "The shared access key should match.");
Assert.That(credentialSignature.SignatureExpiration, Is.EqualTo(signature.SignatureExpiration).Within(TimeSpan.FromSeconds(5)), "The expiration should match.");
}

/// <summary>
/// Retrieves the shared access key from the credential using its private accessor.
/// </summary>
///
/// <param name="instance">The instance to retrieve the key from.</param>
///
/// <returns>The shared access key.</returns>
///
private static string GetSharedAccessKey(EventHubSharedKeyCredential instance) =>
(string)
typeof(EventHubSharedKeyCredential)
.GetProperty("SharedAccessKey", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(instance, null);

/// <summary>
/// Retrieves the shared access signature from the credential using its private accessor.
/// </summary>
///
/// <param name="instance">The instance to retrieve the key from.</param>
///
/// <returns>The shared access key.</returns>
///
private static SharedAccessSignature GetSharedAccessSignature(SharedAccessSignatureCredential instance) =>
(SharedAccessSignature)
typeof(SharedAccessSignatureCredential)
.GetProperty("SharedAccessSignature", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(instance, null);
}
}
2 changes: 1 addition & 1 deletion sdk/eventhub/Azure.Messaging.EventHubs/tests/Authorization/EventHubTokenCredentialTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class EventHubTokenCredentialTests
public static IEnumerable<object[]> SharedAccessSignatureCredentialTestCases()
{
TokenCredential credentialMock = Mock.Of<TokenCredential>();
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", "TOkEn!", DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub", "keyName", "key", "TOkEn!", DateTimeOffset.UtcNow.AddHours(4));

yield return new object[] { new SharedAccessSignatureCredential(signature), true };
yield return new object[] { new EventHubSharedKeyCredential("blah", "foo"), true };
Expand Down
32 changes: 23 additions & 9 deletions ...hub/Azure.Messaging.EventHubs/tests/Authorization/SharedAccessSignatureCredentialTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ public void ConstructorValidatesTheSignature()
/// </summary>
///
[Test]
public void ConstructorValidatesInitializesProperties()
public void ConstructorInitializesProperties()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var credential = new SharedAccessSignatureCredential(signature);

Assert.That(credential.SharedAccessSignature, Is.SameAs(signature), "The credential should allow the signature to be accessed.");
Assert.That(GetSharedAccessSignature(credential), Is.SameAs(signature), "The credential should allow the signature to be accessed.");
}

/// <summary>
Expand All @@ -51,7 +51,7 @@ public void ConstructorValidatesInitializesProperties()
public void GetTokenReturnsTheSignatureValue()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var credential = new SharedAccessSignatureCredential(signature);

Assert.That(credential.GetToken(new TokenRequestContext(), default).Token, Is.SameAs(signature.Value), "The credential should return the signature as the token.");
Expand All @@ -65,7 +65,7 @@ public void GetTokenReturnsTheSignatureValue()
public void GetTokenIgnoresScopeAndCancellationToken()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var credential = new SharedAccessSignatureCredential(signature);

Assert.That(credential.GetToken(new TokenRequestContext(new[] { "test", "this" }), CancellationToken.None).Token, Is.SameAs(signature.Value), "The credential should return the signature as the token.");
Expand All @@ -79,7 +79,7 @@ public void GetTokenIgnoresScopeAndCancellationToken()
public async Task GetTokenAsyncReturnsTheSignatureValue()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var credential = new SharedAccessSignatureCredential(signature);
var cancellation = new CancellationTokenSource();
AccessToken token = await credential.GetTokenAsync(new TokenRequestContext(), cancellation.Token);
Expand All @@ -95,7 +95,7 @@ public async Task GetTokenAsyncReturnsTheSignatureValue()
public async Task GetTokenAsyncIgnoresScopeAndCancellationToken()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.AddHours(4));
var credential = new SharedAccessSignatureCredential(signature);
var cancellation = new CancellationTokenSource();
AccessToken token = await credential.GetTokenAsync(new TokenRequestContext(new string[0]), cancellation.Token);
Expand All @@ -111,7 +111,7 @@ public async Task GetTokenAsyncIgnoresScopeAndCancellationToken()
public void GetTokenExtendsAnExpiredToken()
{
var value = "TOkEn!";
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, DateTimeOffset.UtcNow.Subtract(TimeSpan.FromHours(2)));
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, DateTimeOffset.UtcNow.Subtract(TimeSpan.FromHours(2)));
var credential = new SharedAccessSignatureCredential(signature);

var expectedExpiration = DateTimeOffset.Now.Add(GetSignatureExtensionDuration());
Expand All @@ -127,13 +127,27 @@ public void GetTokenExtendsATokenCloseToExpiring()
{
var value = "TOkEn!";
var tokenExpiration = DateTimeOffset.UtcNow.Add(TimeSpan.FromSeconds(GetSignatureRefreshBuffer().TotalSeconds / 2));
var signature = new SharedAccessSignature(string.Empty, "keyName", "key", value, tokenExpiration);
var signature = new SharedAccessSignature("hub-name", "keyName", "key", value, tokenExpiration);
var credential = new SharedAccessSignatureCredential(signature);

var expectedExpiration = DateTimeOffset.Now.Add(GetSignatureExtensionDuration());
Assert.That(credential.GetToken(new TokenRequestContext(), default).ExpiresOn, Is.EqualTo(expectedExpiration).Within(TimeSpan.FromMinutes(1)));
}

/// <summary>
/// Retrieves the shared access signature from the credential using its private accessor.
/// </summary>
///
/// <param name="instance">The instance to retrieve the key from.</param>
///
/// <returns>The shared access key</returns>
///
private static SharedAccessSignature GetSharedAccessSignature(SharedAccessSignatureCredential instance) =>
(SharedAccessSignature)
typeof(SharedAccessSignatureCredential)
.GetProperty("SharedAccessSignature", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(instance, null);

/// <summary>
/// Gets the refresh buffer for the <see cref="SharedAccessSignatureCredential" /> using
/// its private field.
Expand Down
Loading