Skip to content

refactor: change authentication order and seperate out existing crede…#296

Open
BartoszBlizniak wants to merge 9 commits into
masterfrom
bblizniak/credential-chain-priority
Open

refactor: change authentication order and seperate out existing crede…#296
BartoszBlizniak wants to merge 9 commits into
masterfrom
bblizniak/credential-chain-priority

Conversation

@BartoszBlizniak
Copy link
Copy Markdown
Member

Description

Establishes explicit credential resolution priority for the CLI:
--api-key flag > CLOUDSMITH_API_KEY env var > credentials.ini > keyring SSO

Introduces two new providers (EnvVarProvider, CredentialsFileProvider) and splits the single api_key field in CredentialContext into per-source fields (api_key_from_flag, api_key_from_env, api_key_from_file), allowing the chain to evaluate each source independently and in the correct order.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Other (please describe)

Additional Notes

  • Default chain order changed from Keyring → CLIFlag to CLIFlag → EnvVar → CredentialsFile → Keyring
  • api_key_from_file capture moved to common_cli_config_options (co-located with load_creds_file) to remove implicit cross-decorator ordering dependency
  • Full test coverage added for each provider in isolation and all priority combinations

Base automatically changed from iduffy/credential-provider-chain to master May 20, 2026 09:49
@BartoszBlizniak
Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: BartoszBlizniak <55028730+BartoszBlizniak@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented May 20, 2026

@copilot resolve the merge conflicts in this pull request

Resolved in merge commit 08d7868: I merged origin/master into this branch and fixed all merge conflicts while preserving the credential-chain priority changes.

@BartoszBlizniak BartoszBlizniak marked this pull request as ready for review May 20, 2026 10:17
@BartoszBlizniak BartoszBlizniak requested a review from a team as a code owner May 20, 2026 10:17
Copilot AI review requested due to automatic review settings May 20, 2026 10:17
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@BartoszBlizniak
Copy link
Copy Markdown
Member Author

@claude review once

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors credential resolution in the CLI to enforce an explicit priority order across multiple credential sources (CLI flag, env var, credentials.ini, then keyring SSO), and adds provider-level + chain-level tests to validate the precedence.

Changes:

  • Adds EnvVarProvider and CredentialsFileProvider, and expands the default provider chain order to CLIFlag → EnvVar → CredentialsFile → Keyring.
  • Splits CredentialContext.api_key into per-source fields (api_key_from_flag, api_key_from_env, api_key_from_file) and wires these through CLI option/config loading.
  • Adds unit tests for each provider and integration tests covering all priority combinations.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
cloudsmith_cli/core/credentials/providers/env_var.py New provider for resolving API keys from env-derived context.
cloudsmith_cli/core/credentials/providers/credentials_file.py New provider for resolving API keys from credentials-file-derived context.
cloudsmith_cli/core/credentials/providers/cli_flag.py Updates CLI flag provider to read api_key_from_flag and improves source detail.
cloudsmith_cli/core/credentials/providers/init.py Exposes the new providers via package exports.
cloudsmith_cli/core/credentials/models.py Splits credential context into per-source fields.
cloudsmith_cli/core/credentials/chain.py Updates default provider chain order and imports new providers.
cloudsmith_cli/cli/decorators.py Captures per-source API keys (flag/env/file) and passes them into CredentialContext.
cloudsmith_cli/cli/config.py Adds Options accessors for per-source API key fields.
cloudsmith_cli/core/tests/test_cli_flag_provider.py Updates tests for new CLI-flag-only context field.
cloudsmith_cli/core/tests/test_env_var_provider.py Adds unit tests for EnvVarProvider.
cloudsmith_cli/core/tests/test_credentials_file_provider.py Adds unit tests for CredentialsFileProvider.
cloudsmith_cli/core/tests/test_credential_provider_chain.py Updates default-chain ordering expectations.
cloudsmith_cli/core/tests/test_credential_context.py Adds tests for new per-source context fields.
cloudsmith_cli/core/tests/test_credential_chain_priority.py Adds integration tests to validate precedence across all sources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cloudsmith_cli/cli/decorators.py Outdated
Comment thread cloudsmith_cli/core/credentials/providers/env_var.py
Comment thread cloudsmith_cli/core/credentials/providers/credentials_file.py
Comment thread cloudsmith_cli/cli/decorators.py Outdated
Comment thread cloudsmith_cli/cli/decorators.py
Comment thread cloudsmith_cli/cli/decorators.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants