refactor!: reject requests to non-HTTPS endpoints by default

BREAKING CHANGE: all functions now reject interacting with non-TLS HTTP endpoints. You can use the `allowInsecureRequests` in the `HttpRequestOptions` interface to revert this behaviour.
panva · Oct 7, 2024 · 4829da6 · 4829da6
1 parent 0f8bcc3
commit 4829da6
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 26 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -229,6 +229,15 @@ export interface JWK {
   readonly [parameter: string]: JsonValue | undefined
 }
 
+/**
+ * By default the module only allows interactions with HTTPS endpoints. Setting this option to
+ * `true` removes that restriction.
+ *
+ * @deprecated To make it stand out as something you shouldn't use, possibly only for local
+ *   development and testing against non-TLS secured environments.
+ */
+export const allowInsecureRequests: unique symbol = Symbol()
+
 /**
  * Use to adjust the assumed current time. Positive and negative finite values representing seconds
  * are allowed. Default is `0` (Date.now() + 0 seconds is used).
@@ -1187,6 +1196,13 @@ export interface HttpRequestOptions {
    * See {@link customFetch}.
    */
   [customFetch]?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>
+
+  /**
+   * See {@link allowInsecureRequests}.
+   *
+   * @deprecated
+   */
+  [allowInsecureRequests]?: boolean
 }
 
 export interface DiscoveryRequestOptions extends HttpRequestOptions {
@@ -1985,7 +2001,8 @@ async function publicJwk(key: CryptoKeyType) {
 function validateEndpoint(
   value: unknown,
   endpoint: keyof AuthorizationServer,
-  useMtlsAlias: boolean,
+  useMtlsAlias: boolean | undefined,
+  enforceHttps: boolean | undefined,
 ) {
   if (typeof value !== 'string') {
     if (useMtlsAlias) {
@@ -1995,19 +2012,33 @@ function validateEndpoint(
     throw new TypeError(`"as.${endpoint}" must be a string`)
   }
 
-  return new URL(value)
+  const url = new URL(value)
+
+  if (enforceHttps && url.protocol !== 'https:') {
+    throw new OperationProcessingError('only requests to HTTPS are allowed', {
+      code: HTTP_REQUEST_FORBIDDEN,
+    })
+  }
+
+  return url
 }
 
 function resolveEndpoint(
   as: AuthorizationServer,
   endpoint: keyof AuthorizationServer,
-  useMtlsAlias = false,
+  useMtlsAlias: boolean | undefined,
+  enforceHttps: boolean | undefined,
 ) {
   if (useMtlsAlias && as.mtls_endpoint_aliases && endpoint in as.mtls_endpoint_aliases) {
-    return validateEndpoint(as.mtls_endpoint_aliases[endpoint], endpoint, useMtlsAlias)
+    return validateEndpoint(
+      as.mtls_endpoint_aliases[endpoint],
+      endpoint,
+      useMtlsAlias,
+      enforceHttps,
+    )
   }
 
-  return validateEndpoint(as[endpoint], endpoint, useMtlsAlias)
+  return validateEndpoint(as[endpoint], endpoint, useMtlsAlias, enforceHttps)
 }
 
 function alias(client: Client, options?: UseMTLSAliasOptions): boolean {
@@ -2040,7 +2071,12 @@ export async function pushedAuthorizationRequest(
   assertAs(as)
   assertClient(client)
 
-  const url = resolveEndpoint(as, 'pushed_authorization_request_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'pushed_authorization_request_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests],
+  )
 
   const body = new URLSearchParams(parameters)
   body.set('client_id', client.client_id)
@@ -2445,6 +2481,12 @@ export async function protectedResourceRequest(
     throw new TypeError('"url" must be an instance of URL')
   }
 
+  if (options?.[allowInsecureRequests] !== true && url.protocol !== 'https:') {
+    throw new OperationProcessingError('only requests to HTTPS are allowed', {
+      code: HTTP_REQUEST_FORBIDDEN,
+    })
+  }
+
   headers = prepareHeaders(headers)
 
   if (options?.DPoP === undefined) {
@@ -2511,7 +2553,12 @@ export async function userInfoRequest(
   assertAs(as)
   assertClient(client)
 
-  const url = resolveEndpoint(as, 'userinfo_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'userinfo_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests] !== true,
+  )
 
   const headers = prepareHeaders(options?.headers)
   if (client.userinfo_signed_response_alg) {
@@ -2885,7 +2932,12 @@ async function tokenEndpointRequest(
   parameters: URLSearchParams,
   options?: Omit<TokenEndpointRequestOptions, 'additionalParameters'>,
 ): Promise<Response> {
-  const url = resolveEndpoint(as, 'token_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'token_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests] !== true,
+  )
 
   parameters.set('grant_type', grantType)
   const headers = prepareHeaders(options?.headers)
@@ -3652,6 +3704,10 @@ export const RESPONSE_IS_NOT_JSON = 'OAUTH_RESPONSE_IS_NOT_JSON'
  * @ignore
  */
 export const RESPONSE_IS_NOT_CONFORM = 'OAUTH_RESPONSE_IS_NOT_CONFORM'
+/**
+ * @ignore
+ */
+export const HTTP_REQUEST_FORBIDDEN = 'OAUTH_HTTP_REQUEST_FORBIDDEN'
 
 function checkJwtType(expected: string, result: Awaited<ReturnType<typeof validateJwt>>) {
   if (typeof result.header.typ !== 'string' || normalizeTyp(result.header.typ) !== expected) {
@@ -3789,7 +3845,12 @@ export async function revocationRequest(
     throw new TypeError('"token" must be a non-empty string')
   }
 
-  const url = resolveEndpoint(as, 'revocation_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'revocation_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests] !== true,
+  )
 
   const body = new URLSearchParams(options?.additionalParameters)
   body.set('token', token)
@@ -3895,7 +3956,12 @@ export async function introspectionRequest(
     throw new TypeError('"token" must be a non-empty string')
   }
 
-  const url = resolveEndpoint(as, 'introspection_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'introspection_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests] !== true,
+  )
 
   const body = new URLSearchParams(options?.additionalParameters)
   body.set('token', token)
@@ -4038,7 +4104,7 @@ async function jwksRequest(
 ): Promise<Response> {
   assertAs(as)
 
-  const url = resolveEndpoint(as, 'jwks_uri')
+  const url = resolveEndpoint(as, 'jwks_uri', false, options?.[allowInsecureRequests] !== true)
 
   const headers = prepareHeaders(options?.headers)
   headers.set('accept', 'application/json')
@@ -4788,7 +4854,12 @@ export async function deviceAuthorizationRequest(
   assertAs(as)
   assertClient(client)
 
-  const url = resolveEndpoint(as, 'device_authorization_endpoint', alias(client, options))
+  const url = resolveEndpoint(
+    as,
+    'device_authorization_endpoint',
+    alias(client, options),
+    options?.[allowInsecureRequests] !== true,
+  )
 
   const body = new URLSearchParams(parameters)
   body.set('client_id', client.client_id)

diff --git a/tap/end2end-client-credentials.ts b/tap/end2end-client-credentials.ts
@@ -71,7 +71,7 @@ export default (QUnit: QUnit) => {
       }
 
       const as = await lib
-        .discoveryRequest(issuerIdentifier)
+        .discoveryRequest(issuerIdentifier, { [lib.allowInsecureRequests]: true })
         .then((response) => lib.processDiscoveryResponse(issuerIdentifier, response))
 
       const params = new URLSearchParams()
@@ -88,12 +88,14 @@ export default (QUnit: QUnit) => {
           clientCredentialsGrantRequest = () =>
             lib.clientCredentialsGrantRequest(as, client, params, {
               DPoP,
+              [lib.allowInsecureRequests]: true,
               ...authenticated,
             })
         } else {
           clientCredentialsGrantRequest = () =>
             lib.genericTokenEndpointRequest(as, client, 'client_credentials', params, {
               DPoP,
+              [lib.allowInsecureRequests]: true,
               ...authenticated,
             })
         }
@@ -124,6 +126,7 @@ export default (QUnit: QUnit) => {
                   },
                 }
               : undefined,
+            [lib.allowInsecureRequests]: true,
             async [lib.customFetch](...params: Parameters<typeof fetch>) {
               if (authMethod === 'private_key_jwt') {
                 if (params[1]?.body instanceof URLSearchParams) {
@@ -141,7 +144,9 @@ export default (QUnit: QUnit) => {
           const result = await lib.processIntrospectionResponse(as, client, response)
 
           if (jwtIntrospection) {
-            await lib.validateJwtIntrospectionSignature(as, response)
+            await lib.validateJwtIntrospectionSignature(as, response, {
+              [lib.allowInsecureRequests]: true,
+            })
           }
 
           t.propContains(result, {
@@ -153,7 +158,10 @@ export default (QUnit: QUnit) => {
         }
 
         {
-          let response = await lib.revocationRequest(as, client, access_token, authenticated)
+          let response = await lib.revocationRequest(as, client, access_token, {
+            [lib.allowInsecureRequests]: true,
+            ...authenticated,
+          })
           await lib.processRevocationResponse(response)
         }
       }

diff --git a/tap/end2end-device-code.ts b/tap/end2end-device-code.ts
@@ -27,15 +27,17 @@ export default (QUnit: QUnit) => {
       const DPoP = dpop ? await lib.generateKeyPair(alg as lib.JWSAlgorithm) : undefined
 
       const as = await lib
-        .discoveryRequest(issuerIdentifier)
+        .discoveryRequest(issuerIdentifier, { [lib.allowInsecureRequests]: true })
         .then((response) => lib.processDiscoveryResponse(issuerIdentifier, response))
 
       const resource = 'urn:example:resource:jwt'
       const params = new URLSearchParams()
       params.set('resource', resource)
       params.set('scope', 'api:write')
 
-      let response = await lib.deviceAuthorizationRequest(as, client, params)
+      let response = await lib.deviceAuthorizationRequest(as, client, params, {
+        [lib.allowInsecureRequests]: true,
+      })
 
       let result = await lib.processDeviceAuthorizationResponse(as, client, response)
       const { verification_uri_complete, device_code } = result
@@ -49,7 +51,10 @@ export default (QUnit: QUnit) => {
 
       {
         const deviceCodeGrantRequest = () =>
-          lib.deviceCodeGrantRequest(as, client, device_code, { DPoP })
+          lib.deviceCodeGrantRequest(as, client, device_code, {
+            DPoP,
+            [lib.allowInsecureRequests]: true,
+          })
         let response = await deviceCodeGrantRequest()
 
         const processDeviceCodeResponse = () => lib.processDeviceCodeResponse(as, client, response)
@@ -85,12 +90,15 @@ export default (QUnit: QUnit) => {
                   },
                 }
               : undefined,
+            [lib.allowInsecureRequests]: true,
             async [lib.customFetch](...params: Parameters<typeof fetch>) {
               const url = new URL(params[0] as string)
               const { headers, method } = params[1]!
               const request = new Request(url, { headers, method })
 
-              const jwtAccessToken = await lib.validateJwtAccessToken(as, request, resource)
+              const jwtAccessToken = await lib.validateJwtAccessToken(as, request, resource, {
+                [lib.allowInsecureRequests]: true,
+              })
 
               t.propContains(jwtAccessToken, {
                 client_id: client.client_id,

diff --git a/tap/end2end.ts b/tap/end2end.ts
@@ -165,14 +165,18 @@ export default (QUnit: QUnit) => {
           nonce!,
           lib.expectNoState,
           maxAge,
+          { [lib.allowInsecureRequests]: true },
         )
-      } else {
-        callbackParams = await (jarm ? lib.validateJwtAuthResponse : lib.validateAuthResponse)(
+      } else if (jarm) {
+        callbackParams = await lib.validateJwtAuthResponse(
           as,
           client,
           currentUrl,
           lib.expectNoState,
+          { [lib.allowInsecureRequests]: true },
         )
+      } else {
+        callbackParams = lib.validateAuthResponse(as, client, currentUrl, lib.expectNoState)
       }
 
       {
@@ -183,7 +187,7 @@ export default (QUnit: QUnit) => {
             callbackParams,
             'http://localhost:3000/cb',
             code_verifier,
-            { DPoP },
+            { DPoP, [lib.allowInsecureRequests]: true },
           )
         let response = await authorizationCodeGrantRequest()
 
@@ -205,10 +209,14 @@ export default (QUnit: QUnit) => {
           throw new Error()
         }
         const { sub } = lib.getValidatedIdTokenClaims(result)
-        await lib.validateIdTokenSignature(as, result)
+        await lib.validateIdTokenSignature(as, result, { [lib.allowInsecureRequests]: true })
 
         {
-          const userInfoRequest = () => lib.userInfoRequest(as, client, access_token, { DPoP })
+          const userInfoRequest = () =>
+            lib.userInfoRequest(as, client, access_token, {
+              DPoP,
+              [lib.allowInsecureRequests]: true,
+            })
           let response = await userInfoRequest().catch(async (err) => {
             if (isDpopNonceError(t, err)) {
               return userInfoRequest()
@@ -220,13 +228,18 @@ export default (QUnit: QUnit) => {
           await lib.processUserInfoResponse(as, client, sub, response)
 
           if (jwtUserinfo) {
-            await lib.validateJwtUserInfoSignature(as, response)
+            await lib.validateJwtUserInfoSignature(as, response, {
+              [lib.allowInsecureRequests]: true,
+            })
           }
         }
 
         {
           const refreshTokenGrantRequest = () =>
-            lib.refreshTokenGrantRequest(as, client, refresh_token, { DPoP })
+            lib.refreshTokenGrantRequest(as, client, refresh_token, {
+              DPoP,
+              [lib.allowInsecureRequests]: true,
+            })
           let response = await refreshTokenGrantRequest().catch((err) => {
             if (isDpopNonceError(t, err)) {
               // the AS-signalled nonce is now cached, retrying