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

Authentication Provider API #88309

Closed
RMacfarlane opened this issue Jan 8, 2020 · 29 comments
Closed

Authentication Provider API #88309

RMacfarlane opened this issue Jan 8, 2020 · 29 comments
Assignees
Labels
api-finalization authentication Issues with the Authentication platform feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan settings-sync
Milestone

Comments

@RMacfarlane
Copy link
Contributor

RMacfarlane commented Jan 8, 2020

Problem

There are currently some extensions that attempt to provide authentication abilities that can be reused by other extensions. (An example being the Azure Account extension). Now that we've begun working on login for settings sync, it's worth revisiting if authentication should be a first-class concept in VS Code. By exposing an API to contribute an authentication flow

  • the core of VSCode can potentially leverage authentication
  • other extensions can leverage authentication
  • UI for account management could be centralized

Proposal

I propose introducing a concept of an "AuthenticationProvider". Such a provider implements methods for logging in and logging out of a specified account, and exposes a list of accounts that are currently available with an event listener for changes to these. This abstracts away refreshing tokens from consumers - the AuthenticationProvider extension can manage refreshing in the background and fire an event when the accessToken has been changed.

export interface Account {
	readonly id: string;
	readonly accessToken: string;
	readonly displayName: string;
}

export interface AuthenticationProvider {
	readonly id: string; // perhaps "type"? Would be something like "GitHub", "MSA", etc.
	readonly displayName: string;

	accounts: ReadonlyArray<Account>;
	onDidChangeAccounts: Event<ReadonlyArray<Account>>;

	login(): Promise<Account>;
	logout(accountId: string): Promise<void>;
}

export namespace authentication {
	export function registerAuthenticationProvider(provider: AuthenticationProvider): Disposable;
	export const authenticationProviders: ReadonlyArray<AuthenticationProvider>;
}

Consumers would need to know the id of the provider they're looking for. For example, the settings sync code would look for an "MSA" provider since this is what the setting sync backend currently needs.

Since the authentication provider extension would be activated in each VS Code window, the extension would be responsible for synchronizing state across instances. By default, such extensions would have ["ui", "workspace"] extensionKind, so that they can store and read credentials on the local machine in both the desktop and web case.

@RMacfarlane
Copy link
Contributor Author

Based on feedback from the API call, I've some changes to the API:

export interface Session {
	id: string;
	accessToken: string;
	displayName: string;
}

export interface AuthenticationProvider {
	readonly id: string;
	readonly displayName: string;
	readonly onDidChangeSessions: Event<void>;

	/**
	 * Returns an array of current sessions.
	 */
	getSessions(): Promise<ReadonlyArray<Session>>;

	/**
	 * Prompts a user to login.
	 */
	login(): Promise<Session>;
	logout(sessionId: string): Promise<void>;
}

export namespace authentication {
	export function registerAuthenticationProvider(provider: AuthenticationProvider): Disposable;

	/**
	 * Fires with the provider id that was registered or unregistered.
	 */
	export const onDidRegisterAuthenticationProvider: Event<string>;
	export const onDidUnregisterAuthenticationProvider: Event<string>;

	/**
	 * Fires with the provider id that changed sessions.
	 */
	export const onDidChangeSessions: Event<string>;
	export function login(providerId: string): Promise<Session>;
	export function logout(providerId: string, accountId: string): Promise<void>;
	export function getSessions(providerId: string): Promise<ReadonlyArray<Session> | undefined>;
}

Account has been replaced with Session. There are now events register and unregister events that can be listened to, as well as more explicit calls that extensions can make to perform an action with a specific provider, instead of searching through an array of providers.

We had discussed combining login and getSessions, but after thinking about it more, I think these should remain separate. When consuming this API with settings sync, I want to be able to check if there are existing sessions without popping up a browser for sign in. However, with the change to expose getSessions instead of the authenticationProviders array, it should be easy to add a consent notification that indicates an extension is trying to access token information from another extension.

We also discussed moving the authentication provider display name out to a contribution point - I think this is reasonable, I just haven't done this yet.

One open question is about additional information that might be needed for login and on the session itself. For OAuth, the clientId and scopes are typically passed as parameters. Should these be added as parameters to login? Or should login take an unstructured, optional parameter bag that the provider would determine? Likewise, for sessions themselves, for OAuth, consumers would likely want to know what scopes a given session has to see if it can be used.

@jounii
Copy link

jounii commented Feb 14, 2020

Sounds very interesting. I have use case this would come handy.

Right now it's just custom development tools implementation, a bit hacky though. Process is to get access token written down to a temporary file in the project's directory for running integration tests against system requiring OAuth login through browser (and MFA) and there's no way adding more permanent application authentication. It's done by running a console script activating OAuth process, then giving url for user to visit in browser and copy the url back to the script waiting for input in console. Then the access token is written to a json file which can be then consumed by other scripts. Was considering even to have headless chrome to avoid the browser step.

Now if the VSCode first class standard OAuth provider (would cover many login cases) could be configured per project basis (.vscode/extensions.json, .vscode/authentication.json?) to write the returned/refreshed access token to a specified file in active project, and have easy way to select specific OAuth target account to login manually...

@bwateratmsft
Copy link
Contributor

bwateratmsft commented Feb 27, 2020

The Docker extension would get a lot of mileage out of some standardization of OAuth flows: microsoft/vscode-docker#869.

For instance, if VSCode could take care of password retrieval and storage, and facilitate browser-based logins (like what the Azure Account extension has to do), that would help a lot.

@KamasamaK
Copy link

This is missing the authentication label.

@RMacfarlane RMacfarlane added the authentication Issues with the Authentication platform label Mar 9, 2020
@RMacfarlane RMacfarlane modified the milestones: March 2020, April 2020 Apr 2, 2020
@RMacfarlane RMacfarlane modified the milestones: April 2020, May 2020 Apr 27, 2020
@OkGoDoIt
Copy link

OkGoDoIt commented May 5, 2020

So would all extensions loading into VSCode be able to read all accessTokens for all of my logged-in authentication providers? The proposed API surface implies so. That feels like a potential security hole. For example, I install an extension to do something but it turns out that it reads my github accessToken and sends that to some server or does something else nefarious with my account without ever having to prompt me for permission.

@KamasamaK
Copy link

@OkGoDoIt There is a feature called "Manage Trusted Extensions" that should address that. There's a brief description in #93811. Regarding the API, check

/**
* Get existing authentication sessions. Rejects if a provider with providerId is not
* registered, or if the user does not consent to sharing authentication information with
* the extension.
* @param providerId The id of the provider to use
* @param scopes A list of scopes representing the permissions requested. These are dependent on the authentication
* provider
*/
export function getSessions(providerId: string, scopes: string[]): Thenable<readonly AuthenticationSession[]>;

which indicates that a user must consent to sharing authentication information with an extension in order for it to be able to retrieve the session.

@neilenns
Copy link
Contributor

When the GitHub Pull Request & Issues extension switched to using this new Authentication API it lost the support for enterprise GitHub deployments. I assume that's because the above API is missing something fundamental for that specific style of access (API key based instead of username/password based?)

What are the plans for updating the API to add in what was missing for GitHub enterprise auth? I assume other extensions will need similar capabilities as well.

@jasonwilliams
Copy link
Contributor

When the GitHub Pull Request & Issues extension switched to using this new Authentication API it lost the support for enterprise GitHub deployments. I assume that's because the above API is missing something fundamental for that specific style of access (API key based instead of username/password based?)

What are the plans for updating the API to add in what was missing for GitHub enterprise auth? I assume other extensions will need similar capabilities as well.

Issue in question:
microsoft/vscode-pull-request-github#1793

This has broken GithubPullRequests for everyone using enterprise

@RMacfarlane RMacfarlane removed this from the May 2020 milestone Jun 1, 2020
@RMacfarlane
Copy link
Contributor Author

Do Microsoft-published extensions get a pass on the above statement?

Not completely, we have a very small list of extensions that are opted into being able to use the proposed API. This helps us iterate on and test API, but is also a source of occasional pain as we've sometimes unintentionally broken these extensions.

@Domiii
Copy link

Domiii commented Dec 9, 2020

@Domiii I can see some login and logout related code in the proposed API

https://github.com/microsoft/vscode/blob/master/src/vs/vscode.proposed.d.ts#L99
https://github.com/microsoft/vscode/blob/master/src/vs/vscode.proposed.d.ts#L145

Yes, it was proposed, and (iirc, also implemented) in the experimental version. But as I said, it did not make it into the final version of the API, for some reason.

No logout here: https://code.visualstudio.com/api/references/vscode-api#AuthenticationSession

I'm bringing up this issue, particularly because it is hard to test authentication without an ability to logout.

@vmiheer
Copy link

vmiheer commented Dec 10, 2020

Sorry for possibly stupid question. But with this proposal can there be "SshAuthenticationProvider" which will store my ssh username + password (say in windows credential manager) and provide it to vscode-remote-ssh plugin?
(I know ssh keys is the way to go about that but I am dealing with stubborn cluster admins).

@ankitbko
Copy link
Member

ankitbko commented Dec 17, 2020

@RMacfarlane When trying to get token for Microsoft I get following error -

An error occurred while signing in:
AADSTS65002: Consent between first party application 'aebc6443-996d-45c2-90f0-388ff96faa56' and first party resource '00000003-0000-0000-c000-000000000000' must be configured via preauthorization - applications owned and operated by Microsoft must get approval from the API owner before requesting tokens for that API. Trace ID: 1c686e2e-9c9d-4e2e-a8bb-f821f439ae00 Correlation ID: a6b11720-bb61-455a-8904-34598e458e83 Timestamp: 2020-12-17 05:04:34Z

What do I need to do? Is there any way to change the applicationid?

@jtoming830
Copy link

I am facing the same issue as @ankitbko
To be honest, information about this functionality in documentation is poor - for example, there are no links to topics with microsoft/github scopes descriptions.

@akosyakov
Copy link
Contributor

akosyakov commented Feb 1, 2021

Will it be possible to contribute several auth provider for the same id? For instance my extension has an access to a generic oauth provider which can handle GitHub as well already instead of using the github authentication extension.

@TylerLeonhardt
Copy link
Member

@RMacfarlane When trying to get token for Microsoft I get following error -

An error occurred while signing in:
AADSTS65002: Consent between first party application 'aebc6443-996d-45c2-90f0-388ff96faa56' and first party resource '00000003-0000-0000-c000-000000000000' must be configured via preauthorization - applications owned and operated by Microsoft must get approval from the API owner before requesting tokens for that API. Trace ID: 1c686e2e-9c9d-4e2e-a8bb-f821f439ae00 Correlation ID: a6b11720-bb61-455a-8904-34598e458e83 Timestamp: 2020-12-17 05:04:34Z

What do I need to do? Is there any way to change the applicationid?

What scopes/API are you using here?

@ankitbko
Copy link
Member

ankitbko commented Feb 9, 2021

I am using vso.code_write, vso.threads_full and vso.work_write

@monil-patel
Copy link

@TylerLeonhardt I am also hitting the same issue when trying to hit ADO apis with specific scopes. Is this solution recommended for auth against ADO?

+1 to the question about logout, is there any other way to log out since its still in proposed-api?

@monil-patel
Copy link

monil-patel commented Feb 17, 2021

@Domiii in case you were still stuck on this, looks like the accounts (person icon) on the activity bar has a way to log out. It will show you a list of accounts used to log in and what extensions are using it.

image

@RMacfarlane
Copy link
Contributor Author

Will it be possible to contribute several auth provider for the same id?

No, each auth provider needs a unique id.

I'm finalizing the provider portion of the API which will allow others to create authentication providers:

	/**
	 * Options for creating an [AuthenticationProvider](#AuthentcationProvider).
	 */
	export interface AuthenticationProviderOptions {
		/**
		 * Whether it is possible to be signed into multiple accounts at once with this provider.
		 * If not specified, will default to false.
		*/
		readonly supportsMultipleAccounts?: boolean;
	}

	/**
* An [event](#Event) which fires when an [AuthenticationSession](#AuthenticationSession) is added, removed, or changed.
*/
	export interface AuthenticationProviderAuthenticationSessionsChangeEvent {
		/**
		 * The [AuthenticationSession](#AuthenticationSession)s of the [AuthenticationProvider](#AuthentiationProvider) that have been added.
		*/
		readonly added: ReadonlyArray<AuthenticationSession>;

		/**
		 * The [AuthenticationSession](#AuthenticationSession)s of the [AuthenticationProvider](#AuthentiationProvider) that have been removed.
		 */
		readonly removed: ReadonlyArray<AuthenticationSession>;

		/**
		 * The [AuthenticationSession](#AuthenticationSession)s of the [AuthenticationProvider](#AuthentiationProvider) that have been changed.
		 */
		readonly changed: ReadonlyArray<AuthenticationSession>;
	}

	/**
	 * A provider for performing authentication to a service.
	 */
	export interface AuthenticationProvider {
		/**
		 * An [event](#Event) which fires when the array of sessions has changed, or data
		 * within a session has changed.
		 */
		readonly onDidChangeSessions: Event<AuthenticationProviderAuthenticationSessionsChangeEvent>;

		/**
		 * Get a list of sessions.
		 * @param scopes An optional list of scopes. If provided, the sessions returned should match
		 * these permissions, otherwise all sessions should be returned.
		 * @returns A promise that resolves to an array of authentication sessions.
		 */
		// eslint-disable-next-line vscode-dts-provider-naming
		getSessions(scopes?: string[]): Thenable<ReadonlyArray<AuthenticationSession>>;

		/**
		 * Prompts a user to login.
		 * @param scopes A list of scopes, permissions, that the new session should be created with.
		 * @returns A promise that resolves to an authentication session.
		 */
		// eslint-disable-next-line vscode-dts-provider-naming
		createSession(scopes: string[]): Thenable<AuthenticationSession>;

		/**
		 * Removes the session corresponding to session id.
		 * @param sessionId The id of the session to remove.
		 */
		// eslint-disable-next-line vscode-dts-provider-naming
		removeSession(sessionId: string): Thenable<void>;
	}


	/**
	 * Namespace for authentication.
	 */
	export namespace authentication {
		...

		/**
		 * Register an authentication provider.
		 *
		 * There can only be one provider per id and an error is being thrown when an id
		 * has already been used by another provider. Ids are case-sensitive.
		 *
		 * @param id The unique identifier of the provider.
		 * @param label The human-readable name of the provider.
		 * @param provider The authentication provider provider.
		 * @params options Additional options for the provider.
		 * @return A [disposable](#Disposable) that unregisters this provider when being disposed.
		 */
		export function registerAuthenticationProvider(id: string, label: string, provider: AuthenticationProvider, options?: AuthenticationProviderOptions): Disposable;
	}

@ankitbko
Copy link
Member

ankitbko commented Feb 23, 2021

So what scopes does this supports for now? Will AZDO and Graph APIs be supported by Microsoft provider?

@RMacfarlane
Copy link
Contributor Author

This issue is just tracking the API, the implementation of the Microsoft provider is unchanged. Right now the provider supports getting an ARM token with "https://management.core.windows.net/.default" and "offline_access". This issue tracks opening up the provider to more scopes by allowing different client ids to be passed

@marcinjahn
Copy link

Does this API allow to create an extension that is able to fetch access tokens from AAD B2C with user delegation?

@Domiii
Copy link

Domiii commented Mar 6, 2021

@Domiii in case you were still stuck...

@monil-patel thank you for the info! I have not revisited the issue yet, but will very soon; so, your help is much appreciated :)

@github-actions github-actions bot locked and limited conversation to collaborators Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization authentication Issues with the Authentication platform feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan settings-sync
Projects
None yet
Development

No branches or pull requests