Skip to content
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

(#3566) Ensure package download from authed source #3577

Merged

Conversation

corbob
Copy link
Member

@corbob corbob commented Nov 28, 2024

Description Of Changes

Update the Credential lookup logic to account for the scenario where NuGet has forgotten the credentials for the source between determining the download Uri and trying to download from it. In this instance the target Uri is a superstring of the source Uri (that is for a source of https://repo/repository/my-repository/, the download might be https://repo/repository/my-repository/my-package/1.1.1). This would not match the credential lookup, but it should use the configured credential.

Motivation and Context

Sometimes NuGet forgets where it's downloading from, and needs to get the credentials again.

Testing

Ran tests through Test Kitchen.

Operating Systems Testing

  • Windows Server 2016
  • Windows Server 2019

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v3 compatibility checked?

Related Issue

Fixes #3566

Update the Credential lookup logic to account for the scenario where
NuGet has forgotten the credentials for the source between determining
the download Uri and trying to download from it. In this instance the
target Uri is a superstring of the source Uri (that is for a source of
`https://repo/repository/my-repository/`, the download might be
`https://repo/repository/my-repository/my-package/1.1.1`). This would
not match the credential lookup, but it should use the configured
credential.
@corbob
Copy link
Member Author

corbob commented Nov 28, 2024

PR is draft while I work out a few other Test Kitchen issues.

This commit fixes hanging tests and bring the changes from 981a5d9 into
the support branch.

Going forward, the official signing certificate will only be in play on
tagged builds.  As such, we can't easily assert on the certificate
thumbprint. Instead, let's check on the Issuer name, and if the
official one, check for certificate being valid, otherwise let's assert
on it not being correct.  This is similar to what is now done elsewhere
when checking for signatures, i.e. in CCR and Chocolatey.Cake.Recipe.
@corbob corbob marked this pull request as ready for review November 29, 2024 00:08
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

LGTM, I will however hold off on merging this until the test kitchen run has finished for this PR.

Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

This PR looks to have increased the amount of failing tests that needs to be investigated before we can merge this one.

Update the wording used on push failures to capture accurate results
even with slightly different wording between Team City and local runs.
When upgrading Chocolatey, if the certificate subject doesn't match a
known Chocolatey certificate we don't replace the file. The certificate
used for signing changed slightly, this adds the new subject to the
check.
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

LGTM, and since the TC build succeeded this time, I'll get this merged.

@AdmiringWorm AdmiringWorm merged commit b584326 into chocolatey:hotfix/1.4.1 Nov 29, 2024
5 checks passed
@corbob corbob deleted the fix-authed-package-download branch November 29, 2024 17:01
@corbob corbob mentioned this pull request Nov 29, 2024
10 tasks
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.

2 participants