Skip to content

Commit

Permalink
Log auth stuff around declaredProviders (#233114)
Browse files Browse the repository at this point in the history
* Log auth stuff around declaredProviders

* add test change

* Use info instead for now

* fix integration tests
  • Loading branch information
TylerLeonhardt authored Nov 5, 2024
1 parent a970a0d commit b9700a0
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { IProductService } from '../../../../platform/product/common/productServ
import { AuthenticationAccessService, IAuthenticationAccessService } from '../../../services/authentication/browser/authenticationAccessService.js';
import { AuthenticationUsageService, IAuthenticationUsageService } from '../../../services/authentication/browser/authenticationUsageService.js';
import { AuthenticationExtensionsService } from '../../../services/authentication/browser/authenticationExtensionsService.js';
import { ILogService, NullLogService } from '../../../../platform/log/common/log.js';

class AuthQuickPick {
private listener: ((e: IQuickPickDidAcceptEvent) => any) | undefined;
Expand Down Expand Up @@ -105,6 +106,7 @@ suite('ExtHostAuthentication', () => {

suiteSetup(async () => {
instantiationService = new TestInstantiationService();
instantiationService.stub(ILogService, new NullLogService());
instantiationService.stub(IDialogService, new TestDialogService({ confirmed: true }));
instantiationService.stub(IStorageService, new TestStorageService());
instantiationService.stub(IQuickInputService, new AuthTestQuickInputService());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ExtensionsRegistry } from '../../../services/extensions/common/extensio
import { ManageTrustedExtensionsForAccountAction } from './actions/manageTrustedExtensionsForAccountAction.js';
import { ManageAccountPreferencesForExtensionAction } from './actions/manageAccountPreferencesForExtensionAction.js';
import { IAuthenticationUsageService } from '../../../services/authentication/browser/authenticationUsageService.js';
import { ILogService } from '../../../../platform/log/common/log.js';

const codeExchangeProxyCommand = CommandsRegistry.registerCommand('workbench.getCodeExchangeProxyEndpoints', function (accessor, _) {
const environmentService = accessor.get(IBrowserWorkbenchEnvironmentService);
Expand Down Expand Up @@ -117,7 +118,10 @@ class AuthenticationContribution extends Disposable implements IWorkbenchContrib
},
});

constructor(@IAuthenticationService private readonly _authenticationService: IAuthenticationService) {
constructor(
@IAuthenticationService private readonly _authenticationService: IAuthenticationService,
@ILogService private readonly _logService: ILogService
) {
super();
this._register(codeExchangeProxyCommand);
this._register(extensionFeature);
Expand All @@ -133,6 +137,7 @@ class AuthenticationContribution extends Disposable implements IWorkbenchContrib

private _registerAuthenticationExtentionPointHandler(): void {
authenticationExtPoint.setHandler((extensions, { added, removed }) => {
this._logService.info(`Found authentication providers. added: ${added.length}, removed: ${removed.length}`);
added.forEach(point => {
for (const provider of point.value) {
if (isFalsyOrWhitespace(provider.id)) {
Expand All @@ -147,6 +152,7 @@ class AuthenticationContribution extends Disposable implements IWorkbenchContrib

if (!this._authenticationService.declaredProviders.some(p => p.id === provider.id)) {
this._authenticationService.registerDeclaredAuthenticationProvider(provider);
this._logService.info(`Declared authentication provider: ${provider.id}`);
} else {
point.collector.error(localize('authentication.idConflict', "This authentication id '{0}' has already been registered", provider.id));
}
Expand All @@ -158,6 +164,7 @@ class AuthenticationContribution extends Disposable implements IWorkbenchContrib
const provider = this._authenticationService.declaredProviders.find(provider => provider.id === point.id);
if (provider) {
this._authenticationService.unregisterDeclaredAuthenticationProvider(provider.id);
this._logService.info(`Undeclared authentication provider: ${provider.id}`);
}
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IAuthenticationAccessService } from './authenticationAccessService.js';
import { AuthenticationProviderInformation, AuthenticationSession, AuthenticationSessionAccount, AuthenticationSessionsChangeEvent, IAuthenticationCreateSessionOptions, IAuthenticationProvider, IAuthenticationService } from '../common/authentication.js';
import { IBrowserWorkbenchEnvironmentService } from '../../environment/browser/environmentService.js';
import { ActivationKind, IExtensionService } from '../../extensions/common/extensions.js';
import { ILogService } from '../../../../platform/log/common/log.js';

export function getAuthenticationProviderActivationEvent(id: string): string { return `onAuthenticationRequest:${id}`; }

Expand Down Expand Up @@ -64,7 +65,8 @@ export class AuthenticationService extends Disposable implements IAuthentication
constructor(
@IExtensionService private readonly _extensionService: IExtensionService,
@IAuthenticationAccessService authenticationAccessService: IAuthenticationAccessService,
@IBrowserWorkbenchEnvironmentService private readonly _environmentService: IBrowserWorkbenchEnvironmentService
@IBrowserWorkbenchEnvironmentService private readonly _environmentService: IBrowserWorkbenchEnvironmentService,
@ILogService private readonly _logService: ILogService
) {
super();

Expand Down Expand Up @@ -95,6 +97,7 @@ export class AuthenticationService extends Disposable implements IAuthentication
return;
}
for (const provider of this._environmentService.options.authenticationProviders) {
this.registerDeclaredAuthenticationProvider(provider);
this.registerAuthenticationProvider(provider.id, provider);
}
}
Expand Down Expand Up @@ -126,6 +129,9 @@ export class AuthenticationService extends Disposable implements IAuthentication
}

registerAuthenticationProvider(id: string, authenticationProvider: IAuthenticationProvider): void {
if (!this._declaredProviders.find(p => p.id === id)) {
this._logService.warn(`Registering an authentication provider that is not declared in the Extension Manifest. This may cause unexpected behavior. id: ${id}, label: ${authenticationProvider.label}`);
}
this._authenticationProviders.set(id, authenticationProvider);
const disposableStore = new DisposableStore();
disposableStore.add(authenticationProvider.onDidChangeSessions(e => this._onDidChangeSessions.fire({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { AuthenticationService } from '../../browser/authenticationService.js';
import { AuthenticationProviderInformation, AuthenticationSessionsChangeEvent, IAuthenticationProvider } from '../../common/authentication.js';
import { TestEnvironmentService } from '../../../../test/browser/workbenchTestServices.js';
import { TestExtensionService, TestProductService, TestStorageService } from '../../../../test/common/workbenchTestServices.js';
import { NullLogService } from '../../../../../platform/log/common/log.js';

function createSession() {
return { id: 'session1', accessToken: 'token1', account: { id: 'account', label: 'Account' }, scopes: ['test'] };
Expand All @@ -37,7 +38,7 @@ suite('AuthenticationService', () => {
setup(() => {
const storageService = disposables.add(new TestStorageService());
const authenticationAccessService = disposables.add(new AuthenticationAccessService(storageService, TestProductService));
authenticationService = disposables.add(new AuthenticationService(new TestExtensionService(), authenticationAccessService, TestEnvironmentService));
authenticationService = disposables.add(new AuthenticationService(new TestExtensionService(), authenticationAccessService, TestEnvironmentService, new NullLogService()));
});

teardown(() => {
Expand Down

0 comments on commit b9700a0

Please sign in to comment.