Authorization error redirect drops error, error_description and state when the redirect URI has a query string #516

Specify your setup

• Operating System: macOS 27.0.1
• Node version: v26.9.0
• npm version: 11.19.1
• version of @node-oauth/oauth2-server: 5.3.1, also on master (8af3d2d)
• which OAuth2 workflow: Authorization Code Grant
• at which workflow step does the error occur: (C) of RFC 6749 §4.1.1, the authorization error response redirected back to the client (§4.1.2.1)

Describe the bug

If the client's registered redirect URI already has a query component (for example https://example.com/cb?foo=bar), the error redirect from AuthorizeHandler has none of the error parameters. The client is sent to https://example.com/cb?foo=bar with no error, error_description or state. A redirect URI without a query gets all three. The success redirect is unaffected.

The cause, from reading lib/handlers/authorize-handler.js: buildErrorRedirectUri() parses the URI with url.parse(redirectUri) and sets uri.query = { error, error_description }, but leaves uri.search as the parser set it ('?foo=bar'). updateResponse() then adds state to uri.query and calls url.format(uri), and url.format() uses search when it's a string, ignoring query:

const url = require('url');
url.format({ protocol: 'https:', slashes: true, host: 'example.com', pathname: '/cb',
  search: '?foo=bar', query: { error: 'access_denied', state: 'xyz' } });
// 'https://example.com/cb?foo=bar'

CodeResponseType#buildRedirectUri() avoids this by setting uri.search = null after adding code to the parsed query, which is why successful redirects keep both the client's parameters and code / state.

To Reproduce

const { Request, Response } = require('@node-oauth/oauth2-server');
const AuthorizeHandler = require('@node-oauth/oauth2-server/lib/handlers/authorize-handler');

async function run(redirectUri) {
  const client = { id: 'c1', grants: ['authorization_code'], redirectUris: [redirectUri] };
  const model = {
    getAccessToken: async () => ({ client, user: {}, accessTokenExpiresAt: new Date(Date.now() + 10000) }),
    getClient: async () => ({ ...client }),
    saveAuthorizationCode: async () => ({ authorizationCode: 'code123', client }),
  };
  const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model });
  const request = new Request({
    body: { client_id: 'c1', response_type: 'code', redirect_uri: redirectUri, allowed: 'false' },
    headers: { Authorization: 'Bearer t' }, method: {}, query: { state: 'xyz' },
  });
  const response = new Response({ body: {}, headers: {} });
  await handler.handle(request, response).catch(() => {});
  console.log(response.get('location'));
}

run('https://example.com/cb');
// https://example.com/cb?error=access_denied&error_description=Access%20denied%3A%20user%20denied%20access%20to%20application&state=xyz
run('https://example.com/cb?foo=bar');
// https://example.com/cb?foo=bar

Any error that reaches buildErrorRedirectUri() behaves the same way. Here it's the user's denial (allowed: 'false').

Expected behavior

https://example.com/cb?foo=bar&error=access_denied&error_description=…&state=xyz. RFC 6749 §4.1.2.1 adds error (required), error_description (optional) and state (required if the request had one) to the query component of the redirection URI. §3.1.2 says the redirection endpoint URI's query component "MUST be retained when adding additional query parameters". So both the client's own parameters and the error parameters should be there.

Additional context

A fix that keeps the client's parameters, mirroring CodeResponseType#buildRedirectUri(): parse the query into an object, add the error fields to it, and clear search, so that url.format() serializes query.

I left this out of #515 on purpose. That PR only replaces url.parse() (#513) and keeps the redirect output identical, including this case. I'm happy to send a follow-up PR with a test for this, once #515 is settled, or fold it into #515 if you prefer.