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

Support for custom proxy auth schemes #3282

Open
segevfiner opened this issue May 20, 2024 · 7 comments
Open

Support for custom proxy auth schemes #3282

segevfiner opened this issue May 20, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@segevfiner
Copy link

This would solve...

Support custom proxy auth schemes, such as Kerberos/Negotiate.

The implementation should look like...

Some ability to supply a callback or interceptor or anything really that can handle custom authentication schemes to a proxy. Such as Negotiate/Kerberos

I have also considered...

Currently trying to figure out how to do it by myself, but undici is quite complicated to figure out how to do so.

Basically, we need to catch a 407 from the proxy and if the Proxy-Authenticate header includes the scheme we want to handle, then we need to retry the request with a dynamic authorization header, e.g. using the kerberos package, possibly multiple times for a correct implementation.

Additional context

Kind of like what Microsoft hacked together for the stock http/https module here: https://github.com/microsoft/vscode/blob/9c2b9327e5cdce9583ef6cfe5072102b150cf7cb/src/vs/workbench/api/node/proxyResolver.ts#L131-L165 and via @vscode/proxy-agent.

@segevfiner segevfiner added the enhancement New feature or request label May 20, 2024
@segevfiner
Copy link
Author

segevfiner commented May 20, 2024

It would be easiest if ProxyAgent just exposes an appropriate callback for this in this style that I linked in the description. But as I need something right now, I tried kludging stuff around by Dispatcher.compose a RetryHandler into the Pool used to establish the proxy connection via the clientFactory option (As retrying the request by myself via the dispatch mechanism seems difficult, but there is likely a much simpler way to do this for this case, as I don't need the entirety of the RetryHandler, just the ability to retry the request)

Sadly it doesn't handle onUpgrade so I also add to extend it to handle 407 and error appropriately. I naively tried to compose a handler after the RetryHandler to throw an error but that causes RetryHandler to abort and not retry, so had to extend it.

I'll post my code here for reference once I have cleaned it up. Hopefully it will serve as inspiration on how to actually implement this correctly.

P.S. I have hit a lot of missing types (TypeScript):

  • clientFactory for ProxyAgent
  • errors.RequestRetryError
  • opts.servername
  • RetryHandler.abort
  • RetryHandler.retryOpts
  • RetryHandler.retryCount
  • RetryHandler.handler
  • Whether you extend DecoratorHandler or any other handler, TypeScript doesn't recognize any of the methods that you can override, or that you can call them via super, e.g. onUpgrade, onHeaders.

@segevfiner
Copy link
Author

segevfiner commented May 21, 2024

Here is what I managed to cobble together so far:

First version
import stream from 'node:stream';
import { Dispatcher, EnvHttpProxyAgent, Pool, RetryHandler, errors, setGlobalDispatcher, util } from 'undici';
import { IncomingHttpHeaders } from 'undici/types/header';

class RetryHandler2 extends RetryHandler {
  declare retryCount: number;
  declare retryOpts: Required<RetryHandler.RetryOptions>;
  declare handler: Required<Dispatcher.DispatchHandlers>;
  declare abort: (err?: Error) => void;

  onUpgrade(statusCode: number, headers: Buffer[] | string[] | null, socket: stream.Duplex) {
    this.retryCount++;

    if (statusCode >= 300) {
      if (this.retryOpts.statusCodes.includes(statusCode) === false) {
        return this.handler.onUpgrade(statusCode, headers, socket);
      } else {
        this.abort(
          // @ts-expect-error Missing types
          // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
          new errors.RequestRetryError('Request failed', statusCode, {
            headers,
            count: this.retryCount,
          })
        );
        return false;
      }
    }

    // @ts-expect-error Missing types
    // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call
    return super.onUpgrade(statusCode, headers, socket);
  }
}

setGlobalDispatcher(
  // XXX Will emit an experimental warning
  new EnvHttpProxyAgent({
    // @ts-expect-error clientFactory missing in types
    clientFactory: (origin: URL, opts: object): Dispatcher =>
      new Pool(origin, opts).compose([
        (dispatch) => {
          return function interceptedDispatch(opts, handler) {
            return dispatch(
              opts,
              new RetryHandler2(
                {
                  ...opts,
                  retryOptions: {
                    retry: function (err, state, callback) {
                      // @ts-expect-error Missing types
                      // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
                      if (err instanceof errors.RequestRetryError && err.statusCode === 407) {
                        // @ts-expect-error Missing types
                        // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
                        const headers = util.parseHeaders(err.headers);
                        const header = headers['proxy-authenticate'];
                        const authenticate = Array.isArray(header)
                          ? header
                          : typeof header === 'string'
                          ? [header]
                          : [];
                        if (authenticate.some((a) => /^(Negotiate|Kerberos)( |$)/i.test(a))) {
                          import('kerberos')
                            .then(async ({ default: kerberos }) => {
                              const spn =
                                // @ts-expect-error Missing types
                                // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
                                process.platform === 'win32' ? `HTTP/${opts.servername}` : `HTTP@${opts.servername}`;
                              const client = await kerberos.initializeClient(spn);
                              const response = await client.step('');
                              (state.opts.headers as IncomingHttpHeaders)['proxy-authorization'] =
                                'Negotiate ' + response;
                              callback(null);
                            })
                            .catch(callback);
                          return null;
                        }
                      }

                      callback(err);
                      return null;
                    },
                    maxRetries: 1,
                    statusCodes: [407],
                    errorCodes: [],
                    retryAfter: false,
                  },
                },
                {
                  handler,
                  dispatch,
                }
              )
            );
          };
        },
      ]),
  })
);
Second version
// #region RetryHandler2
class RetryHandler2 extends RetryHandler {
  declare retryCount: number;
  declare retryOpts: Required<RetryHandler.RetryOptions>;
  declare handler: Required<Dispatcher.DispatchHandlers>;
  declare abort: (err?: Error) => void;

  onUpgrade(statusCode: number, headers: Buffer[] | string[] | null, socket: stream.Duplex) {
    this.retryCount++;

    if (statusCode >= 300) {
      if (this.retryOpts.statusCodes.includes(statusCode) === false) {
        return this.handler.onUpgrade(statusCode, headers, socket);
      } else {
        this.abort(
          // @ts-expect-error Missing types for errors.RequestRetryError
          // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
          new errors.RequestRetryError('Request failed', statusCode, {
            headers,
            count: this.retryCount,
          })
        );
        return false;
      }
    }

    // @ts-expect-error Missing types for RetryHandler actually implemented onUpgrade
    // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call
    return super.onUpgrade(statusCode, headers, socket);
  }
}
// //#endregion RetryHandler2

function kerberosInterceptor(dispatch: Dispatcher['dispatch']): Dispatcher['dispatch'] {
  return function kerberosInterceptor(opts, handler) {
    // TODO We can try to cache the Proxy-Authenticate header like we do in
    // lookupProxyAuthorization, and pre-emptively send Negotiate rather than
    // get hit with a 407 each time

    return dispatch(
      opts,
      new RetryHandler2(
        {
          ...opts,
          retryOptions: {
            retry: function (err, state, callback) {
              // @ts-expect-error Missing types for errors.RequestRetryError
              // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
              if (err instanceof errors.RequestRetryError && err.statusCode === 407) {
                // @ts-expect-error Missing types for errors.RequestRetryError
                // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
                const headers = util.parseHeaders(err.headers);
                const header = headers['proxy-authenticate'];
                const authenticate = Array.isArray(header) ? header : typeof header === 'string' ? [header] : [];
                if (authenticate.some((a) => /^(Negotiate|Kerberos)( |$)/i.test(a))) {
                  import('kerberos')
                    .then(async ({ default: kerberos }) => {
                      const spn =
                        // @ts-expect-error Missing types for opts.servername
                        // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
                        process.platform === 'win32' ? `HTTP/${opts.servername}` : `HTTP@${opts.servername}`;
                      // TODO You are technically supposed to pass mechOID based on whether it is Negotiate or Kerberos
                      const client = await kerberos.initializeClient(spn);
                      // TODO When falling back to NTLM in Negotitate, you need multiple steps
                      const response = await client.step('');
                      (state.opts.headers as IncomingHttpHeaders)['proxy-authorization'] = 'Negotiate ' + response;
                      callback(null);
                    })
                    .catch(callback);
                  return null;
                }
              }

              callback(err);
              return null;
            },
            maxRetries: 1,
            statusCodes: [407],
            errorCodes: [],
            retryAfter: false,
          },
        },
        {
          handler,
          dispatch,
        }
      )
    );
  };
}

@metcoder95
Copy link
Member

Nice work, I believe this can live as a standalone npm package that exposes this interceptor.

@segevfiner
Copy link
Author

segevfiner commented May 22, 2024

Yeah. But the code feels a bit kludgey for now... Maybe a different kind of handler than RetryHandler2 to handle this, so it's cleaner? Like an AuthenticationHandler, that one could possibly live in undici? with the actual callback that does authentication being in a separate npm package? I kinda used RetryHandler like that cause I wasn't sure how to implement my own handler that does retries when writing it, but it should be possible to create a more condescend one to only handle 401/407. But one thing I'm not sure about is keeping per request state in such handlers, does a handler not get called concurrently if there are multiple async requests going on at the same time? Wouldn't that mess up the state the handler keeps on this such as retryCount?

@metcoder95
Copy link
Member

I can see an interceptor for authentication, but it will require to be generic rather than scoped to an specific technology.

You can customize RetryHandler as you need 👍

And the handlers are made to be scoped per request, so each handler should preserve the state of a single request; the same applies to the retry handler, the request that triggered the handler will be scoped to it, if a subsequent request is made another handler will be instantiated.

@segevfiner
Copy link
Author

Yeah, the idea is a generic authentication handler that is like retry handler but only for handling 401 or 407 that receives a callback for the actual handling of the auth method.

@metcoder95
Copy link
Member

Got it, a mix of Retry and Proxy-Like agent; it could be a good package, not so sure of undici core is the place for that.

It can make usage of the RetryHandler extending it as you provided in your examples; possibly extending the handler to call the retry callback with the dispatch opts so it can overwrite it when it detects a 401 or 407 (tho I'd focus lonely in 407 as 401 is something that should be known ahead of time)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants