Skip to content

fix(JFrogToolsInstaller): authenticate CLI download for OIDC service connections#638

Open
agrasth wants to merge 3 commits into
devfrom
RTECO-1402-oidc-tools-installer-cli-download
Open

fix(JFrogToolsInstaller): authenticate CLI download for OIDC service connections#638
agrasth wants to merge 3 commits into
devfrom
RTECO-1402-oidc-tools-installer-cli-download

Conversation

@agrasth

@agrasth agrasth commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

The JFrog Tools Installer task fails to download the JFrog CLI with HTTP 401 when the Artifactory service connection uses OIDC, if the CLI is not already staged in the agent tool cache:

Downloading: ***artifactory/jfrog-cli-virtual/2.85.0/jfrog-cli-linux-amd64/jf
##[error]Error occurred while executing task: Failed while attempting to download JFrog CLI ...
Error: Unexpected HTTP response: 401

It works with access-token connections, and it works on agents where the CLI is already cached (the download is skipped entirely), which is why the failure only surfaces for OIDC + a fresh agent.

Root cause

JFrogToolsInstaller built the CLI-download authentication via createAuthHandlers(), which only understands static credentials:

  • apitoken → Bearer handler
  • username + password → Basic handler
  • otherwise → [] (anonymous)

An OIDC service connection carries none of those endpoint parameters — its credential only exists after an OIDC token exchange. So createAuthHandlers() returned [], the CLI was downloaded anonymously, and a private cliInstallationRepo responded 401.

The extension's existing OIDC exchange runs jf eot, i.e. it needs the CLI to already exist — which is a chicken-and-egg problem at install time, since the CLI is the very artifact being downloaded.

Fix

  • exchangeOidcTokenViaRest() — a CLI-independent OIDC exchange. Reuses the existing fetchAzureOidcToken() for the Azure DevOps ID token, then POSTs it to JFrog Access /access/api/v1/oidc/token with the same request jf eot sends for an Azure provider (grant_type / subject_token_type / subject_token / provider_name / provider_type=Azure / audience), and returns the exchanged access token.
  • createCliDownloadAuthHandlers() — routes OIDC connections through the REST exchange (Bearer of the exchanged token) and every other connection type through the existing createAuthHandlers() (unchanged).
  • Lazy resolution — the Tools Installer passes a handler provider; getCliPath() only invokes it when a download is actually required, so a cached CLI never triggers an exchange.
  • Extracted a shared resolvePlatformUrl() helper (dedup with the existing OIDC flow).

Backward compatibility

The only shared code path changed is getCliPath() (used by all 18 tasks); it accepts an array or a provider function, and arrays pass through untouched. Verified behavior across scenarios:

  • Existing task (no URL) → releases URL + [] handlers — unchanged
  • Existing task, CLI cached → no download — unchanged
  • Legacy array handlers + download URL → passed through unchanged
  • Installer, token connection → Bearer(apitoken), no exchange
  • Installer, OIDC connection → Bearer(exchanged token) authenticates the download (the fix)
  • Installer, OIDC + cached CLI → exchange never runs, no download
  • Exchange failure → task fails cleanly with the error surfaced

Token, basic, anonymous, cached, custom-path, releases-fallback and the extractors flow are all unchanged. tsc and eslint are clean on the changed files.

Tests

Adds unit tests covering the OIDC download path (Bearer built from the exchanged token) and the non-OIDC path (static credentials, no exchange).

…connections

The JFrog Tools Installer built the CLI download auth via createAuthHandlers(),
which only understands static credentials (access token / username+password).
For an OIDC service connection none of those endpoint parameters exist, so it
returned no auth handlers and downloaded the CLI anonymously — failing with
HTTP 401 on a private cliInstallationRepo. It only worked when the CLI was
already staged in the agent tool cache (the download was skipped entirely).

Add a CLI-independent OIDC token exchange (exchangeOidcTokenViaRest) that posts
the Azure DevOps ID token to JFrog Access /access/api/v1/oidc/token and uses the
returned access token as a Bearer credential for the download. The existing
CLI-based exchange cannot be used here because the CLI is the very artifact being
downloaded (chicken-and-egg).

createCliDownloadAuthHandlers() routes OIDC connections through the REST exchange
and all other connection types through the existing createAuthHandlers(), so
token/basic/anonymous behavior is unchanged. The handler provider is resolved
lazily in getCliPath(), so a cached CLI never triggers an exchange.

Adds unit tests covering the OIDC and non-OIDC CLI download auth paths.
The tests project never pinned typescript and its lockfile is not committed,
so `npm i` re-resolved ts-node's unbounded `typescript: >=2.7` peer to the
latest release. TypeScript 6.0.x breaks ts-node 10.9.2's config reader
(TypeError: Cannot read properties of undefined (reading 'fileExists')),
failing every suite before any test runs. This surfaced now (not on the last
green dev run) because TS 6.0 was published after it. Pin typescript to ^5.2.2
to match the root project and keep ts-node working.
@naveenku-jfrog naveenku-jfrog added the safe to test Approve running integration tests on a pull request label Jul 14, 2026
@github-actions github-actions Bot removed the safe to test Approve running integration tests on a pull request label Jul 14, 2026
@agrasth agrasth added the safe to test Approve running integration tests on a pull request label Jul 14, 2026
@github-actions github-actions Bot removed the safe to test Approve running integration tests on a pull request label Jul 14, 2026

@bhanurp bhanurp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ticket Alignment

  • Implements the ticket's core ask: JFrogToolsInstaller now authenticates the CLI download itself for OIDC-based Artifactory service connections, via a new CLI-independent REST OIDC exchange (exchangeOidcTokenViaRest) wired in through createCliDownloadAuthHandlers, matching the requested "use artifactoryConnection to do the OIDC exchange to download from cliInstallationRepo" behavior.
  • Nothing required by the ticket appears to be missing: the fix is backward compatible (static-credential and anonymous paths untouched, download still skipped entirely when the CLI is already cached, so no exchange runs), which covers all three of the ticket's reported/working scenarios (Windows cached, OIDC + fresh Linux agent, access-token connections).
  • Out of scope but reasonable/low-risk: extraction of resolvePlatformUrl() as a shared helper (dedup with the pre-existing CLI-based OIDC flow) and a typescript devDependency bump for the test project.

Findings Summary

Major

  1. jfrog-tasks-utils/utils.d.ts was not updated for this change. createCliDownloadAuthHandlers and exchangeOidcTokenViaRest are newly exported from utils.js but have no type declarations, and executeCliTask's cliAuthHandlers parameter is still typed as ifm.IRequestHandler[] even though toolsInstaller.js now passes a () => Promise<IRequestHandler[]> provider. This isn't hypothetical: the PR's own tests/tests.ts had to bypass the type system with (jfrogUtils as any).createCliDownloadAuthHandlers(...) (see lines 142 and 166) because the declared module type doesn't have the method. Any other TypeScript consumer of @jfrog/tasks-utils loses type safety/autocomplete for this new API and would hit the same compile error the tests worked around.

Minor
2. No test exercises the "lazy resolution" guarantee that is central to this fix's safety story (per the PR description: "a cached CLI never triggers an exchange"). The two new tests only unit-test createCliDownloadAuthHandlers in isolation; there's no test that drives getCliPath/executeCliTask with a pre-cached tool and asserts the auth-handler provider function is never invoked.
3. There's no negative-path test for exchangeOidcTokenViaRest (non-200 response or a response missing access_token) to confirm the CLI download task fails with the expected wrapped error message instead of an unhandled rejection.

Nit
4. createCliDownloadAuthHandlers and fetchOidcTokenIfConfigured now each independently re-implement the "is this an OIDC connection?" check (getEndpointAuthorizationParameter(service, 'oidcProviderName', true)). Worth factoring into a small shared isOidcConnection(service) helper alongside the already-extracted resolvePlatformUrl() so the two OIDC entry points can't drift apart later.

Overall the core fix is sound and well-targeted at the reported 401; the main gap is the stale type declarations forcing an unsafe cast in the test suite.

isToolExists: isToolExists,
buildCliArtifactoryDownloadUrl: buildCliArtifactoryDownloadUrl,
createAuthHandlers: createAuthHandlers,
createCliDownloadAuthHandlers: createCliDownloadAuthHandlers,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major: This newly exported createCliDownloadAuthHandlers (and exchangeOidcTokenViaRest below it) has no corresponding declaration in jfrog-tasks-utils/utils.d.ts. executeCliTask's cliAuthHandlers type there is also still ifm.IRequestHandler[] only, even though it must now also accept a () => Promise<IRequestHandler[]> provider. This is why tests/tests.ts had to call this function via (jfrogUtils as any) instead of the typed jfrogUtils import — any other TypeScript consumer of this package loses type safety for the new API. Suggested fix: update utils.d.ts to declare both new exports and widen executeCliTask's cliAuthHandlers param to ifm.IRequestHandler[] | (() => Promise<ifm.IRequestHandler[]>).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added createCliDownloadAuthHandlers, exchangeOidcTokenViaRest, isOidcConnection, resolvePlatformUrl to .d.ts; widened cliAuthHandlers to IRequestHandler[] | (() => Promise<IRequestHandler[]>)

Comment thread jfrog-tasks-utils/utils.js Outdated
* @returns {Promise<Array>} Authentication handlers for the CLI download.
*/
async function createCliDownloadAuthHandlers(serviceConnection, exchangeFn = exchangeOidcTokenViaRest) {
const oidcProviderName = tl.getEndpointAuthorizationParameter(serviceConnection, 'oidcProviderName', true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The OIDC-connection check (getEndpointAuthorizationParameter(serviceConnection, 'oidcProviderName', true)) is now duplicated here and in fetchOidcTokenIfConfigured. Consider extracting a small isOidcConnection(service) helper next to resolvePlatformUrl() so both OIDC entry points stay in sync if the detection logic ever changes.

@agrasth agrasth Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted isOidcConnection() helper, used in both createCliDownloadAuthHandlers and fetchOidcTokenIfConfigured


const statusCode = response.message.statusCode;
const responseBody = await response.readBody();
if (statusCode !== 200) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: No test covers this failure branch (non-200 response, or a 200 response missing access_token) for exchangeOidcTokenViaRest. Given this is the new code path that authenticates the CLI download itself, a test asserting the task fails with a clear error (rather than an unhandled rejection) when the OIDC exchange fails would be valuable regression coverage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added two tests: exchange HTTP failure (403) and missing access_token.

Comment thread tests/tests.ts
TestUtils.isSkipTest('unit'),
);

runSyncTest(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: These two tests validate createCliDownloadAuthHandlers in isolation but don't cover the "lazy resolution" claim from the PR description — that a cached CLI never triggers an OIDC exchange. Consider adding a test that drives getCliPath/executeCliTask with toolLib.findLocalTool (or the custom CLI path check) returning a hit, and asserts the auth-handler provider function passed in is never called.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "Cached CLI never triggers the OIDC auth-handler provider" test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants