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

Microsoft Auth Provider should support overriding client id and tenant id #115626

Open
TylerLeonhardt opened this issue Feb 2, 2021 · 7 comments
Assignees
Labels
api-proposal feature-request Request for new features or functionality microsoft-authentication Issues with the Microsoft Authentication extension
Milestone

Comments

@TylerLeonhardt
Copy link
Member

The Microsoft Auth Provider uses a specific AAD application with client id hardcoded here:

const clientId = 'aebc6443-996d-45c2-90f0-388ff96faa56';
const tenant = 'organizations';

However, this application only has access to a handful of scopes, and to add allowed scopes to this client id is a manual process (which for an external extension author means opening an issue here and then having one of us add that scope to the allowed scopes for the application)

As an extension author, I should easily be able to create my own AAD application (in the Azure Portal for example) and use that client id instead of the one vscode uses so that I can have control over the scopes I care about and, if this exists, I can get telemetry when my client id is used.

Since we have abstracted auth providers, I think it's fitting to be able to pass additional auth provider specific options down to an auth provider. For example, the Microsoft auth provider would take a client id and tenant that would replace the hard coded string above.

Proposal:

        /**
	 * Options to be used when getting an [AuthenticationSession](#AuthenticationSession) from an [AuthenticationProvider](#AuthenticationProvider).
	 */
	export interface AuthenticationGetSessionOptions {
		/**
		 * Whether login should be performed if there is no matching session.
		 *
		 * If true, a modal dialog will be shown asking the user to sign in. If false, a numbered badge will be shown
		 * on the accounts activity bar icon. An entry for the extension will be added under the menu to sign in. This
		 * allows quietly prompting the user to sign in.
		 *
		 * Defaults to false.
		 */
		createIfNone?: boolean;

		/**
		 * Whether the existing user session preference should be cleared.
		 *
		 * For authentication providers that support being signed into multiple accounts at once, the user will be
		 * prompted to select an account to use when [getSession](#authentication.getSession) is called. This preference
		 * is remembered until [getSession](#authentication.getSession) is called with this flag.
		 *
		 * Defaults to false.
		 */
		clearSessionPreference?: boolean;

		/*************/
		/***  NEW  ***/
		/*************/
                /**
                 * Provider specific options for getting this session (i.e. client id, tenant)
                 */
		providerOptions?: { [key: string]: any; }
	}

The Auth Provider would then need to be responsible for deciding if it already has created a session with these options or if it needs to create a new session based on these options.

@TylerLeonhardt TylerLeonhardt added this to the February 2021 milestone Feb 2, 2021
@TylerLeonhardt TylerLeonhardt added api-proposal authentication Issues with the Authentication platform labels Feb 2, 2021
@RMacfarlane RMacfarlane removed this from the February 2021 milestone Feb 2, 2021
@TylerLeonhardt TylerLeonhardt added this to the February 2021 milestone Feb 2, 2021
@TylerLeonhardt TylerLeonhardt self-assigned this Feb 2, 2021
@RMacfarlane RMacfarlane modified the milestones: February 2021, March 2021 Feb 22, 2021
@ankitbko
Copy link
Member

So will current auth provider API also allow me to authenticate via Azure DevOps app? The AzDO app requires a different app registration process as described here - https://docs.microsoft.com/en-us/azure/devops/integrate/get-started/authentication/oauth?view=azure-devops#register-your-app

@TylerLeonhardt TylerLeonhardt modified the milestones: March 2021, April 2021 Mar 23, 2021
@TylerLeonhardt
Copy link
Member Author

@ankitbko ideally, yes... but it doesn't today, as you probably know. We will be talking with the AAD team to make sure we do this correctly.

@RMacfarlane RMacfarlane modified the milestones: April 2021, Backlog Apr 27, 2021
@RMacfarlane RMacfarlane added the feature-request Request for new features or functionality label May 7, 2021
@RMacfarlane RMacfarlane removed their assignment May 12, 2021
@TylerLeonhardt
Copy link
Member Author

small update in this space... I recently added a sample auth provider here:
https://github.com/microsoft/vscode-extension-samples/tree/main/authenticationprovider-sample

using Azure DevOps PATs has the sample. For anyone looking to interact with Azure DevOps APIs, give this sample a look and use it until we better understand the work involved to properly support AzDO in the inbox Microsoft Auth Provider.

@roeap
Copy link

roeap commented Oct 23, 2022

@TylerLeonhardt - Trying to implement my own version of the provided plugin to get aad login to work with our client id / tenant I stumbled across these lines in the codebase:

private getClientId(scopes: string[]) {
return scopes.reduce<string | undefined>((prev, current) => {
if (current.startsWith('VSCODE_CLIENT_ID:')) {
return current.split('VSCODE_CLIENT_ID:')[1];
}
return prev;
}, undefined) ?? DEFAULT_CLIENT_ID;
}

From there it seems one can in fact provide custom client and tenant ids by passing them in via the requested scopes. Giving it a quick try it seemed to work as long as the redirect url is properly configured on the application. Is this an official (yet AFAIK undocumented) feature, that can be used?

@TylerLeonhardt
Copy link
Member Author

@roeap that will be there for the foreseeable future... but I specifically haven't closed this issue as it could change in the future.

@dmgrok
Copy link

dmgrok commented Feb 15, 2024

Any news on this, please? I wanted to use a custom client_id for an OpenID authorization workflow as well

@TylerLeonhardt
Copy link
Member Author

If you find yourself overriding the client id with the VSCODE_CLIENT_ID:<id> scope, just keep in mind that there are some redirect urls you need:

Public Client (if you want to support VS Code for the desktop):

  • ms-appx-web://Microsoft.AAD.BrokerPlugin/<your-client-id> - to support broker flows which are currently supported on Windows 64bit
  • http://localhost - to support non-broker flows on all other platforms
  • https://vscode.dev/redirect - to support the old auth flow which should disappear in the next few months

SPA (if you want to support working in VS Code for the Web aka https://vscode.dev):

  • https://vscode.dev/msal - stable
  • https://insiders.vscode.dev/msal - insiders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-proposal feature-request Request for new features or functionality microsoft-authentication Issues with the Microsoft Authentication extension
Projects
None yet
Development

No branches or pull requests

5 participants