Test: Auth provider proposed API

[RMacfarlane]

Testing [https://github.com//issues/88309](Auth provider proposed api}

• anyOS @sbatten
• anyOS @eamodio

Complexity: 4

---

The proposal for the authentication provider API currently looks like:

vscode/src/vs/vscode.proposed.d.ts

Lines 21 to 60 in 8d36d09

	export interface Session {
	id: string;
	accessToken: string;
	displayName: string;
	scopes: string[]
	}
	export interface AuthenticationProvider {
	/**
	* Used as an identifier for extensions trying to work with a particular
	* provider: 'Microsoft', 'GitHub', etc. id must be unique, registering
	* another provider with the same id will fail.
	*/
	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(scopes: string[]): 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>;
	export const providers: ReadonlyArray<AuthenticationProvider>;
	}

This API enables two different use cases: I can write and register my own auth provider, or I can access existing auth providers and leverage them. The first part of this has been fairly well tested by work I've done for settings sync and for creating a GitHub provider. Please review this part of the API and give any feedback you have about it, but focus on testing using authentication.providers and authentication.onDid[Un]RegisterAuthenticationProvider events. A provider with id "MSA" which supplies the auth for settings sync should be available.
Please test that

• You can access this provider and call methods on it from an extension
• When you call getSessions or login, a dialog asking for user consent is shown

[sbatten]

I tested the methods and API seemed to work well. One thing I thought of that may not be a concern is the onDidChangeSessions event. If an extension decides to listen to this event, I feel their next call will be to immediately call getSessions which will trigger a notification to the user. If the extension really does need the access token, then this is fair, but potentially the extension might see that the change does not warrant any action and would rather not bug the user with a notification. If this is a realistic concern, perhaps we should have a lighter sessions request to get all but the access token, or perhaps we should only prompt the user when getting the access token.

[eamodio]

Looks good overall!

Although, I agree with @sbatten that getSessions shouldn't be the trigger for the prompt -- instead it should be on access to the accessToken (maybe id as well if it could contain sensitive info -- not sure), since an extension might very well be looking for existing sessions with certain accounts/scopes before deciding to use one.

Do you think it is worth onDidRegisterAuthenticationProvider & onDidUnregisterAuthenticationProvider vs just a single onDidChangeAuthenticationProviders?

Does login need to take scopes? Should it be optional? I also think it might be nice to have login take some sort of name/id (something the caller controls), so you can more easily look for your own sessions (if needed).

Should the provider really have the logout method, vs the Session itself? Should any extension be able to log out an existing session?

I would also suggest that displayName on session be renamed to account or accountName or username.

IMO, getSessions should return readonly sessions:

getSessions(): Promise<ReadonlyArray<Readonly<Session>>>;

[RMacfarlane]

Thanks for the great feedback!

For

Does login need to take scopes? Should it be optional?

I'm starting to think login may need to take an options parameter that is defined as any and further described by the auth provider extension itself. For the Auth extension to be used by the Live Share extension, it needs to pass in a different set of scopes than what we do for settings sync. But it also needs to set a different clientId and tenant parameter as well.

The rest of the suggestions sound good to me, I'll start creating some issue for them.