Skip to content

Commit

Permalink
Improve reporting auth errors
Browse files Browse the repository at this point in the history
  • Loading branch information
pkukielka committed Jan 14, 2025
1 parent 1ed8392 commit 23fdad5
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 38 deletions.
12 changes: 11 additions & 1 deletion lib/shared/src/auth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,15 @@ export interface EnterpriseUserDotComError {
enterprise: string
}

export type AuthenticationError = NetworkAuthError | InvalidAccessTokenError | EnterpriseUserDotComError
export interface AuthConfigError extends AuthenticationErrorMessage {
type: 'auth-config-error'
}

export type AuthenticationError =
| NetworkAuthError
| InvalidAccessTokenError
| EnterpriseUserDotComError
| AuthConfigError

export interface AuthenticationErrorMessage {
title?: string
Expand Down Expand Up @@ -90,6 +98,8 @@ export function getAuthErrorMessage(error: AuthenticationError): AuthenticationE
"in through your organization's enterprise instance instead. If you need assistance " +
'please contact your Sourcegraph admin.',
}
case 'auth-config-error':
return error
}
}

Expand Down
1 change: 1 addition & 0 deletions lib/shared/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type TokenSource = 'redirect' | 'paste'
export interface AuthCredentials {
serverEndpoint: string
credentials: HeaderCredential | TokenCredential | undefined
error?: any
}

export interface HeaderCredential {
Expand Down
73 changes: 46 additions & 27 deletions lib/shared/src/configuration/auth-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,38 +64,57 @@ export async function resolveAuth(
const { authExternalProviders, overrideServerEndpoint, overrideAuthToken } = configuration
const serverEndpoint = normalizeServerEndpointURL(overrideServerEndpoint || endpoint)

if (overrideAuthToken) {
return { credentials: { token: overrideAuthToken }, serverEndpoint }
}

const credentials = await getExternalProviderAuthResult(serverEndpoint, authExternalProviders).catch(
error => {
throw new Error(`Failed to execute external auth command: ${error}`)
try {
if (overrideAuthToken) {
return { credentials: { token: overrideAuthToken }, serverEndpoint }
}
)

if (credentials) {
return {
credentials: {
expiration: credentials?.expiration,
getHeaders() {
return credentials.headers
},
},
const credentials = await getExternalProviderAuthResult(
serverEndpoint,
authExternalProviders
).catch(error => {
throw new Error(`Failed to execute external auth command: ${error.message || error}`)
})

if (credentials) {
if (credentials?.expiration) {
const expirationMs = credentials?.expiration * 1000
const expireInMs = expirationMs - Date.now()
if (expireInMs < 0) {
throw new Error(
'Credentials expiration cannot be se to the past date: ' +
`${new Date(expirationMs)} (${credentials.expiration})`
)
}
}
return {
credentials: {
expiration: credentials?.expiration,
getHeaders() {
return credentials.headers
},
},
serverEndpoint,
}
}
}

const token = await clientSecrets.getToken(serverEndpoint).catch(error => {
throw new Error(
`Failed to get access token for endpoint ${serverEndpoint}: ${error.message || error}`
)
})
const token = await clientSecrets.getToken(serverEndpoint).catch(error => {
throw new Error(
`Failed to get access token for endpoint ${serverEndpoint}: ${error.message || error}`
)
})

return {
credentials: token
? { token, source: await clientSecrets.getTokenSource(serverEndpoint) }
: undefined,
serverEndpoint,
return {
credentials: token
? { token, source: await clientSecrets.getTokenSource(serverEndpoint) }
: undefined,
serverEndpoint,
}
} catch (error) {
return {
credentials: undefined,
serverEndpoint,
error,
}
}
}
12 changes: 5 additions & 7 deletions lib/shared/src/configuration/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,18 @@ async function resolveConfiguration({
if (cred !== undefined && 'expiration' in cred && cred.expiration !== undefined) {
const expirationMs = cred.expiration * 1000
const expireInMs = expirationMs - Date.now()
if (expireInMs < 0) {
throw new Error(
'Credentials expiration cannot be se to the past date:' +
`${new Date(expirationMs)} (${cred.expiration})`
)
}
setInterval(() => _refreshConfigRequests.next(), expireInMs)
}
return { configuration: clientConfiguration, clientState, auth, isReinstall }
} catch (error) {
// We don't want to throw here, because that would cause the observable to terminate and
// all callers receiving no further config updates.
logError('resolveConfiguration', `Error resolving configuration: ${error}`)
const auth = { credentials: undefined, serverEndpoint }
const auth = {
credentials: undefined,
serverEndpoint,
error: error,
}
return { configuration: clientConfiguration, clientState, auth, isReinstall }
}
}
Expand Down
22 changes: 19 additions & 3 deletions vscode/src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,7 @@ export async function showSignInMenu(
const { configuration } = await currentResolvedConfig()
const auth = await resolveAuth(selectedEndpoint, configuration, secretStorage)

let authStatus = auth.credentials
? await authProvider.validateAndStoreCredentials(auth, 'store-if-valid')
: undefined
let authStatus = await authProvider.validateAndStoreCredentials(auth, 'store-if-valid')

if (!authStatus?.authenticated) {
const token = await showAccessTokenInputBox(selectedEndpoint)
Expand Down Expand Up @@ -406,6 +404,24 @@ export async function validateCredentials(
signal?: AbortSignal,
clientConfig?: CodyClientConfig
): Promise<AuthStatus> {
if (config.auth.error !== undefined) {
logDebug(
'auth',
`Failed to authenticate to ${config.auth.serverEndpoint} due to configuration error`,
config.auth.error
)
return {
authenticated: false,
endpoint: config.auth.serverEndpoint,
pendingValidation: false,
error: {
type: 'auth-config-error',
title: 'Auth config error',
message: config.auth.error?.message ?? config.auth.error,
},
}
}

// An access token is needed except for Cody Web, which uses cookies.
if (!config.auth.credentials && !clientCapabilities().isCodyWeb) {
return { authenticated: false, endpoint: config.auth.serverEndpoint, pendingValidation: false }
Expand Down

0 comments on commit 23fdad5

Please sign in to comment.