Skip to content

Commit c0aa0e8

Browse files
reggireggi
authored
chore: remove unnecessary ifs (#8453)
1 parent 848a0db commit c0aa0e8

File tree

3 files changed

+101
-77
lines changed

3 files changed

+101
-77
lines changed

‎lib/utils/oidc.js‎

Lines changed: 57 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -46,67 +46,62 @@ async function oidc ({ packageName, registry, opts, config }) {
4646
*/
4747
let idToken = process.env.NPM_ID_TOKEN
4848

49-
if (idToken) {
50-
// NPM_ID_TOKEN present
51-
} else {
52-
// NPM_ID_TOKEN not present, checking for GITHUB_ACTIONS
53-
if (ciInfo.GITHUB_ACTIONS) {
54-
/**
55-
* GitHub Actions provides these environment variables:
56-
* - `ACTIONS_ID_TOKEN_REQUEST_URL`: The URL to request the ID token.
57-
* - `ACTIONS_ID_TOKEN_REQUEST_TOKEN`: The token to authenticate the request.
58-
* Only when a workflow has the following permissions:
59-
* ```
60-
* permissions:
61-
* id-token: write
62-
* ```
63-
* @see https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers#adding-permissions-settings
64-
*/
65-
if (
66-
process.env.ACTIONS_ID_TOKEN_REQUEST_URL &&
67-
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN
68-
) {
69-
/**
70-
* The specification for an audience is `npm:registry.npmjs.org`,
71-
* where "registry.npmjs.org" can be any supported registry.
72-
*/
73-
const audience = `npm:${new URL(registry).hostname}`
74-
const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL)
75-
url.searchParams.append('audience', audience)
76-
const startTime = Date.now()
77-
const response = await fetch(url.href, {
78-
retry: opts.retry,
79-
headers: {
80-
Accept: 'application/json',
81-
Authorization: `Bearer ${process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN}`,
82-
},
83-
})
84-
85-
const elapsedTime = Date.now() - startTime
86-
87-
log.http(
88-
'fetch',
89-
`GET ${url.href} ${response.status} ${elapsedTime}ms`
90-
)
91-
92-
const json = await response.json()
93-
94-
if (!response.ok) {
95-
log.verbose('oidc', `Failed to fetch id_token from GitHub: received an invalid response`)
96-
return undefined
97-
}
49+
if (!idToken && ciInfo.GITHUB_ACTIONS) {
50+
/**
51+
* GitHub Actions provides these environment variables:
52+
* - `ACTIONS_ID_TOKEN_REQUEST_URL`: The URL to request the ID token.
53+
* - `ACTIONS_ID_TOKEN_REQUEST_TOKEN`: The token to authenticate the request.
54+
* Only when a workflow has the following permissions:
55+
* ```
56+
* permissions:
57+
* id-token: write
58+
* ```
59+
* @see https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers#adding-permissions-settings
60+
*/
61+
if (!(
62+
process.env.ACTIONS_ID_TOKEN_REQUEST_URL &&
63+
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN
64+
)) {
65+
log.silly('oidc', 'Skipped because incorrect permissions for id-token within GitHub workflow')
66+
return undefined
67+
}
9868

99-
if (!json.value) {
100-
log.verbose('oidc', `Failed to fetch id_token from GitHub: missing value`)
101-
return undefined
102-
}
69+
/**
70+
* The specification for an audience is `npm:registry.npmjs.org`,
71+
* where "registry.npmjs.org" can be any supported registry.
72+
*/
73+
const audience = `npm:${new URL(registry).hostname}`
74+
const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL)
75+
url.searchParams.append('audience', audience)
76+
const startTime = Date.now()
77+
const response = await fetch(url.href, {
78+
retry: opts.retry,
79+
headers: {
80+
Accept: 'application/json',
81+
Authorization: `Bearer ${process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN}`,
82+
},
83+
})
10384

104-
idToken = json.value
105-
} else {
106-
log.silly('oidc', 'Skipped because incorrect permissions for id-token within GitHub workflow')
107-
return undefined
108-
}
85+
const elapsedTime = Date.now() - startTime
86+
87+
log.http(
88+
'fetch',
89+
`GET ${url.href} ${response.status} ${elapsedTime}ms`
90+
)
91+
92+
const json = await response.json()
93+
94+
if (!response.ok) {
95+
log.verbose('oidc', `Failed to fetch id_token from GitHub: received an invalid response`)
96+
return undefined
97+
}
98+
99+
if (!json.value) {
100+
log.verbose('oidc', `Failed to fetch id_token from GitHub: missing value`)
101+
return undefined
109102
}
103+
104+
idToken = json.value
110105
}
111106

112107
if (!idToken) {
@@ -117,9 +112,9 @@ async function oidc ({ packageName, registry, opts, config }) {
117112
// this checks if the user configured provenance or it's the default unset value
118113
const isDefaultProvenance = config.isDefault('provenance')
119114
const provenanceIntent = config.get('provenance')
120-
const skipProvenance = isDefaultProvenance || provenanceIntent
121115

122-
if (skipProvenance) {
116+
// if provenance is the default value or the user explicitly set it
117+
if (isDefaultProvenance || provenanceIntent) {
123118
const [headerB64, payloadB64] = idToken.split('.')
124119
let enableProvenance = false
125120
if (headerB64 && payloadB64) {
@@ -158,9 +153,7 @@ async function oidc ({ packageName, registry, opts, config }) {
158153
method: 'POST',
159154
})
160155
} catch (error) {
161-
if (error?.body?.message) {
162-
log.verbose('oidc', `Failed with body message "${error.body.message}"`)
163-
}
156+
log.verbose('oidc', `Failed token exchange request with body message: ${error?.body?.message || 'Unknown error'}`)
164157
return undefined
165158
}
166159

@@ -178,7 +171,7 @@ async function oidc ({ packageName, registry, opts, config }) {
178171
config.set(authTokenKey, response.token, 'user')
179172
log.verbose('oidc', `Successfully retrieved and set token`)
180173
} catch (error) {
181-
log.verbose('oidc', `Failure with message "${error?.message || 'Unknown error'}"`)
174+
log.verbose('oidc', `Failure with message: ${error?.message || 'Unknown error'}`)
182175
}
183176
return undefined
184177
}

‎test/fixtures/mock-oidc.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,12 @@ const mockOidc = async (t, {
136136

137137
const oidcPublishTest = (opts) => {
138138
return async (t) => {
139-
const { npm, joinedOutput } = await mockOidc(t, opts)
139+
const { logsContain } = opts
140+
const { npm, joinedOutput, logs } = await mockOidc(t, opts)
140141
await npm.exec('publish', [])
142+
logsContain?.forEach(item => {
143+
t.ok(logs.includes(item), `Expected log to include: ${item}`)
144+
})
141145
t.match(joinedOutput(), '+ @npmcli/[email protected]')
142146
}
143147
}

‎test/lib/commands/publish.js‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,9 @@ t.test('oidc token exchange - no provenance', t => {
10081008
publishOptions: {
10091009
token: 'existing-fallback-token',
10101010
},
1011+
logsContain: [
1012+
'verbose oidc Failed to fetch id_token from GitHub: received an invalid response',
1013+
],
10111014
}))
10121015

10131016
t.test('oidc token invalid body with fallback', oidcPublishTest({
@@ -1022,6 +1025,9 @@ t.test('oidc token exchange - no provenance', t => {
10221025
publishOptions: {
10231026
token: 'existing-fallback-token',
10241027
},
1028+
logsContain: [
1029+
'verbose oidc Failed to fetch id_token from GitHub: missing value',
1030+
],
10251031
}))
10261032

10271033
t.test('token exchange 500 with fallback', oidcPublishTest({
@@ -1043,9 +1049,12 @@ t.test('oidc token exchange - no provenance', t => {
10431049
publishOptions: {
10441050
token: 'existing-fallback-token',
10451051
},
1052+
logsContain: [
1053+
'verbose oidc Failed token exchange request with body message: oidc token exchange failed',
1054+
],
10461055
}))
10471056

1048-
t.test('token exchange 500 (with no body message) with fallback', oidcPublishTest({
1057+
t.test('token exchange 500 with no body message with fallback', oidcPublishTest({
10491058
oidcOptions: { github: true },
10501059
config: {
10511060
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
@@ -1062,6 +1071,9 @@ t.test('oidc token exchange - no provenance', t => {
10621071
publishOptions: {
10631072
token: 'existing-fallback-token',
10641073
},
1074+
logsContain: [
1075+
'verbose oidc Failed token exchange request with body message: Unknown error',
1076+
],
10651077
}))
10661078

10671079
t.test('token exchange invalid body with fallback', oidcPublishTest({
@@ -1082,26 +1094,35 @@ t.test('oidc token exchange - no provenance', t => {
10821094
publishOptions: {
10831095
token: 'existing-fallback-token',
10841096
},
1097+
logsContain: [
1098+
'verbose oidc Failed because token exchange was missing the token in the response body',
1099+
],
10851100
}))
10861101

1087-
t.test('github + missing ACTIONS_ID_TOKEN_REQUEST_URL', oidcPublishTest({
1102+
t.test('github missing ACTIONS_ID_TOKEN_REQUEST_URL', oidcPublishTest({
10881103
oidcOptions: { github: true, ACTIONS_ID_TOKEN_REQUEST_URL: '' },
10891104
config: {
10901105
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
10911106
},
10921107
publishOptions: {
10931108
token: 'existing-fallback-token',
10941109
},
1110+
logsContain: [
1111+
'silly oidc Skipped because incorrect permissions for id-token within GitHub workflow',
1112+
],
10951113
}))
10961114

1097-
t.test('gitlab + missing NPM_ID_TOKEN', oidcPublishTest({
1115+
t.test('gitlab missing NPM_ID_TOKEN', oidcPublishTest({
10981116
oidcOptions: { gitlab: true, NPM_ID_TOKEN: '' },
10991117
config: {
11001118
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
11011119
},
11021120
publishOptions: {
11031121
token: 'existing-fallback-token',
11041122
},
1123+
logsContain: [
1124+
'silly oidc Skipped because no id_token available',
1125+
],
11051126
}))
11061127

11071128
t.test('no ci', oidcPublishTest({
@@ -1112,6 +1133,9 @@ t.test('oidc token exchange - no provenance', t => {
11121133
publishOptions: {
11131134
token: 'existing-fallback-token',
11141135
},
1136+
logsContain: [
1137+
'silly oidc Skipped because unsupported CI environment',
1138+
],
11151139
}))
11161140

11171141
// default registry success
@@ -1136,21 +1160,24 @@ t.test('oidc token exchange - no provenance', t => {
11361160
},
11371161
}))
11381162

1139-
t.test('global try / catch failure via malformed url', oidcPublishTest({
1163+
t.test('global try-catch failure via malformed url', oidcPublishTest({
11401164
config: {
11411165
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
11421166
},
11431167
oidcOptions: {
11441168
github: true,
1145-
// malformed url should trigger a global try / catch
1169+
// malformed url should trigger a global try-catch
11461170
ACTIONS_ID_TOKEN_REQUEST_URL: '//github.com',
11471171
},
11481172
publishOptions: {
11491173
token: 'existing-fallback-token',
11501174
},
1175+
logsContain: [
1176+
'verbose oidc Failure with message: Invalid URL',
1177+
],
11511178
}))
11521179

1153-
t.test('global try / catch failure via throw non Error', async t => {
1180+
t.test('global try-catch failure via throw non Error', async t => {
11541181
const { npm, logs, joinedOutput, ACTIONS_ID_TOKEN_REQUEST_URL } = await mockOidc(t, {
11551182
config: {
11561183
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
@@ -1167,7 +1194,7 @@ t.test('oidc token exchange - no provenance', t => {
11671194
constructor (...args) {
11681195
const [url] = args
11691196
if (url === ACTIONS_ID_TOKEN_REQUEST_URL) {
1170-
throw 'Specifically throwing a non errror object to test global try / catch'
1197+
throw 'Specifically throwing a non errror object to test global try-catch'
11711198
}
11721199
super(...args)
11731200
}
@@ -1179,7 +1206,7 @@ t.test('oidc token exchange - no provenance', t => {
11791206

11801207
await npm.exec('publish', [])
11811208
t.match(joinedOutput(), '+ @npmcli/[email protected]')
1182-
t.ok(logs.includes('verbose oidc Failure with message "Unknown error"'))
1209+
t.ok(logs.includes('verbose oidc Failure with message: Unknown error'))
11831210
})
11841211

11851212
t.test('default registry success gitlab', oidcPublishTest({
@@ -1200,7 +1227,7 @@ t.test('oidc token exchange - no provenance', t => {
12001227

12011228
// custom registry success
12021229

1203-
t.test('custom registry (config) success github', oidcPublishTest({
1230+
t.test('custom registry config success github', oidcPublishTest({
12041231
oidcOptions: { github: true },
12051232
config: {
12061233
registry: 'https://registry.zzz.org',
@@ -1220,7 +1247,7 @@ t.test('oidc token exchange - no provenance', t => {
12201247
},
12211248
}))
12221249

1223-
t.test('custom registry (scoped config) success github', oidcPublishTest({
1250+
t.test('custom registry scoped config success github', oidcPublishTest({
12241251
oidcOptions: { github: true },
12251252
config: {
12261253
'@npmcli:registry': 'https://registry.zzz.org',
@@ -1243,7 +1270,7 @@ t.test('oidc token exchange - no provenance', t => {
12431270
},
12441271
}))
12451272

1246-
t.test('custom registry (publishConfig) success github', oidcPublishTest({
1273+
t.test('custom registry publishConfig success github', oidcPublishTest({
12471274
oidcOptions: { github: true },
12481275
packageJson: {
12491276
publishConfig: {
@@ -1292,7 +1319,7 @@ t.test('oidc token exchange - no provenance', t => {
12921319
t.end()
12931320
})
12941321

1295-
t.test('oidc token exchange -- provenance', (t) => {
1322+
t.test('oidc token exchange - provenance', (t) => {
12961323
const githubPublicIdToken = githubIdToken({ visibility: 'public' })
12971324
const gitlabPublicIdToken = gitlabIdToken({ visibility: 'public' })
12981325
const SIGSTORE_ID_TOKEN = sigstoreIdToken()

0 commit comments

Comments
 (0)