Revert "Improve reporting auth errors (#6639)"

This reverts commit 1c16f35.
sourcegraph · Jan 24, 2025 · 70adef1 · 70adef1
1 parent a1f5eee
commit 70adef1
Show file tree

Hide file tree

Showing 12 changed files with 45 additions and 194 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,7 +16,6 @@ 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}")
         }
       }
@@ -51,14 +50,3 @@ 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,15 +1,8 @@
 @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: RepoNamePatternEnum, // Oneof: .*
+  val repoNamePattern: String,
   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,7 +2,6 @@
 package com.sourcegraph.cody.agent.protocol_generated;
 
 object Constants {
-  const val Wildcard = ".*"
   const val Applied = "Applied"
   const val Applying = "Applying"
   const val Automatic = "Automatic"
@@ -16,7 +15,6 @@ 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
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('.*', 'Wildcard').replace(':', '_').replace('/', '_')
+        const escaped = name.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,15 +62,7 @@ export interface EnterpriseUserDotComError {
     enterprise: string
 }
 
-export interface AuthConfigError extends AuthenticationErrorMessage {
-    type: 'auth-config-error'
-}
-
-export type AuthenticationError =
-    | NetworkAuthError
-    | InvalidAccessTokenError
-    | EnterpriseUserDotComError
-    | AuthConfigError
+export type AuthenticationError = NetworkAuthError | InvalidAccessTokenError | EnterpriseUserDotComError
 
 export interface AuthenticationErrorMessage {
     title?: string
@@ -98,8 +90,6 @@ 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,7 +18,6 @@ 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,10 +61,9 @@ describe('auth-resolver', () => {
     })
 
     test('resolve custom auth provider', async () => {
-        const futureEpoch = Date.UTC(2050) / 1000
         const credentialsJson = JSON.stringify({
             headers: { Authorization: 'token X' },
-            expiration: futureEpoch,
+            expiration: 1337,
         })
 
         const auth = await resolveAuth(
@@ -92,75 +91,11 @@ describe('auth-resolver', () => {
         expect(auth.serverEndpoint).toBe('https://my-server.com/')
 
         const headerCredential = auth.credentials as HeaderCredential
-        expect(headerCredential.expiration).toBe(futureEpoch)
+        expect(headerCredential.expiration).toBe(1337)
         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,56 +64,38 @@ export async function resolveAuth(
     const { authExternalProviders, overrideServerEndpoint, overrideAuthToken } = configuration
     const serverEndpoint = normalizeServerEndpointURL(overrideServerEndpoint || endpoint)
 
-    try {
-        if (overrideAuthToken) {
-            return { credentials: { token: overrideAuthToken }, serverEndpoint }
-        }
+    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.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 credentials = await getExternalProviderAuthResult(serverEndpoint, authExternalProviders).catch(
+        error => {
+            throw new Error(`Failed to execute external auth command: ${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,
-        }
-    } catch (error) {
+    if (credentials) {
         return {
-            credentials: undefined,
+            credentials: {
+                expiration: credentials?.expiration,
+                getHeaders() {
+                    return credentials.headers
+                },
+            },
             serverEndpoint,
-            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,
+    }
 }
diff --git a/lib/shared/src/configuration/resolver.ts b/lib/shared/src/configuration/resolver.ts
@@ -96,19 +96,22 @@ async function resolveConfiguration({
         const auth = await resolveAuth(serverEndpoint, clientConfiguration, clientSecrets)
         const cred = auth.credentials
         if (cred !== undefined && 'expiration' in cred && cred.expiration !== undefined) {
-            const expireInMs = cred.expiration * 1000 - Date.now()
+            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,
-            error: error,
-        }
+        const auth = { credentials: undefined, serverEndpoint }
         return { configuration: clientConfiguration, clientState, auth, isReinstall }
     }
 }

diff --git a/vscode/src/auth/auth.ts b/vscode/src/auth/auth.ts
@@ -90,7 +90,9 @@ export async function showSignInMenu(
             const { configuration } = await currentResolvedConfig()
             const auth = await resolveAuth(selectedEndpoint, configuration, secretStorage)
 
-            let authStatus = await authProvider.validateAndStoreCredentials(auth, 'store-if-valid')
+            let authStatus = auth.credentials
+                ? await authProvider.validateAndStoreCredentials(auth, 'store-if-valid')
+                : undefined
 
             if (!authStatus?.authenticated) {
                 const token = await showAccessTokenInputBox(selectedEndpoint)
@@ -404,24 +406,6 @@ 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 }