Skip to content

Commit

Permalink
Refactor auth error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
pkukielka committed Jan 23, 2025
1 parent 9d44ba3 commit 70a27b7
Show file tree
Hide file tree
Showing 18 changed files with 179 additions and 191 deletions.
161 changes: 85 additions & 76 deletions lib/shared/src/auth/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { isDotCom } from '../sourcegraph-api/environments'
import { NeedsAuthChallengeError } from '../sourcegraph-api/errors'
import type { UserProductSubscription } from '../sourcegraph-api/userProductSubscription'

/**
Expand Down Expand Up @@ -50,90 +49,100 @@ export interface UnauthenticatedAuthStatus {
pendingValidation: boolean
}

export class AuthenticationError extends Error {
public title: string
public showTryAgain = false

constructor(title: string, message: string) {
super(message)
this.title = title
}
}

/**
* 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'
export class AvailabilityError extends AuthenticationError {
constructor() {
super('Network Error', 'Sourcegraph is unreachable')
// 'Network issues prevented Cody from signing in.'
this.showTryAgain = true
}
}

/**
* Whether the error is due to a proxy needing the user to complete an auth challenge. See
* {@link NeedsAuthChallengeError}.
*/
needsAuthChallenge?: boolean
}

export interface InvalidAccessTokenError {
type: 'invalid-access-token'
}

export interface EnterpriseUserDotComError {
type: 'enterprise-user-logged-into-dotcom'
enterprise: string
}

export interface AuthConfigError {
type: 'auth-config-error'
message: string
}

export interface ExternalAuthProviderError {
type: 'external-auth-provider-error'
message: string
}

export type AuthenticationError =
| AvailabilityError
| InvalidAccessTokenError
| EnterpriseUserDotComError
| AuthConfigError
| ExternalAuthProviderError

export interface AuthenticationErrorMessage {
title?: string
message: string
showTryAgain?: boolean
}

export function getAuthErrorMessage(error: AuthenticationError): AuthenticationErrorMessage {
switch (error.type) {
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',
message: 'The access token is invalid or has expired',
}
case 'enterprise-user-logged-into-dotcom':
return {
title: 'Enterprise User Authentication Error',
message:
'Based on your email address we think you may be an employee of ' +
`${error.enterprise}. To get access to all your features please sign ` +
"in through your organization's enterprise instance instead. If you need assistance " +
'please contact your Sourcegraph admin.',
}
case 'auth-config-error':
return {
title: 'Auth Config Error',
message: error.message,
}
case 'external-auth-provider-error':
return {
title: 'External Auth Provider Error',
message: error.message,
}
export class InvalidAccessTokenError extends AuthenticationError {
// type: 'invalid-access-token'
constructor() {
super('Invalid Access Token', 'The access token is invalid or has expired')
//'Your authentication has expired.\nSign in again to continue using Cody.',
}
}

export class EnterpriseUserDotComError extends AuthenticationError {
// type: 'enterprise-user-logged-into-dotcom'
constructor(enterprise: string) {
super(
'Enterprise User Authentication Error',
'Based on your email address we think you may be an employee of ' +
`${enterprise}. To get access to all your features please sign ` +
"in through your organization's enterprise instance instead. If you need assistance " +
'please contact your Sourcegraph admin.'
)
}
}

export class AuthConfigError extends AuthenticationError {
// type: 'auth-config-error'
constructor(message: string) {
super('Auth Config Error', message)
}
}

export class ExternalAuthProviderError extends AuthenticationError {
// public title = 'External Auth Provider Error'
constructor(message: string) {
super('External Auth Provider Error', message)
}
}

/**
* An error indicating that the user needs to complete an authentication challenge.
*/
export class NeedsAuthChallengeError extends AuthenticationError {
constructor() {
// 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.
super(
'Tap Your YubiKey to Authenticate',
`Your device's authentication expired and must be renewed to access Sourcegraph on your organization's network.`
)
}
}

export function isExternalProviderAuthError(error: unknown): error is ExternalAuthProviderError {
return error instanceof ExternalAuthProviderError
}

export function isNeedsAuthChallengeError(error: unknown): error is NeedsAuthChallengeError {
return error instanceof NeedsAuthChallengeError
}

export function isAvailabilityError(error: unknown): error is AvailabilityError {
return error instanceof AvailabilityError
}

export function isInvalidAccessTokenError(error: unknown): error is InvalidAccessTokenError {
return error instanceof InvalidAccessTokenError
}

export function isEnterpriseUserDotComError(error: unknown): error is EnterpriseUserDotComError {
return error instanceof EnterpriseUserDotComError
}

export const AUTH_STATUS_FIXTURE_AUTHED: AuthenticatedAuthStatus = {
endpoint: 'https://example.com',
authenticated: true,
Expand Down
4 changes: 2 additions & 2 deletions lib/shared/src/configuration/auth-resolver.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Subject } from 'observable-fns'
import { ExternalAuthProviderError } from '..'
import type {
AuthCredentials,
ClientConfiguration,
ExternalAuthCommand,
ExternalAuthProvider,
} from '../configuration'
import { logError } from '../logger'
import { ExternalProviderAuthError } from '../sourcegraph-api/errors'
import type { ClientSecrets } from './resolver'

export const externalAuthRefresh = new Subject<void>()
Expand Down Expand Up @@ -116,7 +116,7 @@ function createHeaderCredentials(
externalAuthRefresh.next()

logError('resolveAuth', `External Auth Provider Error: ${error}`)
throw new ExternalProviderAuthError(
throw new ExternalAuthProviderError(
error instanceof Error ? error.message : String(error)
)
}
Expand Down
2 changes: 0 additions & 2 deletions lib/shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,6 @@ export {
isNetworkError,
isNetworkLikeError,
isRateLimitError,
NeedsAuthChallengeError,
isNeedsAuthChallengeError,
} from './sourcegraph-api/errors'
export {
SourcegraphGraphQLAPIClient,
Expand Down
45 changes: 6 additions & 39 deletions lib/shared/src/sourcegraph-api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { differenceInDays, format, formatDistanceStrict, formatRelative } from '

import { isError } from '../utils'

import type { AuthenticationErrorMessage } from '../auth/types'
import { AuthenticationError } from '../auth/types'
import type { BrowserOrNodeResponse } from './graphql/client'

function formatRetryAfterDate(retryAfterDate: Date): string {
Expand Down Expand Up @@ -89,8 +89,11 @@ export function isNetworkError(error: Error): error is NetworkError {
return error instanceof NetworkError
}

export function isAuthError(error: unknown): boolean {
return error instanceof NetworkError && (error.status === 401 || error.status === 403)
export function isAuthError(error: unknown): error is AuthenticationError | NetworkError {
return (
(error instanceof NetworkError && (error.status === 401 || error.status === 403)) ||
error instanceof AuthenticationError
)
}

export class AbortError extends Error {
Expand Down Expand Up @@ -136,39 +139,3 @@ export function isNetworkLikeError(error: Error): boolean {
message.includes('SELF_SIGNED_CERT_IN_CHAIN')
)
}

export class ExternalProviderAuthError extends Error {
// Added to make TypeScript understand that ExternalProviderAuthError is not the same as Error.
public readonly isExternalProviderAuthError = true
}

export function isExternalProviderAuthError(error: unknown): error is ExternalProviderAuthError {
return error instanceof ExternalProviderAuthError
}

/**
* 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
}
3 changes: 1 addition & 2 deletions lib/shared/src/sourcegraph-api/graphql/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { SourcegraphGraphQLAPIClient } from '../..'
import { NeedsAuthChallengeError, SourcegraphGraphQLAPIClient } from '../..'
import * as fetchModule from '../../fetch'
import { NeedsAuthChallengeError } from '../errors'

vi.mocked(fetchModule)

Expand Down
3 changes: 2 additions & 1 deletion lib/shared/src/sourcegraph-api/graphql/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ import isEqual from 'lodash/isEqual'
import omit from 'lodash/omit'
import { Observable } from 'observable-fns'
import semver from 'semver'
import { NeedsAuthChallengeError, isNeedsAuthChallengeError } from '../..'
import { dependentAbortController, onAbort } from '../../common/abortController'
import { type PickResolvedConfiguration, resolvedConfig } from '../../configuration/resolver'
import { logError } from '../../logger'
import { distinctUntilChanged, firstValueFrom } from '../../misc/observable'
import { addTraceparent, wrapInActiveSpan } from '../../tracing'
import { isError } from '../../utils'
import { addCodyClientIdentificationHeaders } from '../client-name-version'
import { NeedsAuthChallengeError, isAbortError, isNeedsAuthChallengeError } from '../errors'
import { isAbortError } from '../errors'
import { addAuthHeaders } from '../utils'
import { type GraphQLResultCache, ObservableInvalidatedGraphQLResultCacheFactory } from './cache'
import {
Expand Down
11 changes: 5 additions & 6 deletions vscode/src/auth/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
type AuthStatus,
AvailabilityError,
CLIENT_CAPABILITIES_FIXTURE,
InvalidAccessTokenError,
NeedsAuthChallengeError,
SourcegraphGraphQLAPIClient,
mockAuthStatus,
Expand Down Expand Up @@ -36,7 +38,7 @@ describe('showSignInMenu', () => {
.fn<(typeof AuthProviderModule.authProvider)['validateAndStoreCredentials']>()
.mockResolvedValue({
authenticated: false,
error: { type: 'availability-error' },
error: new AvailabilityError(),
endpoint: 'https://example.com',
pendingValidation: false,
}),
Expand Down Expand Up @@ -64,7 +66,7 @@ describe('showSignInMenu', () => {
.fn<(typeof AuthProviderModule.authProvider)['validateAndStoreCredentials']>()
.mockResolvedValue({
authenticated: false,
error: { type: 'invalid-access-token' },
error: new InvalidAccessTokenError(),
endpoint: 'https://example.com',
pendingValidation: false,
}),
Expand Down Expand Up @@ -117,10 +119,7 @@ describe('validateCredentials', () => {

expect(result).toEqual<AuthStatus>({
authenticated: false,
error: {
type: 'availability-error',
needsAuthChallenge: true,
},
error: new NeedsAuthChallengeError(),
endpoint: 'https://sourcegraph.test',
pendingValidation: false,
})
Expand Down
Loading

0 comments on commit 70a27b7

Please sign in to comment.