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.

Sign up for GitHub

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

Jump to bottom

Log auth stuff around declaredProviders #233114

Merged
merged 4 commits into from
Nov 5, 2024
+19 −3
Merged

Log auth stuff around declaredProviders #233114

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8233e72
Log auth stuff around declaredProviders
TylerLeonhardt Nov 5, 2024
bcb68e6
add test change
TylerLeonhardt Nov 5, 2024
53fff85
Use info instead for now
TylerLeonhardt Nov 5, 2024
895e1f9
fix integration tests
TylerLeonhardt Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/vs/workbench/contrib/authentication/browser/authentication.contribution.ts
View file
Open in desktop
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
8 changes: 7 additions & 1 deletion src/vs/workbench/services/authentication/browser/authenticationService.ts
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion src/vs/workbench/services/authentication/test/browser/authenticationService.test.ts
View file
Open in desktop
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
Loading