microsoft / vscode-azureresourcegroups

VS Code extension for managing Azure resources.
https://marketplace.visualstudio.com/items?itemName=ms-azuretools.vscode-azureresourcegroups
MIT License
52 stars 28 forks source link

Use the new shared authentication provider package #707

Closed bwateratmsft closed 1 year ago

bwateratmsft commented 1 year ago

This ended up being more substantial than I anticipated, but I got all the automated tests passing, and a very quick glance at features seems to be working fine.

alexweininger commented 1 year ago

Just a note: I'm still having issues on sign-in when using an account that has subscriptions in a non-default tenant. I'll be working on fixing it next week.

nturinski commented 1 year ago

The subscriptions don't automatically refresh after filtering them.

bwateratmsft commented 1 year ago

The subscriptions don't automatically refresh after filtering them.

Fixed!

alexweininger commented 1 year ago

Being signed in with multiple Microsoft accounts at the same time seems to really throw the extension into a frenzy. It was having errors when I had 3 accounts. Signed out of 1, it loaded subs from one of the accounts. Then signed out of all of them and now it shows subs for a different account, but I'm completely signed out.

image
bwateratmsft commented 1 year ago

That seems like it might be an issue with the auth provider...

bwateratmsft commented 1 year ago

In fact, I wonder if it's this: https://github.com/microsoft/vscode/issues/179202

nturinski commented 1 year ago

Might consider fixing this issue as well. https://github.com/microsoft/vscode-azureappservice/issues/2464

Whenever we do a request, we should probably verify with MS Auth provider whether or not the token we have is still valid.

bwateratmsft commented 1 year ago

Might consider fixing this issue as well. microsoft/vscode-azureappservice#2464

Whenever we do a request, we should probably verify with MS Auth provider whether or not the token we have is still valid.

The authentication provider guarantees that it won't issue expired tokens and AFAIK we don't cache them anywhere (i.e., getToken returns the result from a call to getSession), so I don't think having expired creds is even possible.

nturinski commented 1 year ago

It happens whenever you leave the VS Code for the Web window idle for like am hour or so. Your next request will have an expired token and you have to refresh the webpage to get the new one (but not required to sign in again). I don't think this should be unique to the web, it's just the only place we actually it's live right now though.

But that was an old implementation and maybe this current shared one wouldn't have the same issue.

bwateratmsft commented 1 year ago

Looking closer, it looks like everything is substantially the same (in terms of what caching is happening) as the old implementation, except for where we have subscriptionResultsTask as a cached variable. But the implementation of the TokenCredential is still via getSession.

I don't think this will fix https://github.com/microsoft/vscode-azureappservice/issues/2464 but I also don't think it will be any worse.