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.

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

Improve reporting auth errors #6639

Merged
merged 4 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ sealed class AuthenticationError {
"network-error" -> context.deserialize<NetworkAuthError>(element, NetworkAuthError::class.java)
"invalid-access-token" -> context.deserialize<InvalidAccessTokenError>(element, InvalidAccessTokenError::class.java)
"enterprise-user-logged-into-dotcom" -> context.deserialize<EnterpriseUserDotComError>(element, EnterpriseUserDotComError::class.java)
"auth-config-error" -> context.deserialize<AuthConfigError>(element, AuthConfigError::class.java)
else -> throw Exception("Unknown discriminator ${element}")
}
}
Expand Down Expand Up @@ -50,3 +51,14 @@ data class EnterpriseUserDotComError(
}
}

data class AuthConfigError(
val title: String? = null,
val message: String,
val type: TypeEnum, // Oneof: auth-config-error
) : AuthenticationError() {

enum class TypeEnum {
@SerializedName("auth-config-error") `Auth-config-error`,
}
}

Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
@file:Suppress("FunctionName", "ClassName", "unused", "EnumEntryName", "UnusedImport")
package com.sourcegraph.cody.agent.protocol_generated;

import com.google.gson.annotations.SerializedName;

data class CodyContextFilterItem(
val repoNamePattern: String,
val repoNamePattern: RepoNamePatternEnum, // Oneof: .*
val filePathPatterns: List<String>? = null,
)
) {

enum class RepoNamePatternEnum {
@SerializedName(".*") Wildcard,
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package com.sourcegraph.cody.agent.protocol_generated;

object Constants {
const val Wildcard = ".*"
const val Applied = "Applied"
const val Applying = "Applying"
const val Automatic = "Automatic"
Expand All @@ -15,6 +16,7 @@ object Constants {
const val agentic = "agentic"
const val ask = "ask"
const val assistant = "assistant"
const val `auth-config-error` = "auth-config-error"
const val authenticated = "authenticated"
const val autocomplete = "autocomplete"
const val balanced = "balanced"
Expand Down
Empty file modified agent/scripts/reverse-proxy.py
100644 → 100755
Empty file.
21 changes: 21 additions & 0 deletions agent/scripts/simple-external-auth-provider.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env python3
import json
import time
from datetime import datetime

def generate_credentials():
current_epoch = int(time.time()) + 100

credentials = {
"headers": {
"Authorization": "Bearer SomeUser",
"Expiration": current_epoch,
},
"expiration": current_epoch
}

# Print JSON to stdout
print(json.dumps(credentials))

if __name__ == "__main__":
generate_credentials()
2 changes: 1 addition & 1 deletion agent/src/cli/scip-codegen/emitters/KotlinEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export class KotlinFormatter extends Formatter {
}

override formatFieldName(name: string): string {
const escaped = name.replace(':', '_').replace('/', '_')
const escaped = name.replace('.*', 'Wildcard').replace(':', '_').replace('/', '_')
const isKeyword = this.options.reserved.has(escaped)
const needsBacktick = isKeyword || !/^[a-zA-Z0-9_]+$/.test(escaped)
// Replace all non-alphanumeric characters with underscores
Expand Down
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
69 changes: 67 additions & 2 deletions lib/shared/src/configuration/auth-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ describe('auth-resolver', () => {
})

test('resolve custom auth provider', async () => {
const futureEpoch = Date.UTC(2050) / 1000
const credentialsJson = JSON.stringify({
headers: { Authorization: 'token X' },
expiration: 1337,
expiration: futureEpoch,
})

const auth = await resolveAuth(
Expand Down Expand Up @@ -91,11 +92,75 @@ describe('auth-resolver', () => {
expect(auth.serverEndpoint).toBe('https://my-server.com/')

const headerCredential = auth.credentials as HeaderCredential
expect(headerCredential.expiration).toBe(1337)
expect(headerCredential.expiration).toBe(futureEpoch)
expect(headerCredential.getHeaders()).toStrictEqual({
Authorization: 'token X',
})

expect(JSON.stringify(headerCredential)).not.toContain('token X')
})

test('resolve custom auth provider error handling - bad JSON', async () => {
const auth = await resolveAuth(
'sourcegraph.com',
{
authExternalProviders: [
{
endpoint: 'https://my-server.com',
executable: {
commandLine: ['echo x'],
shell: isWindows() ? process.env.ComSpec : '/bin/bash',
timeout: 5000,
windowsHide: true,
},
},
],
overrideServerEndpoint: 'https://my-server.com',
overrideAuthToken: undefined,
},
new TempClientSecrets(new Map())
)

expect(auth.serverEndpoint).toBe('https://my-server.com/')

expect(auth.credentials).toBe(undefined)
expect(auth.error.message).toContain('Failed to execute external auth command: Unexpected token')
})

test('resolve custom auth provider error handling - bad expiration', async () => {
const expiredEpoch = Date.UTC(2020) / 1000
const credentialsJson = JSON.stringify({
headers: { Authorization: 'token X' },
expiration: expiredEpoch,
})

const auth = await resolveAuth(
'sourcegraph.com',
{
authExternalProviders: [
{
endpoint: 'https://my-server.com',
executable: {
commandLine: [
isWindows() ? `echo ${credentialsJson}` : `echo '${credentialsJson}'`,
],
shell: isWindows() ? process.env.ComSpec : '/bin/bash',
timeout: 5000,
windowsHide: true,
},
},
],
overrideServerEndpoint: 'https://my-server.com',
overrideAuthToken: undefined,
},
new TempClientSecrets(new Map())
)

expect(auth.serverEndpoint).toBe('https://my-server.com/')

expect(auth.credentials).toBe(undefined)
expect(auth.error.message).toContain(
'Credentials expiration cannot be set to a date in the past'
)
})
})
72 changes: 45 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,56 @@ 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
if (expirationMs < Date.now()) {
throw new Error(
'Credentials expiration cannot be set to a date in the past: ' +
`${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,
}
}
}
15 changes: 6 additions & 9 deletions lib/shared/src/configuration/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,19 @@ async function resolveConfiguration({
const auth = await resolveAuth(serverEndpoint, clientConfiguration, clientSecrets)
const cred = auth.credentials
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})`
)
}
const expireInMs = cred.expiration * 1000 - Date.now()
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
Loading