refactor: remove use of node:url in favour of WHATWG URL

certification/fapi/index.js

@@ -5,7 +5,6 @@ import * as path from 'node:path';
 import * as crypto from 'node:crypto';
 import * as https from 'node:https';
 import { promisify } from 'node:util';
-import { URL } from 'node:url';
 
 import { dirname } from 'desm';
 import * as jose from 'jose';

lib/actions/authorization/interactions.js

@@ -1,5 +1,3 @@
-import * as url from 'node:url';
-
 import upperFirst from '../../helpers/_/upper_first.js';
 import camelCase from '../../helpers/_/camel_case.js';
 import * as ssHandler from '../../helpers/samesite_handler.js';
@@ -129,7 +127,7 @@ export default async function interactions(resumeRouteName, ctx, next) {
     oidc.provider.cookieName('interaction'),
     uid,
     {
-      path: url.parse(destination).pathname,
+      path: new URL(destination, ctx.oidc.issuer).pathname,
       ...cookieOptions,
       maxAge: ttl * 1000,
     },
@@ -141,7 +139,7 @@ export default async function interactions(resumeRouteName, ctx, next) {
     uid,
     {
       ...cookieOptions,
-      path: url.parse(returnTo).pathname,
+      path: new URL(returnTo).pathname,
       domain: undefined,
       httpOnly: true,
       maxAge: ttl * 1000,

lib/actions/authorization/resume.js

@@ -1,5 +1,3 @@
-import * as url from 'node:url';
-
 import upperFirst from '../../helpers/_/upper_first.js';
 import camelCase from '../../helpers/_/camel_case.js';
 import nanoid from '../../helpers/nanoid.js';
@@ -79,7 +77,7 @@ export default async function resumeAction(allowList, resumeRouteName, ctx, next
 
   const clearOpts = {
     ...cookieOptions,
-    path: url.parse(ctx.oidc.urlFor(resumeRouteName, { uid: interactionSession.uid })).pathname,
+    path: new URL(ctx.oidc.urlFor(resumeRouteName, { uid: interactionSession.uid })).pathname,
   };
   ssHandler.set(
     ctx.oidc.cookies,

lib/actions/interaction.js

@@ -1,5 +1,4 @@
 import { strict as assert } from 'node:assert';
-import * as url from 'node:url';
 import * as querystring from 'node:querystring';
 import { inspect } from 'node:util';
 
@@ -34,7 +33,7 @@ your configuration is not in effect');
   /* eslint-enable */
 
   instance(provider).configuration('interactions').url = async function interactionUrl(ctx, interaction) {
-    return url.parse(ctx.oidc.urlFor('interaction', { uid: interaction.uid })).pathname;
+    return new URL(ctx.oidc.urlFor('interaction', { uid: interaction.uid })).pathname;
   };
 
   return {

lib/helpers/client_schema.js

@@ -1,5 +1,3 @@
-import * as url from 'node:url';
-
 import { CLIENT_ATTRIBUTES } from '../consts/index.js';
 
 import * as validUrl from './valid_url.js';
@@ -520,14 +518,13 @@ export default function getSchema(provider) {
       uris.forEach((redirectUri) => {
         let hostname;
         let protocol;
+        let hash;
         try {
-          ({ hostname, protocol } = new URL(redirectUri));
+          ({ hostname, protocol, hash } = new URL(redirectUri));
         } catch (err) {
           this.invalidate(`${label} must only contain valid uris`);
         }
 
-        const { hash } = url.parse(redirectUri);
-
         if (hash) {
           this.invalidate(`${label} must not contain fragments`);
         }

lib/helpers/oidc_context.js

@@ -1,5 +1,4 @@
 import * as events from 'node:events';
-import * as url from 'node:url';
 
 import ctxRef from '../models/ctx_ref.js';
 
@@ -94,7 +93,7 @@ export default function getContext(provider) {
         || this.ctx.req.baseUrl // expressApp.use('/op', provider.callback());
         || ''; // no mount
 
-      return url.resolve(this.ctx.href, provider.pathFor(name, { mountPath, ...opt }));
+      return new URL(provider.pathFor(name, { mountPath, ...opt }), this.ctx.href).href;
     }
 
     promptPending(name) {

lib/helpers/redirect_uri.js

@@ -1,23 +1,16 @@
-import * as url from 'node:url';
-import * as querystring from 'node:querystring';
-
 export default function redirectUri(uri, payload, mode) {
-  const parsed = url.parse(uri, true);
-
-  parsed.search = null;
-
-  // handles a case where url module adds unintended / to the pathname
-  // i.e. http://www.example.com => http://www.example.com/
-  if (parsed.pathname === '/' && !uri.endsWith('/')) parsed.pathname = null;
+  const parsed = new URL(uri);
 
   switch (mode) {
     case 'fragment':
-      parsed.hash = querystring.stringify(payload);
+      parsed.hash = new URLSearchParams(payload).toString();
       break;
     default:
-      Object.assign(parsed.query, payload);
+      for (const [k, v] of Object.entries(payload)) {
+        parsed.searchParams.set(k, v);
+      }
       break;
   }
 
-  return url.format(parsed);
+  return parsed.href;
 }

lib/models/client.js

@@ -1,5 +1,4 @@
 /* eslint-disable max-classes-per-file */
-import { format } from 'node:url';
 import * as crypto from 'node:crypto';
 import { STATUS_CODES } from 'node:http';
 
@@ -37,6 +36,18 @@ const fingerprint = (properties) => hash(properties, {
   respectType: false,
 });
 
+function URLparse(url) {
+  if (URL.parse) {
+    return URL.parse(url);
+  }
+
+  try {
+    return new URL(url);
+  } catch {
+    return null;
+  }
+}
+
 const validateJWKS = (jwks) => {
   if (jwks !== undefined) {
     if (!Array.isArray(jwks?.keys) || !jwks.keys.every(isPlainObject)) {
@@ -111,10 +122,6 @@ function checkJWK(jwk) {
   return jwk;
 }
 
-function stripFragment(uri) {
-  return format(new URL(uri), { fragment: false });
-}
-
 function deriveEncryptionKey(secret, length) {
   const digest = length <= 32 ? 'sha256' : length <= 48 ? 'sha384' : length <= 64 ? 'sha512' : false; // eslint-disable-line no-nested-ternary
   if (!digest) {
@@ -505,40 +512,47 @@ export default function getClient(provider) {
     }
 
     redirectUriAllowed(value) {
-      let parsed;
-      try {
-        parsed = new URL(value);
-      } catch (err) {
-        return false;
-      }
+      const parsed = URLparse(value);
+      if (!parsed) return false;
 
-      const match = this.redirectUris.includes(value);
+      const match = this.redirectUris.find((allowed) => URLparse(allowed)?.href === parsed.href);
       if (
-        match
+        !!match
         || this.applicationType !== 'native'
         || parsed.protocol !== 'http:'
         || !LOOPBACKS.has(parsed.hostname)
       ) {
-        return match;
+        return !!match;
       }
 
       parsed.port = '';
 
       return !!this.redirectUris
-        .find((registeredUri) => {
-          const registered = new URL(registeredUri);
+        .find((allowed) => {
+          const registered = URLparse(allowed);
+          if (!registered) return false;
           registered.port = '';
           return parsed.href === registered.href;
         });
     }
 
     requestUriAllowed(uri) {
-      const requested = stripFragment(uri);
-      return !!(this.requestUris || []).find((enabled) => requested === stripFragment(enabled));
+      const requested = URLparse(uri);
+      if (!requested) return false;
+      requested.hash = '';
+      return !!this.requestUris?.find((enabled) => {
+        const parsed = URLparse(enabled);
+        if (!parsed) return false;
+        parsed.hash = '';
+        return requested.href === parsed.href;
+      });
     }
 
-    postLogoutRedirectUriAllowed(uri) {
-      return this.postLogoutRedirectUris.includes(uri);
+    postLogoutRedirectUriAllowed(value) {
+      const parsed = URLparse(value);
+      if (!parsed) return false;
+      return !!this.postLogoutRedirectUris
+        .find((allowed) => URLparse(allowed)?.href === parsed.href);
     }
 
     static async validate(metadata) {

lib/provider.js

@@ -1,7 +1,6 @@
 // eslint-disable-next-line import/order
 import * as attention from './helpers/attention.js';
 
-import * as url from 'node:url';
 import { strict as assert } from 'node:assert';
 import * as events from 'node:events';
 
@@ -101,11 +100,13 @@ class Provider extends events.EventEmitter {
     assert.equal(typeof issuer, 'string', 'Issuer Identifier must be a string');
     assert(isWebUri(issuer), 'Issuer Identifier must be a valid web uri');
 
-    const components = url.parse(issuer);
-    assert(components.host, 'Issuer Identifier must have a host component');
-    assert(components.protocol, 'Issuer Identifier must have an URI scheme component');
-    assert(!components.search, 'Issuer Identifier must not have a query component');
-    assert(!components.hash, 'Issuer Identifier must not have a fragment component');
+    const {
+      host, protocol, search, hash, href,
+    } = new URL(issuer);
+    assert(host, 'Issuer Identifier must have a host component');
+    assert(protocol, 'Issuer Identifier must have an URI scheme component');
+    assert(!search && !href.endsWith('?'), 'Issuer Identifier must not have a query component');
+    assert(!hash && !href.endsWith('#'), 'Issuer Identifier must not have a fragment component');
 
     super();
 
@@ -152,7 +153,7 @@ class Provider extends events.EventEmitter {
     this.#BackchannelAuthenticationRequest = models.getBackchannelAuthenticationRequest(this);
     this.#PushedAuthorizationRequest = models.getPushedAuthorizationRequest(this);
     this.#OIDCContext = getContext(this);
-    const { pathname } = url.parse(this.issuer);
+    const { pathname } = new URL(this.issuer);
     this.#mountPath = pathname.endsWith('/') ? pathname.slice(0, -1) : pathname;
     instance(this).requestUriCache = new RequestUriCache(this);
 
@@ -164,7 +165,9 @@ class Provider extends events.EventEmitter {
     delete configuration.clients;
   }
 
-  urlFor(name, opt) { return url.resolve(this.issuer, this.pathFor(name, opt)); }
+  urlFor(name, opt) {
+    return new URL(this.pathFor(name, opt), this.issuer).href;
+  }
 
   registerGrantType(name, handler, params, dupes) {
     instance(this).configuration('grantTypes').add(name);

test/backchannel_logout/backchannel_logout.test.js

@@ -158,7 +158,7 @@ describe('Back-Channel Logout 1.0', () => {
 
     it('triggers the backchannelLogout for all visited clients [when global logout]', async function () {
       const session = this.getSession();
-      session.state = { secret: '123', clientId: 'client', postLogoutRedirectUri: '/' };
+      session.state = { secret: '123', clientId: 'client', postLogoutRedirectUri: 'https://rp.example.com/' };
       const params = { logout: 'yes', xsrf: '123' };
       const client = await this.provider.Client.find('client');
       const client2 = await this.provider.Client.find('second-client');
@@ -201,7 +201,7 @@ describe('Back-Channel Logout 1.0', () => {
 
     it('still triggers the backchannelLogout for the specific client [when no global logout]', async function () {
       const session = this.getSession();
-      session.state = { secret: '123', clientId: 'client', postLogoutRedirectUri: '/' };
+      session.state = { secret: '123', clientId: 'client', postLogoutRedirectUri: 'https://rp.example.com/' };
       const params = { xsrf: '123' };
       const client = await this.provider.Client.find('client');
       const client2 = await this.provider.Client.find('second-client');
@@ -226,7 +226,7 @@ describe('Back-Channel Logout 1.0', () => {
     });
 
     it('ignores the backchannelLogout when client does not support', async function () {
-      this.getSession().state = { secret: '123', clientId: 'client', postLogoutRedirectUri: '/' };
+      this.getSession().state = { secret: '123', clientId: 'client', postLogoutRedirectUri: 'https://rp.example.com/' };
       const params = { logout: 'yes', xsrf: '123' };
       const client = await this.provider.Client.find('client');
       const client2 = await this.provider.Client.find('second-client');

test/configuration/client_metadata.test.js

@@ -455,7 +455,6 @@ describe('Client metadata validation', () => {
         require_pushed_authorization_requests: false,
       }, { features: { pushedAuthorizationRequests: { allowUnregisteredRedirectUris: true } } });
     });
-    rejects(this.title, ['https://rp.example.com#'], /redirect_uris must not contain fragments$/);
     rejects(
       this.title,
       ['https://rp.example.com#whatever'],
@@ -546,11 +545,6 @@ describe('Client metadata validation', () => {
     allows(this.title, ['https://some'], {
       application_type: 'web',
     });
-    rejects(
-      this.title,
-      ['https://rp.example.com#'],
-      /post_logout_redirect_uris must not contain fragments$/,
-    );
     rejects(
       this.title,
       ['https://rp.example.com#whatever'],

test/end_session/end_session.test.js

@@ -420,7 +420,7 @@ describe('logout endpoint', () => {
           expect(ctx.oidc.entities).to.have.keys('Client', 'Session');
         }, done));
 
-        this.getSession().state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' };
+        this.getSession().state = { secret: '123', postLogoutRedirectUri: 'https://rp.example.com/', clientId: 'client' };
 
         this.agent.post('/session/end/confirm')
           .send({ xsrf: '123', logout: 'yes' })
@@ -437,7 +437,7 @@ describe('logout endpoint', () => {
         sinon.spy(authorizationCodeAdapter, 'revokeByGrantId');
         const session = this.getSession();
 
-        session.state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' };
+        session.state = { secret: '123', postLogoutRedirectUri: 'https://rp.example.com/', clientId: 'client' };
         session.authorizations.client.persistsLogout = true;
 
         const [firstGrant, secondGrant] = Object.keys(session.authorizations)
@@ -514,7 +514,7 @@ describe('logout endpoint', () => {
 
       it('forwards the state too', function () {
         this.getSession().state = {
-          secret: '123', postLogoutRedirectUri: '/', clientId: 'client', state: 'foobar',
+          secret: '123', postLogoutRedirectUri: 'https://rp.example.com/', clientId: 'client', state: 'foobar',
         };
 
         i(this.provider).configuration().cookies.long.domain = '.oidc.dev';
@@ -526,13 +526,13 @@ describe('logout endpoint', () => {
             delete i(this.provider).configuration().cookies.long.domain;
           })
           .expect(303)
-          .expect('location', '/?state=foobar');
+          .expect('location', 'https://rp.example.com/?state=foobar');
       });
 
       it('handles a no existing session state', async function () {
         Object.assign(this.getSession(), {
           state: {
-            secret: '123', postLogoutRedirectUri: '/', clientId: 'client', state: 'foobar',
+            secret: '123', postLogoutRedirectUri: 'https://rp.example.com/', clientId: 'client', state: 'foobar',
           },
           authorizations: undefined,
         });