ServiceBus SDK: Rbac support #6393

bainian12345 · 2019-05-22T21:25:41Z

Removed dependency to ADAL other than testing purposes
Added connection string support for RBAC authentication
Modified client and AadTokenProvider APIs to allow more flexible ways to provide authentication

nemakam · 2019-05-22T21:31:13Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/QueueClient.cs

+        /// <param name="receiveMode">Mode of receive of messages. Defaults to <see cref="ReceiveMode"/>.PeekLock.</param>
+        /// <param name="retryPolicy">Retry policy for queue operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the queue, which is opened during the first send/receive operation.</remarks>
+        public static QueueClient CreateWithAzureActiveDirectory(


CreateWithAzureActiveDirectory [](start = 34, length = 30)

Do we need this API? We already have the overload where you can pass your own ITokenProvider. #Closed

We also accept connectionStringBuilder. So lets not duplicate the APIs

In reply to: 286696363 [](ancestors = 286696363)

I'm curious as to the purpose of not just documenting that the consumer should create and use an CreateAzureActiveDirectoryTokenProvider or CreateAzureActiveDirectoryTokenProvider and pass them to the ITokenProvider overload. Is there a benefit to consumers that I'm overlooking?

I don't want to block on this, but I'm still curious as to the answer. I feel that I'm missing some important context...

It's removed and will no longer part of this change. It was an effort to keep consistency with the EventHubs SDK but it seems like there are good reasons to avoid these redundant APIs.

In reply to: 292678097 [](ancestors = 292678097)

nemakam · 2019-05-22T21:32:16Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/QueueClient.cs

+        /// <param name="receiveMode">Mode of receive of messages. Defaults to <see cref="ReceiveMode"/>.PeekLock.</param>
+        /// <param name="retryPolicy">Retry policy for queue operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the queue, which is opened during the first send/receive operation.</remarks>
+        public static QueueClient CreateWithAzureActiveDirectory(


static [](start = 15, length = 6)

Even if this API is needed, this should not be static. This SDK has slightly different API syntax compared to the older one. Here you should be able to do a new QueueClient() , and not QueueClient.CreateWith* #Closed

If we do constructor, the signature will collide with the one below. Eventhubs is also using Create() methods

In reply to: 286696743 [](ancestors = 286696743)

nemakam · 2019-05-22T21:32:29Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/QueueClient.cs

+        /// <param name="receiveMode">Mode of receive of messages. Defaults to <see cref="ReceiveMode"/>.PeekLock.</param>
+        /// <param name="retryPolicy">Retry policy for queue operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the queue, which is opened during the first send/receive operation.</remarks>
+        public static QueueClient CreateWithManagedIdentity(


CreateWithManagedIdentity [](start = 34, length = 25)

Same as above #Closed

nemakam · 2019-05-22T21:34:28Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/ManagedIdentityTokenProvider.cs

@@ -10,7 +10,7 @@ namespace Microsoft.Azure.ServiceBus.Primitives
    /// <summary>
    /// Represents the Azure Active Directory token provider for Azure Managed Service Identity integration.
    /// </summary>
-    public class ManagedServiceIdentityTokenProvider : TokenProvider
+    public class ManagedIdentityTokenProvider : TokenProvider


ManagedIdentityTokenProvider [](start = 17, length = 28)

Do we need this rename? If we rename the public method then it becomes a breaking change.

If there's enough justification for the rename, lets update the major version in the csproj. #Closed

nemakam · 2019-05-22T21:39:48Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs

+        /// <summary>
+        /// Enables Azure Active Directory Managed Identity authentication when set to 'Managed Identity'
+        /// </summary>
+        public string Authentication { get; set; }


Authentication [](start = 22, length = 14)

Curious as to why this is a string? As compared to other options of - Enum or bool #Closed

I'd suggest the enum approach, personally. Asking consumers to pass a magic string seems like it would offer a challenge to discovering usable values via intellisense.

In the case that you're set on a string, maybe consider adding a pseudo-enumeration to allow discovery and help prevent typos, then? Something like:

public static class AuthenticationType { public static const ManagedIdentity = "Managed Identity"; }

That also gives you the advantage of being able to throw doc comments on the type and member to guide the user experience.

Is it expected to extend methods of authentication in the near future?

This is another area that I've not seen a response and have concerns about user experience. I don't want to block on it, as it is consistent with the direction that the Event Hubs client went, but I would love to see some thoughts after usability testing.

nemakam · 2019-05-22T21:42:17Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs

+        }
+
+        void validate()
+        {


nit: Instead of calling during ToString(), you could just do validation within the setter of each of the property..

Given that you have validation within the setter, you don't need this method anymore.

In reply to: 286699955 [](ancestors = 286699955)

nemakam · 2019-05-22T21:43:00Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnection.cs

@@ -218,6 +218,10 @@ void InitializeConnection(ServiceBusConnectionStringBuilder builder)
            {
                this.TokenProvider = new SharedAccessSignatureTokenProvider(builder.SasKeyName, builder.SasKey);
            }
+            else if (!string.Equals(builder.Authentication, "Managed Identity", StringComparison.OrdinalIgnoreCase))


"Managed Identity" [](start = 60, length = 18)

nit: convert to a const string #Closed

nemakam · 2019-05-22T21:43:52Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnection.cs

@@ -218,6 +218,10 @@ void InitializeConnection(ServiceBusConnectionStringBuilder builder)
            {
                this.TokenProvider = new SharedAccessSignatureTokenProvider(builder.SasKeyName, builder.SasKey);
            }
+            else if (!string.Equals(builder.Authentication, "Managed Identity", StringComparison.OrdinalIgnoreCase))


(!string.Equals [](start = 20, length = 15)

Is it supposed to be *!*string.Equals?? #Closed

nemakam · 2019-05-22T21:45:09Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/TopicClient.cs

+        /// <param name="transportType">Transport type.</param>
+        /// <param name="retryPolicy">Retry policy for topic operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the topic, which is opened during the first send/receive operation.</remarks>
+        public static TopicClient CreateWithAzureActiveDirectory(


CreateWithAzureActiveDirectory [](start = 34, length = 30)

Same as QueueClient. We dont need this API #Closed

nemakam · 2019-05-22T21:45:21Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/SubscriptionClient.cs

+        /// <param name="receiveMode">Mode of receive of messages. Defaults to <see cref="ReceiveMode"/>.PeekLock.</param>
+        /// <param name="retryPolicy">Retry policy for subscription operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the subscription, which is opened during the first send/receive operation.</remarks>
+        public static SubscriptionClient CreateWithAzureActiveDirectory(


CreateWithAzureActiveDirectory [](start = 41, length = 30)

Same as QueueClient. We don't need this API #Closed

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TokenProvider.cs

nemakam · 2019-05-22T21:48:11Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/ServiceBusConnectionStringBuilderTests.cs

+        public void InvalidAadConnectionStringTest()
+        {
+            String connecitionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";
+            Assert.Throws<ArgumentException>(() => new ServiceBusConnectionStringBuilder(connecitionString));


connecitionString [](start = 89, length = 17)

nit: typo #Closed

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs

nemakam · 2019-05-22T21:55:58Z

@bainian12345 - Please add @jsquire as a reviewer. #Closed

jsquire · 2019-05-22T22:32:58Z

@bainian12345 - Please add @jsquire as a reviewer.

Thanks, Neeraj. I flagged myself based on your comment. I'll try to loop back on this before the end of the week with feedback.

jsquire · 2019-05-24T21:13:38Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/ManagedIdentityTokenProvider.cs

@@ -10,7 +10,7 @@ namespace Microsoft.Azure.ServiceBus.Primitives
    /// <summary>
    /// Represents the Azure Active Directory token provider for Azure Managed Service Identity integration.


If you're moving forward with the rename, you'll probably want to consider updating this comment as well.

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/AzureActiveDirectoryTokenProvider.cs

jsquire · 2019-05-24T21:21:19Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/QueueClient.cs

+        /// <param name="receiveMode">Mode of receive of messages. Defaults to <see cref="ReceiveMode"/>.PeekLock.</param>
+        /// <param name="retryPolicy">Retry policy for queue operations. Defaults to <see cref="RetryPolicy.Default"/></param>
+        /// <remarks>Creates a new connection to the queue, which is opened during the first send/receive operation.</remarks>
+        public static QueueClient CreateWithAzureActiveDirectory(


I'm curious as to the purpose of not just documenting that the consumer should create and use an CreateAzureActiveDirectoryTokenProvider or CreateAzureActiveDirectoryTokenProvider and pass them to the ITokenProvider overload. Is there a benefit to consumers that I'm overlooking?

jsquire · 2019-05-24T21:23:42Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs

+        /// <summary>
+        /// Enables Azure Active Directory Managed Identity authentication when set to 'Managed Identity'
+        /// </summary>
+        public string Authentication { get; set; }


I'd suggest the enum approach, personally. Asking consumers to pass a magic string seems like it would offer a challenge to discovering usable values via intellisense.

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs

nemakam · 2019-05-31T21:34:01Z

We also need to integrate the management layer with MSI and AAD (rbac support). Is that going to be part of this PR?

nemakam · 2019-05-31T21:37:00Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Microsoft.Azure.ServiceBus.csproj

@@ -2,7 +2,7 @@
  <PropertyGroup>
    <AssemblyTitle>Azure ServiceBus SDK</AssemblyTitle>
    <Description>This is the next generation Azure Service Bus .NET Standard client library that focuses on queues &amp; topics. For more information about Service Bus, see https://azure.microsoft.com/en-us/services/service-bus/</Description>
-    <VersionPrefix>3.4.0</VersionPrefix>
+    <VersionPrefix>3.5.0</VersionPrefix>


3.5.0 [](start = 19, length = 5)

Major version needs to be changed here, and not minor version. But now to think of it, this should not be your responsibility. The person doing the release should take care of updating the version whenever he/she releases. Maybe you can revert this change.
Sounds good @jsquire?

I agree with both of Neeraj's points. These changes are definitely worth a major bump due to some being breaking changes. Better to only version during release, lest it keep getting bumped inadvertently.

My preference would be to update the changelog.md as well as the version at the time when the decision to ship a new major version is made -- or we should figure out an approach to track the required version changes.

The question that occurs to me with that approach how a contributor would know when they need to increment the version and when that has already been done prior? I fear that we may have a gap with respect to "we're planning to release a new version and these changes are a part of it" versus "I'd like to include these changes for whenever a new major version is planned" versus "I'd like to include these priority changes and they should trigger a minor release and be done soon."

FWIW, in the past (old repo) there were milestones and one of them was the next major. A PR/issue that would be a breaking change would be assigned to that milestone. When it comes to collaborators, especially external, during the PR work/review it becomes clear wherever a change is a breaking one or not and what version should it target (major, minor, or a patch).

And, for consideration, it will be extremely helpful to see issues associated with the PRs that would be aggregated for each release, in the notes section. Similar to what was done here: https://github.com/Azure/azure-service-bus-dotnet/releases

nemakam · 2019-05-31T21:42:07Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs

-            await this.PeekLockTestCase(queueClient.InnerSender, queueClient.InnerReceiver, 10);
-        }
-        #pragma warning restore xUnit1013
+        static readonly Uri ServiceBusAudience = new Uri("https://servicebus.azure.net");


ServiceBusAudience [](start = 28, length = 18)

This is probably not used anywhere?

nemakam · 2019-05-31T21:43:14Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs

+        [Fact]
+        [LiveTest]
+        [DisplayTestMethodName]
+        public async Task QueueWithAadTokenProviderTest()


QueueWithAadTokenProviderTest [](start = 26, length = 29)

Why did you remove these tests?

These are more like end-to-end integration tests that also rely on many other services, not sure if it's best practice to have it here.

In reply to: 289559465 [](ancestors = 289559465)

@bainian12345 does sound like a good candidate for a separate project within the solution.

I'm in agreement with Sean's suggestion. There seem to be several "please don't run these as part of the automation" cases; a separate project or at least isolating them in a dedicated class would help to keep them from being confused with other tests.

SeanFeldman · 2019-06-05T04:41:55Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/ServiceBusConnectionStringBuilderTests.cs

+        [Fact]
+        public void InvalidAadConnectionStringTest()
+        {
+            String connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";


Suggested change

String connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";

var connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";

bainian12345 · 2019-06-10T22:40:23Z

connection string support for ManagementClient has been added.

In reply to: 497867339 [](ancestors = 497867339)

jsquire · 2019-06-11T22:17:48Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Management/ManagementClient.cs

            {
                return new SharedAccessSignatureTokenProvider(builder.SasKeyName, builder.SasKey);
            }
+            else if (!string.IsNullOrEmpty(builder.Authentication))
+            {
+                if (builder.Authentication.Equals(ServiceBusConnectionStringBuilder.AuthenticationType.ManagedIdentity, StringComparison.OrdinalIgnoreCase))


You may want to consider simplifying to something like:

else if (String.Equals(builder.Authentication, ServiceBusConnectionStringBuilder.AuthenticationType.ManagedIdentity, StringComparison.OrdinalIgnoreCase))

jsquire · 2019-06-11T22:20:46Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TokenProvider.cs

-        /// <returns>The <see cref="TokenProvider" /> for returning Json web token.</returns>
-        public static TokenProvider CreateAadTokenProvider(AuthenticationContext authContext, ClientAssertionCertificate clientAssertionCertificate)
+        /// <param name="authCallback">The authentication delegate to provide access token.</param>
+        /// <param name="authority">Address of the authority to issue token.</param>


Respectfully, I'm not sure that these comments offer much context or clarity for users who are unfamiliar. You may want to consider revising them to be more explicit or, if not, just removing them.

jsquire · 2019-06-11T22:24:14Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnection.cs

@@ -218,6 +218,13 @@ void InitializeConnection(ServiceBusConnectionStringBuilder builder)
            {
                this.TokenProvider = new SharedAccessSignatureTokenProvider(builder.SasKeyName, builder.SasKey);
            }
+            else if (!string.IsNullOrEmpty(builder.Authentication))


You may want to consider simplifying to something like:

else if (String.Equals(builder.Authentication, ServiceBusConnectionStringBuilder.AuthenticationType.ManagedIdentity, StringComparison.OrdinalIgnoreCase))

jsquire · 2019-06-11T22:25:54Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs


        const string EntityPathConfigName = "EntityPath";
        const string TransportTypeConfigName = "TransportType";

        const string OperationTimeoutConfigName = "OperationTimeout";

        string entityPath, sasKeyName, sasKey, sasToken, endpoint;
+        string authType;
+
+        public static class AuthenticationType


I'm not sure that this is really discoverable tucked in here as a nested class. How would we expect callers to know to look inside the connection string builder to find the authentication type constants?

Feels a bit redundant to make it its separate class, especially with just one value right now. What do you think about just using a public const like "AuthenticationManagedIdentity"?

In reply to: 292684143 [](ancestors = 292684143)

From the perspective of a consumer of the API that does not know about this particular magic string nor the implementation, can you describe the scenario for how you would expect them to discover this value? I'm not clear on how a consumer would know to explore the nested class or the ConnectionStringBuilder members to discover it.

The reason that I suggest a dedicated type is that if I look in intellisense and see a class called AuthenticationType or similar and explore the members, I'm more likely to be able to discover it. Frankly, that is the reason that I'm pushing for a true enum here. The type name of the enum in the message signature clearly tells callers where to look for the valid set of values with no intuitive leap.

In code terms:

public void DoTheThing(string magicValueThatYouMustKnow) { ... }

versus

public void DoTheThing(MagicValue magicValue) { ... }

In the latter case, I can clearly understand what you're asking of me. In the former, I have to figure it out. Even when the associated doc comment calls it out, it is easy to miss and not nearly as guided an experience.

To be honest, I'm not too concerned about the discovery of this value to external users of this API. It's very unlikely that the user would need to manually do anything with this value, since the connection string is just a generated value that they need to copy+paste from elsewhere and feed as an input to this API. I think having this value being discoverable/convenient to other developers is probably more important.

In reply to: 293031840 [](ancestors = 293031840)

Anything that is public is users concern. Otherwise, it should be internal.
Also, "users" are developers as well 🙂

jsquire · 2019-06-11T22:30:45Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs

@@ -260,6 +302,11 @@ public string GetNamespaceConnectionString()
                connectionStringBuilder.Append($"{OperationTimeoutConfigName}{KeyValueSeparator}{this.OperationTimeout}{KeyValuePairDelimiter}");
            }

+            if (this.Authentication != null)


This pattern of building negates a lot of the advantage of using a StringBuilder. Unless there's a compiler optimization that I'm not aware of, you're essentially allocating another string builder to do the format for the bling string and feeding it's output into your string builder.

You may want to consider just doing AppendFormat or a series of Append directly to your builder.

SeanFeldman · 2019-06-12T20:17:50Z

@jsquire did you see #6393 (comment)?

The change is really forcing abbreviations rather than have clean names.
9579185#diff-ba61021d942752ddbc21fb68743f2258R80

vs

9579185#diff-e62dc6ec981989d39662fe01a49d524dR882

jsquire · 2019-06-12T20:25:50Z

@jsquire did you see #6393 (comment)?

The change is really forcing abbreviations rather than have clean names.
9579185#diff-ba61021d942752ddbc21fb68743f2258R80

vs

9579185#diff-e62dc6ec981989d39662fe01a49d524dR882

I did, and I'm of the same opinion. I'm hopeful that there will be a follow-up set of changes that address some of the feedback around discoverability and naming, which has also been discussed offline. That said, I'm trying to be sensitive to a deliverable date for internal testing of the RBAC feature set by the service teams, so I didn't want to block this PR. Were this intended for public release, I would have taken a different tact.

I've opened #6548 to track this.

jsquire · 2019-06-13T14:07:49Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TokenProvider.cs

+        /// <param name="authCallback">The user defined authentication delegate to provide access token.</param>
+        /// <param name="authority">URL of the Azure Active Directory instance to issue token.</param>
+        /// <param name="state">Custom parameters that may be passed into the authentication delegate.</param>
+        /// <returns>The <see cref="Microsoft.ServiceBus.TokenProvider" /> for returning Json web token.</returns>
        public static TokenProvider CreateAadTokenProvider(


I'd suggest we consider avoiding the abbreviation here and go with something like CreateAzureActiveDirectoryTokenProvider for clarity. This class and method are public and, therefore, discoverable/usable by consumers directly.

Looking at #6393 (comment) wouldn't this change be handled in a separate (follow up) PR, @jsquire?

Yes, but I've referred back to this one in the issue that I opened to track, since there are several comments that were not addressed due to time constraints. I wanted to take one last pass and keep thoughts consolidated so that they're all in one place for future consideration.

Ok. Would expect to see these comments in the new PR with a reference to this PR as a starting point. Otherwise it looks like a call to action on the PR that has been approved 🙂

jsquire · 2019-06-13T14:11:07Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/ServiceBusConnectionStringBuilderTests.cs

@@ -191,5 +191,15 @@ public async Task NonAmqpUriSchemesShouldWorkAsExpected()
                await receiver.CloseAsync();
            });
        }
+
+        [Fact]
+        public void InvalidAadConnectionStringTest()


I'd suggest we consider avoiding the abbreviation here and go with something like InvalidAzureActiveDirectoryConnectionStringFailsValidation for clarity. The more explicit the name, the more helpful it is to troubleshoot from the build dashboard for those without deep familiarity with the code.

jsquire · 2019-06-13T14:12:03Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/ServiceBusConnectionStringBuilderTests.cs

+        [Fact]
+        public void InvalidAadConnectionStringTest()
+        {
+            var connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";


Can we please move these to [InlineData] and try to keep the test scope focused? These are really different cases for the same behavior, not a related set of assertions to validate a single operation.

jsquire · 2019-06-13T14:13:04Z

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs

                }
            });
        }
+
+        [Fact]
+        public async Task AadAuthCallback()


I'd suggest we consider avoiding the abbreviation here and try to find a name that more clearly expresses the semantics of what scenario the test is attempting to validate.

jsquire · 2019-06-13T14:13:55Z

sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnectionStringBuilder.cs

+            {
+                if (!string.IsNullOrWhiteSpace(this.SasKeyName))
+                {
+                    throw Fx.Exception.Argument("Authentication, SharedAccessKeyName", Resources.ArgumentInvalidCombination.FormatForUser("Authentication, SharedAccessKeyName"));


nit: I'd suggest using nameof rather than hardcoding.

ServiceBus SDK: Rbac support

885eee2

nemakam reviewed May 22, 2019

View reviewed changes

sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TokenProvider.cs Show resolved Hide resolved

nemakam reviewed May 22, 2019

View reviewed changes

sdk/servicebus/Microsoft.Azure.ServiceBus/tests/TokenProviderTests.cs Outdated Show resolved Hide resolved

jsquire reviewed May 24, 2019

View reviewed changes

jsquire requested a review from AlexGhiondea May 28, 2019 15:21

CR comments

2b52261

nemakam reviewed May 31, 2019

View reviewed changes

jsquire added breaking-change Client This issue points to a problem in the data-plane of the library. Service Bus labels Jun 1, 2019

jsquire assigned jsquire and bainian12345 and unassigned jsquire Jun 1, 2019

CR comments 2

74f1416

Use string instead of enum for AuthenticationType

f8d3f5f

SeanFeldman reviewed Jun 5, 2019

View reviewed changes

Connectionstring support for management client

ecdb825

jsquire approved these changes Jun 11, 2019

View reviewed changes

SeanFeldman mentioned this pull request Jun 12, 2019

Allow clients to report if they own or share the underlying connection string (Part 2) #6037

Merged

bailiu added 2 commits June 12, 2019 10:50

PR comments

47c385e

Fix API Approval Tests

9579185

jsquire merged commit 59946c8 into Azure:master Jun 12, 2019

jsquire mentioned this pull request Jun 12, 2019

Service Bus RBAC: Revisit Naming and Discoverability of Public API Changes #6548

Closed

jsquire reviewed Jun 13, 2019

View reviewed changes

		@@ -10,7 +10,7 @@ namespace Microsoft.Azure.ServiceBus.Primitives
		/// <summary>
		/// Represents the Azure Active Directory token provider for Azure Managed Service Identity integration.

	String connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";
	var connectionString = "Endpoint=sb://test.servicebus.windows.net/;authentication=Managed Identity;SHAREDACCESSKEYNAME=val";

ServiceBus SDK: Rbac support #6393

ServiceBus SDK: Rbac support #6393

Conversation

bainian12345 commented May 22, 2019

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam May 22, 2019 • edited Loading

Choose a reason for hiding this comment

nemakam commented May 22, 2019 • edited Loading

jsquire commented May 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemakam commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bainian12345 commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsquire Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeanFeldman commented Jun 12, 2019

jsquire commented Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeanFeldman Jun 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam May 22, 2019 •

edited

Loading

nemakam commented May 22, 2019 •

edited

Loading

jsquire Jun 12, 2019 •

edited

Loading

jsquire commented Jun 12, 2019 •

edited

Loading

SeanFeldman Jun 13, 2019 •

edited

Loading