diff --git a/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt b/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt index 1a9d1c571983..d6ce773d168f 100644 --- a/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt +++ b/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt @@ -13,7 +13,7 @@ sealed class AuthenticationError { val deserializer: JsonDeserializer = JsonDeserializer { element: JsonElement, _: Type, context: JsonDeserializationContext -> when (element.getAsJsonObject().get("type").getAsString()) { - "network-error" -> context.deserialize(element, NetworkAuthError::class.java) + "availability-error" -> context.deserialize(element, AvailabilityError::class.java) "invalid-access-token" -> context.deserialize(element, InvalidAccessTokenError::class.java) "enterprise-user-logged-into-dotcom" -> context.deserialize(element, EnterpriseUserDotComError::class.java) "auth-config-error" -> context.deserialize(element, AuthConfigError::class.java) @@ -23,12 +23,12 @@ sealed class AuthenticationError { } } -data class NetworkAuthError( - val type: TypeEnum, // Oneof: network-error +data class AvailabilityError( + val type: TypeEnum, // Oneof: Availability-error ) : AuthenticationError() { enum class TypeEnum { - @SerializedName("network-error") `Network-error`, + @SerializedName("availability-error") `Availability-error`, } } diff --git a/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt b/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt index 6beac30c79ec..db652b4ca310 100644 --- a/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt +++ b/agent/bindings/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt @@ -60,7 +60,7 @@ object Constants { const val method = "method" const val multiple = "multiple" const val native = "native" - const val `network-error` = "network-error" + const val `availability-error` = "availability-error" const val none = "none" const val notification = "notification" const val `object-encoded` = "object-encoded" diff --git a/lib/shared/src/auth/types.ts b/lib/shared/src/auth/types.ts index cca6b6bd6d3a..03a7ce4c218d 100644 --- a/lib/shared/src/auth/types.ts +++ b/lib/shared/src/auth/types.ts @@ -1,4 +1,5 @@ import { isDotCom } from '../sourcegraph-api/environments' +import { NeedsAuthChallengeError } from '../sourcegraph-api/errors' import type { UserProductSubscription } from '../sourcegraph-api/userProductSubscription' /** @@ -49,8 +50,19 @@ export interface UnauthenticatedAuthStatus { pendingValidation: boolean } -export interface NetworkAuthError { - type: 'network-error' +/** + * An error representing the condition where the endpoint is not available due to lack of network + * connectivity, server downtime, or other configuration issues *unrelated to* the validity of the + * credentials. + */ +export interface AvailabilityError { + type: 'availability-error' + + /** + * Whether the error is due to a proxy needing the user to complete an auth challenge. See + * {@link NeedsAuthChallengeError}. + */ + needsAuthChallenge?: boolean } export interface InvalidAccessTokenError { @@ -67,7 +79,7 @@ export interface AuthConfigError extends AuthenticationErrorMessage { } export type AuthenticationError = - | NetworkAuthError + | AvailabilityError | InvalidAccessTokenError | EnterpriseUserDotComError | AuthConfigError @@ -75,15 +87,19 @@ export type AuthenticationError = export interface AuthenticationErrorMessage { title?: string message: string + showTryAgain?: boolean } export function getAuthErrorMessage(error: AuthenticationError): AuthenticationErrorMessage { switch (error.type) { - case 'network-error': - return { - title: 'Network Error', - message: 'Cody is unreachable', - } + case 'availability-error': + return error.needsAuthChallenge + ? NeedsAuthChallengeError.TITLE_AND_MESSAGE + : { + title: 'Network Error', + message: 'Sourcegraph is unreachable', + showTryAgain: true, + } case 'invalid-access-token': return { title: 'Invalid Access Token', diff --git a/lib/shared/src/index.ts b/lib/shared/src/index.ts index 39fe06f2d797..84bae91875c1 100644 --- a/lib/shared/src/index.ts +++ b/lib/shared/src/index.ts @@ -252,6 +252,8 @@ export { isNetworkError, isNetworkLikeError, isRateLimitError, + NeedsAuthChallengeError, + isNeedsAuthChallengeError, } from './sourcegraph-api/errors' export { SourcegraphGraphQLAPIClient, @@ -280,6 +282,8 @@ export { type NLSSearchDynamicFilter, type NLSSearchDynamicFilterKind, type GraphQLAPIClientConfig, + setJSONAcceptContentTypeHeaders, + isCustomAuthChallengeResponse, } from './sourcegraph-api/graphql/client' export type { CodyLLMSiteConfiguration, diff --git a/lib/shared/src/sourcegraph-api/clientConfig.ts b/lib/shared/src/sourcegraph-api/clientConfig.ts index 6b92cbf80156..da460a0cdd65 100644 --- a/lib/shared/src/sourcegraph-api/clientConfig.ts +++ b/lib/shared/src/sourcegraph-api/clientConfig.ts @@ -107,7 +107,7 @@ export class ClientConfigSingleton { // Don't update if the editor is in the background, to avoid network // activity that can cause OS warnings or authorization flows when the // user is not using Cody. See - // linear.app/sourcegraph/issue/CODY-3745/codys-background-periodic-network-access-causes-2fa. + // https://linear.app/sourcegraph/issue/CODY-3745/codys-background-periodic-network-access-causes-2fa. filter((_value): _value is undefined => editorWindowIsFocused()), startWith(undefined), switchMap(() => promiseFactoryToObservable(signal => this.fetchConfig(signal))) diff --git a/lib/shared/src/sourcegraph-api/completions/browserClient.ts b/lib/shared/src/sourcegraph-api/completions/browserClient.ts index 8475b4f3aa74..cdcfeaf0625f 100644 --- a/lib/shared/src/sourcegraph-api/completions/browserClient.ts +++ b/lib/shared/src/sourcegraph-api/completions/browserClient.ts @@ -6,6 +6,7 @@ import { isError } from '../../utils' import { addClientInfoParams, addCodyClientIdentificationHeaders } from '../client-name-version' import { addAuthHeaders } from '../utils' +import { verifyResponseCode } from '../graphql/client' import { CompletionsResponseBuilder } from './CompletionsResponseBuilder' import { type CompletionRequestParameters, SourcegraphCompletionsClient } from './client' import { parseCompletionJSON } from './parse' @@ -60,18 +61,7 @@ export class SourcegraphBrowserCompletionsClient extends SourcegraphCompletionsC openWhenHidden: isRunningInWebWorker, // otherwise tries to call document.addEventListener async onopen(response) { if (!response.ok && response.headers.get('content-type') !== 'text/event-stream') { - let errorMessage: null | string = null - try { - errorMessage = await response.text() - } catch (error) { - // We show the generic error message in this case - console.error(error) - } - const error = new Error( - errorMessage === null || errorMessage.length === 0 - ? `Request failed with status code ${response.status}` - : errorMessage - ) + const error = await verifyResponseCode(response).catch(err => err) cb.onError(error, response.status) abort.abort() return @@ -128,6 +118,7 @@ export class SourcegraphBrowserCompletionsClient extends SourcegraphCompletionsC const { url, serializedParams } = await this.prepareRequest(params, requestParams) const headersInstance = new Headers({ 'Content-Type': 'application/json; charset=utf-8', + Accept: 'text/event-stream', ...configuration.customHeaders, ...requestParams.customHeaders, }) @@ -143,15 +134,7 @@ export class SourcegraphBrowserCompletionsClient extends SourcegraphCompletionsC headers: headersInstance, body: JSON.stringify(serializedParams), signal, - }) - if (!response.ok) { - const errorMessage = await response.text() - throw new Error( - errorMessage.length === 0 - ? `Request failed with status code ${response.status}` - : errorMessage - ) - } + }).then(verifyResponseCode) const data = (await response.json()) as CompletionResponse if (data?.completion) { cb.onChange(data.completion) diff --git a/lib/shared/src/sourcegraph-api/errors.ts b/lib/shared/src/sourcegraph-api/errors.ts index 1e9185ef4fb6..85a62f84ef7b 100644 --- a/lib/shared/src/sourcegraph-api/errors.ts +++ b/lib/shared/src/sourcegraph-api/errors.ts @@ -2,6 +2,7 @@ import { differenceInDays, format, formatDistanceStrict, formatRelative } from ' import { isError } from '../utils' +import type { AuthenticationErrorMessage } from '../auth/types' import type { BrowserOrNodeResponse } from './graphql/client' function formatRetryAfterDate(retryAfterDate: Date): string { @@ -135,3 +136,30 @@ export function isNetworkLikeError(error: Error): boolean { message.includes('SELF_SIGNED_CERT_IN_CHAIN') ) } + +/** + * An error indicating that the user needs to complete an authentication challenge. + */ +export class NeedsAuthChallengeError extends Error { + constructor() { + super(NeedsAuthChallengeError.DESCRIPTION) + } + + /** A human-readable description of this error. */ + static DESCRIPTION = + `Tap your YubiKey to re-authenticate. (Your device's authentication expired and must be renewed to access Sourcegraph on your organization's network.)` + + /** The same, but separated into title and message. */ + static TITLE_AND_MESSAGE: AuthenticationErrorMessage = { + // See + // https://linear.app/sourcegraph/issue/CODY-4695/handle-customer-proxy-re-auth-response-by-retrying-not-prompting-user + // for an explanation of this message. If you need to change it to something more general, + // consult the customers mentioned in that issue. + title: 'Tap Your YubiKey to Authenticate...', + message: `Your device's authentication expired and must be renewed to access Sourcegraph on your organization's network.`, + } +} + +export function isNeedsAuthChallengeError(error: Error): error is NeedsAuthChallengeError { + return error instanceof NeedsAuthChallengeError +} diff --git a/lib/shared/src/sourcegraph-api/graphql/client.test.ts b/lib/shared/src/sourcegraph-api/graphql/client.test.ts new file mode 100644 index 000000000000..cfc41af97ff1 --- /dev/null +++ b/lib/shared/src/sourcegraph-api/graphql/client.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it, vi } from 'vitest' +import { SourcegraphGraphQLAPIClient } from '../..' +import * as fetchModule from '../../fetch' +import { NeedsAuthChallengeError } from '../errors' + +vi.mocked(fetchModule) + +describe('SourcegraphGraphQLClient', () => { + const client = SourcegraphGraphQLAPIClient.withStaticConfig({ + auth: { + credentials: { token: 'test-token' }, + serverEndpoint: 'https://test.sourcegraph.com', + }, + clientState: { anonymousUserID: 'a' }, + configuration: { + telemetryLevel: 'off', + }, + }) + + describe('fetchHTTP', () => { + it('sets Accept header', async () => { + const fetchMock = vi + .spyOn(fetchModule, 'fetch') + .mockImplementation(async () => + Promise.resolve(new Response(JSON.stringify({ data: {} }), { status: 200 })) + ) + await client.fetchHTTP('TestQuery', 'POST', '/graphql', '{}') + + expect(fetchMock).toHaveBeenCalled() + const headers = fetchMock.mock.calls[0][1]?.headers as Headers + expect(headers.get('Accept')).toBe('application/json') + + fetchMock.mockRestore() + }) + }) + + describe('getCurrentUserInfo', () => { + it('returns NeedsAuthChallengeError when response requires auth challenge', async () => { + const fetchMock = vi.spyOn(fetchModule, 'fetch').mockImplementation(async () => + Promise.resolve( + new Response(null, { + status: 401, + headers: new Headers({ + 'X-CustomerName-U2f-Challenge': 'true', + }), + }) + ) + ) + const result = await client.getCurrentUserInfo() + expect(fetchMock).toHaveBeenCalled() + console.log('XX', result) + expect(result).toBeInstanceOf(NeedsAuthChallengeError) + }) + }) +}) diff --git a/lib/shared/src/sourcegraph-api/graphql/client.ts b/lib/shared/src/sourcegraph-api/graphql/client.ts index b0782dd4102e..940b0bb53a87 100644 --- a/lib/shared/src/sourcegraph-api/graphql/client.ts +++ b/lib/shared/src/sourcegraph-api/graphql/client.ts @@ -4,6 +4,7 @@ import { fetch } from '../../fetch' import type { TelemetryEventInput } from '@sourcegraph/telemetry' +import type { IncomingMessage } from 'http' import escapeRegExp from 'lodash/escapeRegExp' import isEqual from 'lodash/isEqual' import omit from 'lodash/omit' @@ -16,7 +17,7 @@ import { distinctUntilChanged, firstValueFrom } from '../../misc/observable' import { addTraceparent, wrapInActiveSpan } from '../../tracing' import { isError } from '../../utils' import { addCodyClientIdentificationHeaders } from '../client-name-version' -import { isAbortError } from '../errors' +import { NeedsAuthChallengeError, isAbortError, isNeedsAuthChallengeError } from '../errors' import { addAuthHeaders } from '../utils' import { type GraphQLResultCache, ObservableInvalidatedGraphQLResultCacheFactory } from './cache' import { @@ -1612,7 +1613,6 @@ export class SourcegraphGraphQLAPIClient { })()) const headers = new Headers(config.configuration?.customHeaders as HeadersInit | undefined) - headers.set('Content-Type', 'application/json; charset=utf-8') if (config.clientState.anonymousUserID && !process.env.CODY_WEB_DONT_SET_SOME_HEADERS) { headers.set('X-Sourcegraph-Actor-Anonymous-UID', config.clientState.anonymousUserID) } @@ -1622,6 +1622,7 @@ export class SourcegraphGraphQLAPIClient { addTraceparent(headers) addCodyClientIdentificationHeaders(headers) addAuthHeaders(config.auth, headers, url) + setJSONAcceptContentTypeHeaders(headers) const { abortController, timeoutSignal } = dependentAbortControllerWithTimeout(signal) return wrapInActiveSpan(`httpapi.fetch${queryName ? `.${queryName}` : ''}`, () => @@ -1638,6 +1639,11 @@ export class SourcegraphGraphQLAPIClient { } } +export function setJSONAcceptContentTypeHeaders(headers: Headers): void { + headers.set('Accept', 'application/json') + headers.set('Content-Type', 'application/json; charset=utf-8') +} + const DEFAULT_TIMEOUT_MSEC = 20000 /** @@ -1664,6 +1670,10 @@ function catchHTTPError( timeoutSignal: Pick ): (error: any) => Error { return (error: any) => { + if (isNeedsAuthChallengeError(error)) { + return error + } + // Throw the plain AbortError for intentional aborts so we handle them with call // flow, but treat timeout aborts as a network error (below) and include the // URL. @@ -1686,13 +1696,55 @@ export const graphqlClient = SourcegraphGraphQLAPIClient.withGlobalConfig() export async function verifyResponseCode( response: BrowserOrNodeResponse ): Promise { + if (isCustomAuthChallengeResponse(response)) { + throw new NeedsAuthChallengeError() + } + if (!response.ok) { - const body = await response.text() + let body: string | undefined + try { + body = await response.text() + } catch {} throw new Error(`HTTP status code ${response.status}${body ? `: ${body}` : ''}`) } return response } +/** + * Some customers use an HTTP proxy in front of Sourcegraph that, when the user needs to complete an + * auth challenge, returns HTTP 401 with a `X-${CUSTOMER}-U2f-Challenge: true` header. Detect these + * kinds of error responses. + */ +export function isCustomAuthChallengeResponse( + response: + | Pick + | Pick +): boolean { + /////////////////////////////////// + // TODO!(sqs): remove + /////////////////////////////////// + if (require('node:fs').existsSync('/tmp/is-custom-auth-challenge-error')) { + return true + } + + function isIncomingMessageType( + v: typeof response + ): v is Pick { + return 'httpVersion' in response + } + const statusCode = isIncomingMessageType(response) ? response.statusCode : response.status + if (statusCode === 401) { + const headerEntries = isIncomingMessageType(response) + ? Object.entries(response.headers) + : Array.from(response.headers.entries()) + return headerEntries.some( + ([name, value]) => /^x-.*-u2f-challenge$/i.test(name) && value === 'true' + ) + } + + return false +} + function hasOutdatedAPIErrorMessages(error: Error): boolean { // Sourcegraph 5.2.3 returns an empty string ("") instead of an error message // when querying non-existent codyContextFilters; this produces diff --git a/vscode/src/auth/auth.test.ts b/vscode/src/auth/auth.test.ts new file mode 100644 index 000000000000..96ce8cc754ca --- /dev/null +++ b/vscode/src/auth/auth.test.ts @@ -0,0 +1,128 @@ +import { + type AuthStatus, + CLIENT_CAPABILITIES_FIXTURE, + NeedsAuthChallengeError, + SourcegraphGraphQLAPIClient, + mockAuthStatus, + mockClientCapabilities, + mockResolvedConfig, +} from '@sourcegraph/cody-shared' +import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest' +import { type MockInstance, vi } from 'vitest' +import type * as vscode from 'vscode' +import * as AuthProviderModule from '../services/AuthProvider' +import { mockLocalStorage } from '../services/LocalStorageProvider' +import { vsCodeMocks } from '../testutils/mocks' +import { showSignInMenu, validateCredentials } from './auth' + +describe('showSignInMenu', () => { + beforeAll(() => { + mockResolvedConfig({ + configuration: { serverEndpoint: 'https://example.com', overrideAuthToken: 'x' }, + auth: { serverEndpoint: 'https://example.com' }, + }) + mockClientCapabilities(CLIENT_CAPABILITIES_FIXTURE) + mockAuthStatus({ authenticated: false }) + mockLocalStorage('noop') + vi.spyOn( + vsCodeMocks.window as unknown as typeof vscode.window, + 'showQuickPick' + ).mockImplementation(async () => ({ id: '1', label: 'l', uri: 'https://example.com' })) + }) + + it('does not show access token input box when authentication fails with availability error', async () => { + vi.spyOn(AuthProviderModule, 'authProvider', 'get').mockReturnValue({ + validateAndStoreCredentials: vi + .fn<(typeof AuthProviderModule.authProvider)['validateAndStoreCredentials']>() + .mockResolvedValue({ + authenticated: false, + error: { type: 'availability-error' }, + endpoint: 'https://example.com', + pendingValidation: false, + }), + } satisfies Pick< + typeof AuthProviderModule.authProvider, + 'validateAndStoreCredentials' + > as unknown as typeof AuthProviderModule.authProvider) + const mockShowInputBox = vi.spyOn( + vsCodeMocks.window as unknown as typeof vscode.window, + 'showInputBox' + ) + const mockShowErrorMessage = vi.spyOn( + vsCodeMocks.window as unknown as typeof vscode.window, + 'showErrorMessage' + ) + + await showSignInMenu() + expect(mockShowInputBox).not.toHaveBeenCalled() + expect(mockShowErrorMessage).toHaveBeenCalledWith('Sourcegraph is unreachable') + }) + + it('shows access token input box when authentication fails with invalid access token error', async () => { + vi.spyOn(AuthProviderModule, 'authProvider', 'get').mockReturnValue({ + validateAndStoreCredentials: vi + .fn<(typeof AuthProviderModule.authProvider)['validateAndStoreCredentials']>() + .mockResolvedValue({ + authenticated: false, + error: { type: 'invalid-access-token' }, + endpoint: 'https://example.com', + pendingValidation: false, + }), + } satisfies Pick< + typeof AuthProviderModule.authProvider, + 'validateAndStoreCredentials' + > as unknown as typeof AuthProviderModule.authProvider) + const mockShowInputBox = vi + .spyOn( + vsCodeMocks.window as unknown as typeof vscode.window, + 'showInputBox' + ) + .mockResolvedValue('my-token') + const mockShowErrorMessage = vi.spyOn( + vsCodeMocks.window as unknown as typeof vscode.window, + 'showErrorMessage' + ) + + await showSignInMenu() + expect(mockShowInputBox).toHaveBeenCalled() + expect(mockShowErrorMessage).toHaveBeenCalledWith('The access token is invalid or has expired') + }) +}) + +describe('validateCredentials', () => { + let getCurrentUserInfoSpy: MockInstance + beforeEach(() => { + getCurrentUserInfoSpy = vi.spyOn(SourcegraphGraphQLAPIClient.prototype, 'getCurrentUserInfo') + }) + + afterEach(() => { + vi.resetAllMocks() + }) + + it('returns unauthenticated status when NeedsAuthChallengeError occurs', async () => { + getCurrentUserInfoSpy.mockResolvedValue(new NeedsAuthChallengeError()) + + const result = await validateCredentials({ + auth: { + serverEndpoint: 'https://sourcegraph.test', + credentials: { token: 'test-token' }, + }, + configuration: { + customHeaders: {}, + }, + clientState: { + anonymousUserID: 'test-user', + }, + }) + + expect(result).toEqual({ + authenticated: false, + error: { + type: 'availability-error', + needsAuthChallenge: true, + }, + endpoint: 'https://sourcegraph.test', + pendingValidation: false, + }) + }) +}) diff --git a/vscode/src/auth/auth.ts b/vscode/src/auth/auth.ts index 414baef92c59..8285557dd881 100644 --- a/vscode/src/auth/auth.ts +++ b/vscode/src/auth/auth.ts @@ -18,6 +18,7 @@ import { graphqlClient, isDotCom, isError, + isNeedsAuthChallengeError, isNetworkLikeError, telemetryRecorder, } from '@sourcegraph/cody-shared' @@ -92,7 +93,9 @@ export async function showSignInMenu( let authStatus = await authProvider.validateAndStoreCredentials(auth, 'store-if-valid') - if (!authStatus?.authenticated) { + // If authentication failed because the credentials were reported as invalid (and not + // due to some other or some ephemeral reason), ask the user for a different token. + if (!authStatus?.authenticated && authStatus.error?.type === 'invalid-access-token') { const token = await showAccessTokenInputBox(selectedEndpoint) if (!token) { return @@ -445,15 +448,19 @@ export async function validateCredentials( const userInfo = await client.getCurrentUserInfo(signal) signal?.throwIfAborted() - if (isError(userInfo) && isNetworkLikeError(userInfo)) { + if (isError(userInfo) && (isNetworkLikeError(userInfo) || isNeedsAuthChallengeError(userInfo))) { logDebug( 'auth', - `Failed to authenticate to ${config.auth.serverEndpoint} due to likely network error`, + `Failed to authenticate to ${config.auth.serverEndpoint} due to likely network or endpoint availability error`, userInfo.message ) + const needsAuthChallenge = isNeedsAuthChallengeError(userInfo) return { authenticated: false, - error: { type: 'network-error' }, + error: { + type: 'availability-error', + needsAuthChallenge, + }, endpoint: config.auth.serverEndpoint, pendingValidation: false, } diff --git a/vscode/src/chat/chat-view/ChatController.ts b/vscode/src/chat/chat-view/ChatController.ts index c478a5a1efdf..411bafdbb7a2 100644 --- a/vscode/src/chat/chat-view/ChatController.ts +++ b/vscode/src/chat/chat-view/ChatController.ts @@ -413,6 +413,10 @@ export class ChatController implements vscode.Disposable, vscode.WebviewViewProv ) break case 'auth': { + if (message.authKind === 'refresh') { + authProvider.refresh() + break + } if (message.authKind === 'simplified-onboarding') { const endpoint = DOTCOM_URL.href diff --git a/vscode/src/chat/protocol.ts b/vscode/src/chat/protocol.ts index f0ca64e9d404..17d76ffa8ce4 100644 --- a/vscode/src/chat/protocol.ts +++ b/vscode/src/chat/protocol.ts @@ -117,7 +117,14 @@ export type WebviewMessage = } | { command: 'auth' - authKind: 'signin' | 'signout' | 'support' | 'callback' | 'simplified-onboarding' | 'switch' + authKind: + | 'signin' + | 'signout' + | 'support' + | 'callback' + | 'simplified-onboarding' + | 'switch' + | 'refresh' endpoint?: string | undefined | null value?: string | undefined | null authMethod?: AuthMethod | undefined | null diff --git a/vscode/src/completions/default-client.ts b/vscode/src/completions/default-client.ts index 992b174d5642..867a76cb49ba 100644 --- a/vscode/src/completions/default-client.ts +++ b/vscode/src/completions/default-client.ts @@ -29,6 +29,7 @@ import { isRateLimitError, logResponseHeadersToSpan, recordErrorToSpan, + setJSONAcceptContentTypeHeaders, setSingleton, singletonNotYetSet, tracer, @@ -70,7 +71,7 @@ class DefaultCodeCompletionsClient implements CodeCompletionsClient { // Force HTTP connection reuse to reduce latency. // c.f. https://github.com/microsoft/vscode/issues/173861 - headers.set('Content-Type', 'application/json; charset=utf-8') + setJSONAcceptContentTypeHeaders(headers) addCodyClientIdentificationHeaders(headers) addAuthHeaders(auth, headers, url) diff --git a/vscode/src/completions/nodeClient.ts b/vscode/src/completions/nodeClient.ts index cd561d66491f..c459be7ca3b6 100644 --- a/vscode/src/completions/nodeClient.ts +++ b/vscode/src/completions/nodeClient.ts @@ -10,6 +10,7 @@ import { type CompletionParameters, type CompletionRequestParameters, type CompletionResponse, + NeedsAuthChallengeError, NetworkError, RateLimitError, SourcegraphCompletionsClient, @@ -21,6 +22,7 @@ import { getSerializedParams, getTraceparentHeaders, globalAgentRef, + isCustomAuthChallengeResponse, isError, logError, onAbort, @@ -158,9 +160,15 @@ export class SourcegraphNodeCompletionsClient extends SourcegraphCompletionsClie } } - // For failed requests, we just want to read the entire body and - // ultimately return it to the error callback. + // Handle custom auth challenges. + if (isCustomAuthChallengeResponse(res)) { + handleError(new NeedsAuthChallengeError()) + return + } + if (statusCode >= 400) { + // For failed requests, we just want to read the entire body and + // ultimately return it to the error callback. // Bytes which have not been decoded as UTF-8 text let bufferBin = Buffer.of() // Text which has not been decoded as a server-sent event (SSE) diff --git a/vscode/src/services/AuthProvider.ts b/vscode/src/services/AuthProvider.ts index 3b3731dffd39..a7416c8d2e38 100644 --- a/vscode/src/services/AuthProvider.ts +++ b/vscode/src/services/AuthProvider.ts @@ -4,6 +4,7 @@ import { type ClientCapabilitiesWithLegacyFields, ClientConfigSingleton, DOTCOM_URL, + EMPTY, NEVER, type ResolvedConfiguration, type Unsubscribable, @@ -24,7 +25,7 @@ import { } from '@sourcegraph/cody-shared' import { normalizeServerEndpointURL } from '@sourcegraph/cody-shared/src/configuration/auth-resolver' import isEqual from 'lodash/isEqual' -import { Observable, Subject } from 'observable-fns' +import { Observable, Subject, interval } from 'observable-fns' import * as vscode from 'vscode' import { serializeConfigSnapshot } from '../../uninstall/serializeConfig' import { type ResolvedConfigurationCredentialsOnly, validateCredentials } from '../auth/auth' @@ -135,6 +136,34 @@ class AuthProvider implements vscode.Disposable { .subscribe({}) ) + // Try to reauthenticate periodically when the authentication failed due to an availability + // error (which is ephemeral and the underlying error condition may no longer exist). + this.subscriptions.push( + authStatus + .pipe( + switchMap(authStatus => { + if ( + !authStatus.authenticated && + authStatus.error?.type === 'availability-error' && + authStatus.error.needsAuthChallenge + ) { + // This interval is short because we want to quickly authenticate after + // the user successfully performs the auth challenge. If automatic auth + // refresh is expanded to include other conditions (such as any network + // connectivity gaps), it should probably have a longer interval, and we + // need to respect + // https://linear.app/sourcegraph/issue/CODY-3745/codys-background-periodic-network-access-causes-2fa. + const intervalMsec = 2500 + return interval(intervalMsec) + } + return EMPTY + }) + ) + .subscribe(() => { + this.refreshRequests.next() + }) + ) + // Keep context updated with auth status. this.subscriptions.push( authStatus.subscribe(authStatus => { @@ -318,7 +347,8 @@ function reportAuthTelemetryEvent(authStatus: AuthStatus): void { let eventValue: 'disconnected' | 'connected' | 'failed' if ( !authStatus.authenticated && - (authStatus.error?.type === 'network-error' || authStatus.error?.type === 'invalid-access-token') + (authStatus.error?.type === 'availability-error' || + authStatus.error?.type === 'invalid-access-token') ) { eventValue = 'failed' } else if (authStatus.authenticated) { diff --git a/vscode/src/services/StatusBar.ts b/vscode/src/services/StatusBar.ts index 65f140c94ba0..f60aca55ad52 100644 --- a/vscode/src/services/StatusBar.ts +++ b/vscode/src/services/StatusBar.ts @@ -323,13 +323,13 @@ export class CodyStatusBar implements vscode.Disposable { } } - if (!authStatus.authenticated && authStatus.error?.type === 'network-error') { + if (!authStatus.authenticated && authStatus.error?.type === 'availability-error') { return { icon: 'disabled', tooltip: 'Network issues prevented Cody from signing in.', style: 'error', tags, - interact: interactNetworkIssues, + interact: authStatus.error.needsAuthChallenge ? interactAuth : interactNetworkIssues, } } diff --git a/vscode/webviews/AuthPage.tsx b/vscode/webviews/AuthPage.tsx index afce58fbf772..6bc3f577820d 100644 --- a/vscode/webviews/AuthPage.tsx +++ b/vscode/webviews/AuthPage.tsx @@ -338,7 +338,7 @@ const ClientSignInForm: React.FC = memo( isSubmitting: false, showAuthError: !!authStatus?.authenticated || - authStatus?.error?.type === 'network-error', + authStatus?.error?.type === 'availability-error', })) }, 8000) } @@ -375,7 +375,7 @@ const ClientSignInForm: React.FC = memo( serverInvalid={ authStatus && !authStatus.authenticated && - authStatus?.error?.type === 'network-error' + authStatus?.error?.type === 'availability-error' } className="tw-m-2" > diff --git a/vscode/webviews/CodyPanel.story.tsx b/vscode/webviews/CodyPanel.story.tsx index 244eb52c2200..1b562ca00b0b 100644 --- a/vscode/webviews/CodyPanel.story.tsx +++ b/vscode/webviews/CodyPanel.story.tsx @@ -44,7 +44,7 @@ export const NetworkError: StoryObj = { clientCapabilities: CLIENT_CAPABILITIES_FIXTURE, authStatus: { ...AUTH_STATUS_FIXTURE_UNAUTHED, - error: { type: 'network-error' }, + error: { type: 'availability-error' }, }, isDotComUser: true, userProductSubscription: { diff --git a/vscode/webviews/components/AuthenticationErrorBanner.tsx b/vscode/webviews/components/AuthenticationErrorBanner.tsx index 4404b5f92b55..3375572132ff 100644 --- a/vscode/webviews/components/AuthenticationErrorBanner.tsx +++ b/vscode/webviews/components/AuthenticationErrorBanner.tsx @@ -11,20 +11,31 @@ interface AuthenticationErrorBannerProps { export const AuthenticationErrorBanner: React.FC = ({ errorMessage, }) => { + const tryAgain = useCallback(() => { + getVSCodeAPI().postMessage({ command: 'auth', authKind: 'refresh' }) + }, []) + const signOut = useCallback(() => { getVSCodeAPI().postMessage({ command: 'auth', authKind: 'signout' }) }, []) return ( -
- - - {errorMessage.title} — - {errorMessage.message} - - +
+
+ + {errorMessage.title} +
+

{errorMessage.message}

+
+ {errorMessage.showTryAgain && ( + + )} + +
) }