chore: remove unnecessary ifs

lib/utils/oidc.js

@@ -46,67 +46,62 @@ async function oidc ({ packageName, registry, opts, config }) {
      */
     let idToken = process.env.NPM_ID_TOKEN
 
-    if (idToken) {
-      // NPM_ID_TOKEN present
-    } else {
-      // NPM_ID_TOKEN not present, checking for GITHUB_ACTIONS
-      if (ciInfo.GITHUB_ACTIONS) {
-        /**
-         * GitHub Actions provides these environment variables:
-         * - `ACTIONS_ID_TOKEN_REQUEST_URL`: The URL to request the ID token.
-         * - `ACTIONS_ID_TOKEN_REQUEST_TOKEN`: The token to authenticate the request.
-         * Only when a workflow has the following permissions:
-         * ```
-         * permissions:
-         *    id-token: write
-         * ```
-         * @see https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers#adding-permissions-settings
-         */
-        if (
-          process.env.ACTIONS_ID_TOKEN_REQUEST_URL &&
-          process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN
-        ) {
-          /**
-           * The specification for an audience is `npm:registry.npmjs.org`,
-           * where "registry.npmjs.org" can be any supported registry.
-           */
-          const audience = `npm:${new URL(registry).hostname}`
-          const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL)
-          url.searchParams.append('audience', audience)
-          const startTime = Date.now()
-          const response = await fetch(url.href, {
-            retry: opts.retry,
-            headers: {
-              Accept: 'application/json',
-              Authorization: `Bearer ${process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN}`,
-            },
-          })
-
-          const elapsedTime = Date.now() - startTime
-
-          log.http(
-            'fetch',
-            `GET ${url.href} ${response.status} ${elapsedTime}ms`
-          )
-
-          const json = await response.json()
-
-          if (!response.ok) {
-            log.verbose('oidc', `Failed to fetch id_token from GitHub: received an invalid response`)
-            return undefined
-          }
+    if (!idToken && ciInfo.GITHUB_ACTIONS) {
+      /**
+       * GitHub Actions provides these environment variables:
+       * - `ACTIONS_ID_TOKEN_REQUEST_URL`: The URL to request the ID token.
+       * - `ACTIONS_ID_TOKEN_REQUEST_TOKEN`: The token to authenticate the request.
+       * Only when a workflow has the following permissions:
+       * ```
+       * permissions:
+       *    id-token: write
+       * ```
+       * @see https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers#adding-permissions-settings
+       */
+      if (!(
+        process.env.ACTIONS_ID_TOKEN_REQUEST_URL &&
+        process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN
+      )) {
+        log.silly('oidc', 'Skipped because incorrect permissions for id-token within GitHub workflow')
+        return undefined
+      }
 
-          if (!json.value) {
-            log.verbose('oidc', `Failed to fetch id_token from GitHub: missing value`)
-            return undefined
-          }
+      /**
+       * The specification for an audience is `npm:registry.npmjs.org`,
+       * where "registry.npmjs.org" can be any supported registry.
+       */
+      const audience = `npm:${new URL(registry).hostname}`
+      const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL)
+      url.searchParams.append('audience', audience)
+      const startTime = Date.now()
+      const response = await fetch(url.href, {
+        retry: opts.retry,
+        headers: {
+          Accept: 'application/json',
+          Authorization: `Bearer ${process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN}`,
+        },
+      })
 
-          idToken = json.value
-        } else {
-          log.silly('oidc', 'Skipped because incorrect permissions for id-token within GitHub workflow')
-          return undefined
-        }
+      const elapsedTime = Date.now() - startTime
+
+      log.http(
+        'fetch',
+        `GET ${url.href} ${response.status} ${elapsedTime}ms`
+      )
+
+      const json = await response.json()
+
+      if (!response.ok) {
+        log.verbose('oidc', `Failed to fetch id_token from GitHub: received an invalid response`)
+        return undefined
+      }
+
+      if (!json.value) {
+        log.verbose('oidc', `Failed to fetch id_token from GitHub: missing value`)
+        return undefined
       }
+
+      idToken = json.value
     }
 
     if (!idToken) {
@@ -117,9 +112,9 @@ async function oidc ({ packageName, registry, opts, config }) {
     // this checks if the user configured provenance or it's the default unset value
     const isDefaultProvenance = config.isDefault('provenance')
     const provenanceIntent = config.get('provenance')
-    const skipProvenance = isDefaultProvenance || provenanceIntent
 
-    if (skipProvenance) {
+    // if provenance is the default value or the user explicitly set it
+    if (isDefaultProvenance || provenanceIntent) {
       const [headerB64, payloadB64] = idToken.split('.')
       let enableProvenance = false
       if (headerB64 && payloadB64) {
@@ -158,9 +153,7 @@ async function oidc ({ packageName, registry, opts, config }) {
         method: 'POST',
       })
     } catch (error) {
-      if (error?.body?.message) {
-        log.verbose('oidc', `Failed with body message "${error.body.message}"`)
-      }
+      log.verbose('oidc', `Failed token exchange request with body message: ${error?.body?.message || 'Unknown error'}`)
       return undefined
     }
 
@@ -178,7 +171,7 @@ async function oidc ({ packageName, registry, opts, config }) {
     config.set(authTokenKey, response.token, 'user')
     log.verbose('oidc', `Successfully retrieved and set token`)
   } catch (error) {
-    log.verbose('oidc', `Failure with message "${error?.message || 'Unknown error'}"`)
+    log.verbose('oidc', `Failure with message: ${error?.message || 'Unknown error'}`)
   }
   return undefined
 }

test/fixtures/mock-oidc.js

@@ -136,8 +136,12 @@ const mockOidc = async (t, {
 
 const oidcPublishTest = (opts) => {
   return async (t) => {
-    const { npm, joinedOutput } = await mockOidc(t, opts)
+    const { logsContain } = opts
+    const { npm, joinedOutput, logs } = await mockOidc(t, opts)
     await npm.exec('publish', [])
+    logsContain?.forEach(item => {
+      t.ok(logs.includes(item), `Expected log to include: ${item}`)
+    })
     t.match(joinedOutput(), '+ @npmcli/[email protected]')
   }
 }

test/lib/commands/publish.js

@@ -1008,6 +1008,9 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failed to fetch id_token from GitHub: received an invalid response',
+    ],
   }))
 
   t.test('oidc token invalid body with fallback', oidcPublishTest({
@@ -1022,6 +1025,9 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failed to fetch id_token from GitHub: missing value',
+    ],
   }))
 
   t.test('token exchange 500 with fallback', oidcPublishTest({
@@ -1043,9 +1049,12 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failed token exchange request with body message: oidc token exchange failed',
+    ],
   }))
 
-  t.test('token exchange 500 (with no body message) with fallback', oidcPublishTest({
+  t.test('token exchange 500 with no body message with fallback', oidcPublishTest({
     oidcOptions: { github: true },
     config: {
       '//registry.npmjs.org/:_authToken': 'existing-fallback-token',
@@ -1062,6 +1071,9 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failed token exchange request with body message: Unknown error',
+    ],
   }))
 
   t.test('token exchange invalid body with fallback', oidcPublishTest({
@@ -1082,26 +1094,35 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failed because token exchange was missing the token in the response body',
+    ],
   }))
 
-  t.test('github + missing ACTIONS_ID_TOKEN_REQUEST_URL', oidcPublishTest({
+  t.test('github missing ACTIONS_ID_TOKEN_REQUEST_URL', oidcPublishTest({
     oidcOptions: { github: true, ACTIONS_ID_TOKEN_REQUEST_URL: '' },
     config: {
       '//registry.npmjs.org/:_authToken': 'existing-fallback-token',
     },
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'silly oidc Skipped because incorrect permissions for id-token within GitHub workflow',
+    ],
   }))
 
-  t.test('gitlab + missing NPM_ID_TOKEN', oidcPublishTest({
+  t.test('gitlab missing NPM_ID_TOKEN', oidcPublishTest({
     oidcOptions: { gitlab: true, NPM_ID_TOKEN: '' },
     config: {
       '//registry.npmjs.org/:_authToken': 'existing-fallback-token',
     },
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'silly oidc Skipped because no id_token available',
+    ],
   }))
 
   t.test('no ci', oidcPublishTest({
@@ -1112,6 +1133,9 @@ t.test('oidc token exchange - no provenance', t => {
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'silly oidc Skipped because unsupported CI environment',
+    ],
   }))
 
   // default registry success
@@ -1136,21 +1160,24 @@ t.test('oidc token exchange - no provenance', t => {
     },
   }))
 
-  t.test('global try / catch failure via malformed url', oidcPublishTest({
+  t.test('global try-catch failure via malformed url', oidcPublishTest({
     config: {
       '//registry.npmjs.org/:_authToken': 'existing-fallback-token',
     },
     oidcOptions: {
       github: true,
-      // malformed url should trigger a global try / catch
+      // malformed url should trigger a global try-catch
       ACTIONS_ID_TOKEN_REQUEST_URL: '//github.com',
     },
     publishOptions: {
       token: 'existing-fallback-token',
     },
+    logsContain: [
+      'verbose oidc Failure with message: Invalid URL',
+    ],
   }))
 
-  t.test('global try / catch failure via throw non Error', async t => {
+  t.test('global try-catch failure via throw non Error', async t => {
     const { npm, logs, joinedOutput, ACTIONS_ID_TOKEN_REQUEST_URL } = await mockOidc(t, {
       config: {
         '//registry.npmjs.org/:_authToken': 'existing-fallback-token',
@@ -1167,7 +1194,7 @@ t.test('oidc token exchange - no provenance', t => {
       constructor (...args) {
         const [url] = args
         if (url === ACTIONS_ID_TOKEN_REQUEST_URL) {
-          throw 'Specifically throwing a non errror object to test global try / catch'
+          throw 'Specifically throwing a non errror object to test global try-catch'
         }
         super(...args)
       }
@@ -1179,7 +1206,7 @@ t.test('oidc token exchange - no provenance', t => {
 
     await npm.exec('publish', [])
     t.match(joinedOutput(), '+ @npmcli/[email protected]')
-    t.ok(logs.includes('verbose oidc Failure with message "Unknown error"'))
+    t.ok(logs.includes('verbose oidc Failure with message: Unknown error'))
   })
 
   t.test('default registry success gitlab', oidcPublishTest({
@@ -1200,7 +1227,7 @@ t.test('oidc token exchange - no provenance', t => {
 
   // custom registry success
 
-  t.test('custom registry (config) success github', oidcPublishTest({
+  t.test('custom registry config success github', oidcPublishTest({
     oidcOptions: { github: true },
     config: {
       registry: 'https://registry.zzz.org',
@@ -1220,7 +1247,7 @@ t.test('oidc token exchange - no provenance', t => {
     },
   }))
 
-  t.test('custom registry (scoped config) success github', oidcPublishTest({
+  t.test('custom registry scoped config success github', oidcPublishTest({
     oidcOptions: { github: true },
     config: {
       '@npmcli:registry': 'https://registry.zzz.org',
@@ -1243,7 +1270,7 @@ t.test('oidc token exchange - no provenance', t => {
     },
   }))
 
-  t.test('custom registry (publishConfig) success github', oidcPublishTest({
+  t.test('custom registry publishConfig success github', oidcPublishTest({
     oidcOptions: { github: true },
     packageJson: {
       publishConfig: {
@@ -1292,7 +1319,7 @@ t.test('oidc token exchange - no provenance', t => {
   t.end()
 })
 
-t.test('oidc token exchange -- provenance', (t) => {
+t.test('oidc token exchange - provenance', (t) => {
   const githubPublicIdToken = githubIdToken({ visibility: 'public' })
   const gitlabPublicIdToken = gitlabIdToken({ visibility: 'public' })
   const SIGSTORE_ID_TOKEN = sigstoreIdToken()