Skip to content

OIDC Logic Isolation and Backward Compatibility Improvements #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Apr 27, 2025

Conversation

EyalDelarea
Copy link
Contributor

@EyalDelarea EyalDelarea commented Apr 21, 2025

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • I used npm run format for formatting the code before submitting the pull request.

🎯 Purpose

This PR introduces a major refactor to the setup-jfrog-cli action to improve readability, testability, and backward compatibility around OpenID Connect (OIDC) authentication. The core goal is to isolate OIDC logic from general utility logic, while ensuring older CLI versions continue to work without regressions.


💡 Motivation

Previously, we relied on jf c add with OIDC parameters. However, this approach does not expose the required step outputs (oidc-user, oidc-token) that users depend on for downstream authentication flows (e.g., docker login, helm repo add, etc.).

To solve this:

  • We now use jf eot internally to exchange the OIDC token and manually set the access token in configuration.
  • This ensures compatibility with older workflows while unlocking output control.

Implementing this flow cleanly across CLI versions required isolating the OIDC logic, which led to a broader refactor to reduce complexity and overhead.


✅ What's Included

🧱 Codebase Structure Refactor

  • Extracted OIDC-related logic to a new module: oidc-utils.ts
  • Split additional logic into:
    • utils.ts (core reusable logic)
    • jobsummary-utils.ts (markdown/summary formatting)
    • types.ts (centralized interfaces)
  • Added strict type annotations throughout the codebase per ESLint rules

🔐 OIDC Token Exchange Flow

  • Uses jf eot if CLI version ≥ 2.75.0
  • Falls back to manual exchange if CLI < 2.75.0
  • Always sets step outputs:
    • oidc-user
    • oidc-token
  • Exports usage tracking flags:
    • JFROG_CLI_USAGE_CONFIG_OIDC
    • JFROG_CLI_USAGE_OIDC_USED

🧪 Improved Test Coverage

  • Split test suite into:
    • utils.spec.ts
    • oidc-utils.spec.ts
    • jobsummary-utils.spec.ts
  • Added cases for:
    • CLI-based vs manual OIDC exchange
    • Step outputs across CLI versions
    • JSON and regex-based CLI output parsing
    • Usage tracking behavior

🧬 Integration Workflow Overhaul

  • Introduced new matrix-based GitHub Actions workflow: oidc-integration-test.yml
  • Runs on:
    • OS: ubuntu, macos, windows
    • CLI versions: 2.74.1, 2.75.0, latest
  • Dynamically creates and deletes real OIDC providers + mappings using JFROG_PLATFORM_URL
  • Validates:
    • CLI connectivity via jf rt ping
    • Presence of oidc-user and oidc-token outputs
    • Compatibility with naming rules (e.g. replacing . in CLI versions with -)

🔍 Why This Matters

  • ✅ Enables the action to support both modern and legacy CLI versions without breakage
  • ✅ Removes 1000+ lines of entangled logic and paves the way for easier long-term maintenance
  • ✅ Ensures plugin authors and integrators can rely on consistent output behavior, no matter the underlying auth mechanism

@EyalDelarea EyalDelarea added safe to test Approve running integration tests on a pull request improvement Automatically generated release notes labels Apr 21, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Apr 21, 2025
@EyalDelarea EyalDelarea linked an issue Apr 21, 2025 that may be closed by this pull request
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Apr 23, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Apr 23, 2025
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Apr 24, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Apr 24, 2025
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Apr 24, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Apr 24, 2025
stderr: 'boom',
};
(getExecOutput as jest.Mock).mockResolvedValueOnce(mockOutput);
it('should correctly set step outputs for CLI token exchange', async (): Promise<void> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a difference between this test and the one above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we have a test to make sure we export the steps output with CLI exchange or manual exchange.
They have different flow

@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Apr 27, 2025
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Apr 27, 2025
Copy link

👍 Frogbot scanned this pull request and did not find any new security issues.


@EyalDelarea EyalDelarea merged commit 4535063 into jfrog:master Apr 27, 2025
18 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Automatically generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OIDC Regression in v4.5.7 – Worked in v4.5.6
2 participants