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

Adds support to adoCodespacesAuth.tenantID extension setting for conf… #15

Merged
merged 5 commits into from
Feb 12, 2024

Conversation

liguori
Copy link
Member

@liguori liguori commented Feb 10, 2024

Provides the 'adoCodespacesAuth.tenantID' extension setting to configure the Entra ID tenant to sign in to.

Address #14

@markphip
Copy link
Contributor

@ronakj can you review this?

src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
@ronakj
Copy link
Contributor

ronakj commented Feb 12, 2024

Mostly looks good functionally. Just some minor code changes and we should be good to go.

Can you also update the package.json version (probably a minor bump to 1.2.0) and add a line in the README.md to explain this configuration? Thanks!

@liguori liguori requested a review from ronakj February 12, 2024 14:26
@ronakj ronakj merged commit 11f7c5b into microsoft:main Feb 12, 2024
2 checks passed
@ronakj
Copy link
Contributor

ronakj commented Feb 12, 2024

Thanks @liguori. I will do a quick test from my side and then release it.

@ronakj
Copy link
Contributor

ronakj commented Feb 12, 2024

When testing this, it seems to not work when requesting token. It authenticates fine initially, but when you request a token with the command ado-auth-helper get-access-token or with git fetch it just hangs.

There are two reasons from what I can see. The PR breaks the default assumption that getAccessToken when called without scopes uses the ADO scope. So when called from socket server handler it doesn't get the default scope now. Another reason might be because we need to pass the tenant in the calls to getAccessToken function when it's called from socket server handler.

@liguori can you create a followup PR. Maybe move the logic for getting tenant from configuration to the getAccessToken function and also fix the default scope assumption.

You can test it by updating the version number, running yarn vsce:package, download the generated vsix bundle and then open a codespace, search for "vsix" and install the local download

image

Then signout, add the tenant id config and then signin to microsoft account. Run ado-auth-helper get-access-token to test if it works.

@ronakj
Copy link
Contributor

ronakj commented Feb 12, 2024

It's late for me today so I am gonna go, but I can also take a stab at fixing this tomorrow if you want.

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

Successfully merging this pull request may close these issues.

3 participants