fix: align auth failure error messages for oauth

In the Typescript CLI, when OAuth fails due to an invalid or expired
token, throw the same "missing auth" error that one would get if not
logged in (no auth token).

The resulting error message will prompt the user to re-authenticate.

This has to be mediated by the legacycli http proxy because the
Typescript side of the CLI is not involved in the OAuth flow at all; it
is handled transparently by the go-application-framework's networking
middleware.

A special HTTP request and response header is used by the legacycli
proxy to indicate whether that middleware auth has failed. If it has,
the functions which perform HTTP client requests with needle will throw
an error that indicates "unauthenticated", resulting in an experience
consistent with the legacy token auth flow in the CLI.

CLI-392

cliv2/internal/proxy/proxy.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"log"
 	"net"
@@ -18,6 +19,7 @@ import (
 	pkg_utils "github.com/snyk/go-application-framework/pkg/utils"
 
 	"github.com/snyk/go-application-framework/pkg/networking/certs"
+	"github.com/snyk/go-application-framework/pkg/networking/middleware"
 	"github.com/snyk/go-httpauth/pkg/httpauth"
 
 	"github.com/snyk/cli/cliv2/internal/constants"
@@ -143,9 +145,28 @@ func (p *WrapperProxy) ProxyInfo() *ProxyInfo {
 	}
 }
 
+// headerSnykAuthFailed is used to indicate there was a failure to establish
+// authorization in a legacycli proxied HTTP request and response.
+//
+// The request header is used to propagate this indication from
+// NetworkAccess.AddHeaders all the way through proxy middleware into the
+// response.
+//
+// The response header is then used by the Typescript CLI to surface an
+// appropriate authentication failure error back to the user.
+//
+// These layers of indirection are necessary because the Typescript CLI is not
+// involved in OAuth authentication at all, but needs to know that an auth
+// failure specifically occurred. HTTP status and error catalog codes aren't
+// adequate for this purpose because there are non-authentication reasons an API
+// request might 401 or 403, such as permissions or entitlements.
+const headerSnykAuthFailed = "snyk-auth-failed"
+
 func (p *WrapperProxy) replaceVersionHandler(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
-	err := p.addHeaderFunc(r)
-	if err != nil {
+	if err := p.addHeaderFunc(r); err != nil {
+		if errors.Is(err, middleware.ErrAuthenticationFailed) {
+			r.Header.Set(headerSnykAuthFailed, "true")
+		}
 		p.DebugLogger.Printf("Failed to add header: %s", err)
 	}
 
@@ -177,6 +198,12 @@ func (p *WrapperProxy) Start() error {
 	proxy.Logger = log.New(&pkg_utils.ToZeroLogDebug{Logger: p.DebugLogger}, "", 0)
 	proxy.OnRequest().DoFunc(p.replaceVersionHandler)
 	proxy.OnRequest().HandleConnect(p)
+	proxy.OnResponse().DoFunc(func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response {
+		if authFailed := resp.Request.Header.Get(headerSnykAuthFailed); authFailed != "" {
+			resp.Header.Set(headerSnykAuthFailed, authFailed)
+		}
+		return resp
+	})
 	proxy.Verbose = true
 	proxyServer := &http.Server{
 		Handler: proxy,

cliv2/internal/proxy/proxy_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/snyk/go-application-framework/pkg/configuration"
 	"github.com/snyk/go-application-framework/pkg/networking/certs"
+	"github.com/snyk/go-application-framework/pkg/networking/middleware"
 	gafUtils "github.com/snyk/go-application-framework/pkg/utils"
 	"github.com/snyk/go-httpauth/pkg/httpauth"
 
@@ -256,3 +257,39 @@ func Test_appendExtraCaCert(t *testing.T) {
 	// cleanup
 	os.Remove(file.Name())
 }
+
+func Test_proxyPropagatesAuthFailureHeader(t *testing.T) {
+	basecache := "testcache"
+	version := "1.1.1"
+	config := configuration.NewInMemory()
+	config.Set(configuration.CACHE_PATH, basecache)
+	config.Set(configuration.INSECURE_HTTPS, false)
+
+	setup(t, basecache, version)
+	defer teardown(t, basecache)
+
+	wp, err := proxy.NewWrapperProxy(config, version, &debugLogger)
+	assert.Nil(t, err)
+	wp.SetHeaderFunction(func(r *http.Request) error {
+		// Simulate a wrapped authentication failure, such as oauth refresh.
+		return fmt.Errorf("nope: %w", middleware.ErrAuthenticationFailed)
+	})
+
+	err = wp.Start()
+	assert.Nil(t, err)
+
+	useProxyAuth := true
+	proxiedClient, err := helper_getHttpClient(wp, useProxyAuth)
+	assert.Nil(t, err)
+
+	res, err := proxiedClient.Get("https://static.snyk.io/cli/latest/version")
+	assert.Nil(t, err)
+	// Assert that the proxy propagates the auth failed marker header to the response.
+	assert.Equal(t, res.Header.Get("snyk-auth-failed"), "true")
+
+	wp.Close()
+
+	// assert cert file is deleted on Close
+	_, err = os.Stat(wp.CertificateLocation)
+	assert.NotNil(t, err) // this means the file is gone
+}

src/lib/ecosystems/resolve-test-facts.ts

@@ -43,9 +43,14 @@ export async function resolveAndTestFacts(
   try {
     return await resolveAndTestFactsUnmanagedDeps(scans, options);
   } catch (error) {
-    const unauthorized = error.code === 401 || error.code === 403;
-
-    if (unauthorized) {
+    const unauthorizedErrorCode = error.code === 401 || error.code === 403;
+    const missingApiToken = error.isMissingApiToken;
+
+    // Decide if the error is an authorization error other than being
+    // unauthenticated (missing or invalid API token). An account lacking
+    // permission, for example.
+    const otherUnauthorized = unauthorizedErrorCode && !missingApiToken;
+    if (otherUnauthorized) {
       throw AuthFailedError(
         'Unauthorized request to unmanaged service',
         error.code,

src/lib/errors/missing-api-token.ts

@@ -6,6 +6,17 @@ export class MissingApiTokenError extends CustomError {
   private static ERROR_MESSAGE =
     '`snyk` requires an authenticated account. Please run `snyk auth` and try again.';
 
+  /**
+   * isMissingApiToken returns whether the error instance is a missing API token
+   * error.
+   *
+   * Defined as a property so that the same expression resolves as "falsy"
+   * (undefined) when other error types are tested.
+   */
+  public get isMissingApiToken(): boolean {
+    return this.strCode === MissingApiTokenError.ERROR_STRING_CODE;
+  }
+
   constructor() {
     super(MissingApiTokenError.ERROR_MESSAGE);
     this.code = MissingApiTokenError.ERROR_CODE;

src/lib/request/constants.ts

@@ -0,0 +1 @@
+export const headerSnykAuthFailed = 'snyk-auth-failed';

src/lib/request/promise.ts

@@ -1,4 +1,6 @@
 import { getAuthHeader } from '../api-token';
+import { MissingApiTokenError } from '../errors';
+import { headerSnykAuthFailed } from './constants';
 import * as request from './index';
 
 export async function makeRequest<T>(payload: any): Promise<T> {
@@ -36,6 +38,9 @@ export async function makeRequestRest<T>(payload: any): Promise<T> {
       if (error) {
         return reject(error);
       }
+      if (res?.headers?.[headerSnykAuthFailed] === 'true') {
+        return reject(new MissingApiTokenError());
+      }
       if (res.statusCode === 400) {
         return reject({
           code: res.statusCode,

src/lib/request/request.ts

@@ -12,6 +12,8 @@ import { getVersion } from '../version';
 import * as https from 'https';
 import * as http from 'http';
 import { jsonStringifyLargeObject } from '../json';
+import { MissingApiTokenError } from '../errors';
+import { headerSnykAuthFailed } from './constants';
 
 const debug = debugModule('snyk:req');
 const snykDebug = debugModule('snyk');
@@ -143,6 +145,9 @@ export async function makeRequest(
 
   return new Promise((resolve, reject) => {
     needle.request(method, url, data, options, (err, res, respBody) => {
+      if (res?.headers?.[headerSnykAuthFailed] === 'true') {
+        return reject(new MissingApiTokenError());
+      }
       // respBody potentially very large, do not output it in debug
       debug('response (%s)', (res || {}).statusCode);
       if (err) {
@@ -173,7 +178,10 @@ export async function streamRequest(
 
 async function getStatusCode(stream: needle.ReadableStream): Promise<number> {
   return new Promise((resolve, reject) => {
-    stream.on('header', (statusCode: number) => {
+    stream.on('header', (statusCode: number, headers: any) => {
+      if (headers?.[headerSnykAuthFailed] === 'true') {
+        return reject(new MissingApiTokenError());
+      }
       resolve(statusCode);
     });
     stream.on('err', (err: Error) => {

test/jest/unit/lib/ecosystems/resolve-test-facts.oauth.spec.ts

@@ -0,0 +1,39 @@
+const mockMakeRequest = jest.fn();
+
+import { Options } from '../../../../../src/lib/types';
+import { MissingApiTokenError } from '../../../../../src/lib/errors';
+
+import { resolveAndTestFacts } from '../../../../../src/lib/ecosystems/resolve-test-facts';
+
+jest.mock('../../../../../src/lib/request/request', () => {
+  return {
+    makeRequest: mockMakeRequest,
+  };
+});
+
+describe('oauth failure', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('rethrows same error for missing api token', async () => {
+    mockMakeRequest.mockRejectedValue(new MissingApiTokenError());
+    await expect(resolveAndTestFacts('cpp', {}, {} as Options)).rejects.toThrow(
+      expect.objectContaining({
+        message:
+          '`snyk` requires an authenticated account. Please run `snyk auth` and try again.',
+      }),
+    );
+  });
+
+  it('rethrows general error for other api auth failures', async () => {
+    const err: any = new Error('nope');
+    err.code = 403;
+    mockMakeRequest.mockRejectedValue(err);
+    await expect(resolveAndTestFacts('cpp', {}, {} as Options)).rejects.toThrow(
+      expect.objectContaining({
+        message: 'Unauthorized request to unmanaged service',
+      }),
+    );
+  });
+});

test/jest/unit/lib/request/request.spec.ts

@@ -0,0 +1,30 @@
+const mockNeedleRequest = jest.fn();
+
+import { makeRequest } from '../../../../../src/lib/request/request';
+import { Payload } from '../../../../../src/lib/request/types';
+
+jest.mock('needle', () => {
+  return {
+    request: mockNeedleRequest,
+  };
+});
+
+describe('needle header auth failed', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('throws missing api token on auth failed marker header', async () => {
+    mockNeedleRequest.mockImplementation((method, url, data, options, fn) => {
+      fn(null, { headers: { 'snyk-auth-failed': 'true' } }, {});
+    });
+    await expect(
+      makeRequest({ url: 'https://example.com' } as Payload),
+    ).rejects.toThrow(
+      expect.objectContaining({
+        message:
+          '`snyk` requires an authenticated account. Please run `snyk auth` and try again.',
+      }),
+    );
+  });
+});