Skip to content
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.

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

AAD login #607

Open
ChoOo7 opened this issue Nov 29, 2021 · 54 comments
Open

AAD login #607

ChoOo7 opened this issue Nov 29, 2021 · 54 comments
Labels

Comments

@ChoOo7
Copy link

ChoOo7 commented Nov 29, 2021

Hello

Can we have an Azure Active directory login to have access to our services bus ?

We don't want to have any password / keys on our computers

Thanks

@ErikMogensen
Copy link
Collaborator

@ChoOo7, that's something that's on the roadmap. However, currently 95% of the code Service Bus Explorer is using to communicate with queues and topics is using the oldest Service Bus SDK. The remaining 5% is using the latest Service Bus SDK. For several reasons we want to migrate everything to the latest SDK and that has a higher priority for me.

However, you or anyone else is most welcome to come up with a design suggestion for AAD login. I recommend starting with that before creating a PR since this a complicated issue.

@Rookian
Copy link

Rookian commented Jan 10, 2022

I think the easiest solution is to use AzureCliCredential or InteractiveBrowserCredential when authenticating against Azure Service Bus. The new SDK already provides an option to use TokenCredential.

 public ServiceBusClient(string fullyQualifiedNamespace, TokenCredential credential);

@SeanFeldman
Copy link
Collaborator

@Rookian

However, currently 95% of the code Service Bus Explorer is using to communicate with queues and topics is using the oldest Service Bus SDK.

@stale stale bot added the wontfix label Mar 13, 2022
@kensykora
Copy link

I really want this feature so we can stop using access keys!

@stale stale bot removed the wontfix label Mar 14, 2022
@jwerner77
Copy link

Azure AD auth will be a huge win for this tool! There is some very valuable functionality such as fix/resubmit from dead letter queue which are not available in the portal. However the only way for a team to use this is they all have to have connection string with the keys which is not secure.

@rangp
Copy link

rangp commented Apr 26, 2022

Is there any update on this?
I just looked at the code and it seems as if the old sdk has been removed. With the new sdk it should be fairly simple to integrate this, since both the ServiceBusClient and ServiceBusAdministrationClient support passing a TokenCredential.

I can see two ways of integrating this:

  1. Add a ui feature that allows one to specify whether to authenticate via active directory or not. Something like this. And if specified, a flag is passed that allows the ServiceBusClient and ServiceBusAdministrationClient to be constructed with InteractiveBrowserCredentials. This would then open a browser window where one can sign in and the rest should be handled by the sdk out of the box
    mockup

  2. add a magic connecting string. This is how the sqlclient handles it too. One could for example specify a connection string like this

Endpoint=sb://....servicebus.windows.net/;Authentication=Active Directory Interactive

Then provide a factory for creating a ServiceBusClient and ServiceBusAdministrationClient that can interpret the connection string and constructs the client with InteractiveBrowserCredentials. Use the factory instead of constructing the clients directly. This solution is not as elegant as the first, but I think it should be very simple to implement, as no ui changes are needed and the code only needs to be changed in a handful of places.

@moverholt-afi
Copy link

This would be a great benefit so that we can leverage the RBAC that we have set up on our ASB resources. Is there any update on projected release?

@stale stale bot added the wontfix label Aug 10, 2022
Repository owner deleted a comment from stale bot Aug 23, 2022
@diligent176
Copy link

This would be great to have as we move over to RBAC model for service bus.

@bjoljo
Copy link

bjoljo commented Oct 12, 2022

Azure AD login would be very usefull, please let us have the option to move away from connection strings with secret keys.

@ErikMogensen ErikMogensen changed the title Windows AAD login AAD login Oct 17, 2022
@ErikMogensen ErikMogensen pinned this issue Oct 17, 2022
@malsey
Copy link

malsey commented Dec 21, 2022

would be great to have this supported

@brswanson
Copy link

Would also really like to have AD auth for this 🙏

@madmalkav
Copy link

Would be incredibly useful to have this implemented.

@zyofeng
Copy link

zyofeng commented Feb 12, 2023

I would say this is an essential feature as our company moves to disable local authentication (SAS key).
Until then, the Azure portal provides an AAD integrated service bus explorer that one can use.

@mdanylyuk
Copy link

Hello,
Supporting AAD is a pretty important thing for the user who cannot use connection strings for security reasons.
Please add it ;)

@sovaska
Copy link

sovaska commented Sep 7, 2023

Pretty much stopped using Service Bus Explorer since Managed Identity is best practice for accessing Service Bus. Would love to still use it, but without Managed Identity support it’s not possible.

@jooooel
Copy link

jooooel commented Oct 19, 2023

Hi @ErikMogensen!

What is your estimated level of effort for someone who hasn't been developing in the code base? If it's possible now (I believe you've migrated from the old SDK?) then I wouldn't mind taking a stab at this. But it would of course depend on the complexity of the issue...

@SeanFeldman
Copy link
Collaborator

@jooooel, unfortunately, the code hasn’t been migrated yet.

@mennolaan
Copy link

Also looking forward to be able to use managed identities. Many companies now move away from the sas tokens and only allow managed identity, thus removing the possibility to servicebus explorer.

@ErikMogensen
Copy link
Collaborator

Sounds good. There is a guide at https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/servicebus/Azure.Messaging.ServiceBus/MigrationGuide_WindowsAzureServiceBus.md

Any feature that is missing in Azure.Messaging.ServiceBus and not documented as missing is interesting so please put a comment here. @paolosalvatori can notify the Service Bus Product Group about it and there is a chance that they may implement it.

There is a large PR for Event Grid, #737, that affects MainForm.cs and perhaps other central parts so please keep an eye on that one to avoid merge issues.

@SeanFeldman
Copy link
Collaborator

@Merlijnv, everything that Erik said.
I can help with the ASB, assuming I have availability to do so. If you want to start a PR and ping me with some questions as you progress, we can try that.

I will try and expand the servicebushelper2 with all the methods that can be used in the new sdk.

`ServiceBusHelper2 is just a helper. Don't worry about it too much. If needs to go, it can be removed as well.

Regarding EventGrid PR - I'm looking after that one. We don't need to worry about it until it's merged into master and then whatever you'll have at that point would need to be rebased. But let's not worry about it ahead of the time.

@0xced
Copy link
Contributor

0xced commented Nov 8, 2023

We decided that cross platform will have to wait at least until WindowsAzure.ServiceBus has been removed.

Are you implying there's a plan to rewrite ServiceBusExplorer with Avalonia UI or Uno Platform? Because cross platform WinForms is definitely not happening.

Edit: Nevermind, I skimmed over this issue a bit too fast. There's already an issue dedicated to cross platform support: #286

@Merlijnv
Copy link

Merlijnv commented Nov 9, 2023

@SeanFeldman Do you maybe have some time maybe next monday at 10am (that's 18.00 my time)? I need some quick walkthrough as to which code will be hit first and how everything gets called.

@Merlijnv
Copy link

Merlijnv commented Nov 9, 2023

@paolosalvatori so far it seems like getting queue message counts is missing or I can't find it. QueueDescription has been replaced with QueueProperties but that does not include MessageCountDetails.

Update: Nevermind QueueRuntimeProperties does include the message counts

@SeanFeldman
Copy link
Collaborator

@SeanFeldman Do you maybe have some time maybe next monday at 10am (that's 18.00 my time)? I need some quick walkthrough as to which code will be hit first and how everything gets called.

Sorry, can't make it in the AM hours. You might have more luck with @paolosalvatori or @ErikMogensen as they are in EU time zones. Queue-related forms and controls might be the simplest path. Topics/subscriptions are a duplication. Something to be aware of, BTW.

@Merlijnv
Copy link

@SeanFeldman @paolosalvatori @ErikMogensen is the retry helper still needed when using servicebusadministrationclient?

@SeanFeldman
Copy link
Collaborator

ServiceBusAdministrationClient still can throw exceptions that would need to be logged and some retried.

@Merlijnv, I suggest you open a PR (even if it's only at a starting point) and have this kind of Q&A on the PR rather than the issue. The issue is more suited to the problem description, status updates, and information not directly implemented. PR is more suited for the discussions and decision-making related to the changes in that PR. Would you agree?

@ancientpanda
Copy link

Any update on this? Entra/AAD support would make this tooling very ideal over having to use less secure SAS tokens.

@Merlijnv
Copy link

Nope currently busy with other more important things so this is on a hold for me but feel free to continue the work.

@maztan

This comment was marked as off-topic.

@ErikMogensen

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@ancientpanda

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@SeanFeldman

This comment was marked as off-topic.

@maztan

This comment was marked as off-topic.

@sean-feldman-sh

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests