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

Authentication fails when trying to resolve a target platform over an authenticated mirror that is a composite repository #3521

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

kevloral
Copy link
Contributor

@kevloral kevloral commented Feb 20, 2024

Test created for reproducing the bug as described in #3501:

  • testTargetDefinitionAuthMirror: tries to resolve a target definition from a composite p2 repository accessed over an authenticated composite mirror.

On the other hand, the following tests pass and have been created just for completion:

  • testAuthMirror: tries to access a composite p2 repository over an authenticated composite mirror.
  • testMirror: tries to access a composite p2 repository over a composite mirror with no authentication.
  • testRepositoryEncrypted: tries to access an authenticated composite p2 repository whose password is encrypted.
  • testTargetDefinition: tries to resolve a target platform from a composite p2 repository.
  • testTargetDefinitionEncrypted: tries to resolve a target definition from an authenticated composite p2 repository whose password is encrypted.
  • testTargetDefinitionMirror: tries to resolve a target definition from a composite p2 repository accessed over a composite mirror with no authentication.

Copy link

github-actions bot commented Feb 20, 2024

Test Results

  588 files  ± 0    588 suites  ±0   3h 35m 55s ⏱️ - 13m 20s
  405 tests + 7    398 ✅ + 7   7 💤 ±0  0 ❌ ±0 
1 215 runs  +21  1 193 ✅ +21  22 💤 ±0  0 ❌ ±0 

Results for commit df0fd65. ± Comparison against base commit 9920425.

♻️ This comment has been updated with latest results.

@kevloral
Copy link
Contributor Author

The problem lied in the fact that RemoteMetadataRepositoryManager was passing the original location to the MavenAuthenticator, instead of the effective one:

On the other hand, RemoteArtifactRepositoryManager was already passing the effective URI instead of the original one, so no change was needed there:

@@ -63,7 +63,7 @@ public IMetadataRepository loadRepository(URI location, IProgressMonitor monitor
public IMetadataRepository loadRepository(URI location, int flags, IProgressMonitor monitor)
throws ProvisionException, OperationCanceledException {
URI effectiveLocation = translateAndPrepareLoad(location);
authenticator.enterLoad(location);
authenticator.enterLoad(effectiveLocation);
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

@laeubi laeubi added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label Mar 18, 2024
RemoteMetadataRepositoryManager now passes the effective URI to the
MavenAuthenticator instead of the original one. This fixes eclipse-tycho#3501.

This commit also includes the test that was used to reproduce the bug:

- testTargetDefinitionAuthMirror: tries to resolve a target definition
from a composite p2 repository accessed over an authenticated composite
mirror.

Also, several other tests have been created just for completion:

- testAuthMirror: tries to access a composite p2 repository over an
authenticated composite mirror.
- testMirror: tries to access a composite p2 repository over a composite
mirror with no authentication.
- testRepositoryEncrypted: tries to access an authenticated composite p2
repository whose password is encrypted.
- testTargetDefinition: tries to resolve a target platform from a
composite p2 repository.
- testTargetDefinitionEncrypted: tries to resolve a target definition
from an authenticated composite p2 repository whose password is
encrypted.
- testTargetDefinitionMirror: tries to resolve a target definition from
a composite p2 repository accessed over a composite mirror with no
authentication.
@laeubi laeubi enabled auto-merge (rebase) March 18, 2024 05:48
@laeubi laeubi merged commit d6366ad into eclipse-tycho:main Mar 18, 2024
10 checks passed
@eclipse-tycho-bot
Copy link

💔 All backports failed

Status Branch Result
tycho-4.0.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 3521

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants