Improve reporting auth errors (#6639)

Fixes https://linear.app/sourcegraph/issue/CODY-4648 ## Changes This PR improves error reporting if auth configuration is broken. ![image](https://github.com/user-attachments/assets/96766d33-6070-4bf0-95ef-82a350a9a16d) ## Test plan 1. Configure custom auth provider as described in #6526 2. Change config to be invalid, e.g. in the `commandLine` change `echo` to `echox` and save a config. 3. You should get logged out and see a detailed error message describing what is wrong. 4. Fix the issue and save the config, error should disappear and you should be logged in back.
sourcegraph · Jan 15, 2025 · 1c16f35 · 1c16f35
1 parent c0f9abd
commit 1c16f35
Show file tree

Hide file tree

Showing 12 changed files with 194 additions and 45 deletions.
diff --git a/.../lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt b/.../lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/AuthenticationError.kt
@@ -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}")
         }
       }
@@ -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`,
+  }
+}
+
diff --git a/...ib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/CodyContextFilterItem.kt b/...ib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/CodyContextFilterItem.kt
@@ -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,
+  }
+}
 
diff --git a/...ngs/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt b/...ngs/kotlin/lib/src/main/kotlin/com/sourcegraph/cody/agent/protocol_generated/Constants.kt
@@ -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"
@@ -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"

diff --git a/agent/scripts/reverse-proxy.py b/agent/scripts/reverse-proxy.py
diff --git a/agent/scripts/simple-external-auth-provider.py b/agent/scripts/simple-external-auth-provider.py
@@ -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()
diff --git a/agent/src/cli/scip-codegen/emitters/KotlinEmitter.ts b/agent/src/cli/scip-codegen/emitters/KotlinEmitter.ts
@@ -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

diff --git a/lib/shared/src/auth/types.ts b/lib/shared/src/auth/types.ts
@@ -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
@@ -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
     }
 }
 

diff --git a/lib/shared/src/configuration.ts b/lib/shared/src/configuration.ts
@@ -18,6 +18,7 @@ export type TokenSource = 'redirect' | 'paste'
 export interface AuthCredentials {
     serverEndpoint: string
     credentials: HeaderCredential | TokenCredential | undefined
+    error?: any
 }
 
 export interface HeaderCredential {

diff --git a/lib/shared/src/configuration/auth-resolver.test.ts b/lib/shared/src/configuration/auth-resolver.test.ts
@@ -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(
@@ -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'
+        )
+    })
 })
diff --git a/lib/shared/src/configuration/auth-resolver.ts b/lib/shared/src/configuration/auth-resolver.ts
@@ -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,
+        }
     }
 }
diff --git a/lib/shared/src/configuration/resolver.ts b/lib/shared/src/configuration/resolver.ts
@@ -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 }
     }
 }

diff --git a/vscode/src/auth/auth.ts b/vscode/src/auth/auth.ts
@@ -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)
@@ -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 }