-
-
Notifications
You must be signed in to change notification settings - Fork 401
[JENKINS-69507] JGit fails 2nd and later https fetch with embedded username & password #1728
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
base: master
Are you sure you want to change the base?
Changes from all commits
628c808
4725f89
dd5a0eb
640df51
2595ee1
09ad243
30266b3
6a985b8
326c300
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| package org.jenkinsci.plugins.gitclient; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; | ||
| import hudson.model.TaskListener; | ||
| import hudson.util.StreamTaskListener; | ||
| import java.io.File; | ||
| import org.eclipse.jgit.transport.URIish; | ||
| import org.jenkinsci.plugins.gitclient.jgit.SmartCredentialsProvider; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
|
|
||
| /** | ||
| * Test that verifies JENKINS-69507 fix: JGit should extract and use | ||
| * credentials embedded in URLs (like https://user:pass@host/repo.git) | ||
| * for subsequent fetch operations. | ||
| * | ||
| * @author Akash Manna | ||
| */ | ||
| class JGitEmbeddedCredentialsTest { | ||
|
|
||
| @TempDir | ||
| File tempDir; | ||
|
|
||
| /** | ||
| * Test that extractAndAddEmbeddedCredentials properly extracts username and password | ||
| * from a URL and adds them to the credentials provider. | ||
| */ | ||
| @Test | ||
| void testExtractEmbeddedCredentials() throws Exception { | ||
| TaskListener listener = StreamTaskListener.fromStdout(); | ||
| JGitAPIImpl gitClient = new JGitAPIImpl(tempDir, listener); | ||
|
|
||
| URIish urlWithCredentials = new URIish("https://testuser:testpass@example.com/repo.git"); | ||
|
|
||
| SmartCredentialsProvider provider = gitClient.getProvider(); | ||
|
|
||
| var credsBefore = provider.getCredentials(); | ||
| assertFalse( | ||
| credsBefore.containsKey("https://testuser:testpass@example.com/repo.git") | ||
| || credsBefore.containsKey("https://example.com/repo.git"), | ||
| "Credentials should not exist before extraction"); | ||
|
|
||
| gitClient.addCredentials(urlWithCredentials.toString(), createTestCredentials("testuser", "testpass")); | ||
|
|
||
| var credsAfter = provider.getCredentials(); | ||
| assertTrue(credsAfter.size() > credsBefore.size(), "Credentials should be added"); | ||
| } | ||
|
Comment on lines
+26
to
+49
|
||
|
|
||
| /** | ||
| * Test that URLs without credentials don't cause issues | ||
| */ | ||
| @Test | ||
| void testUrlWithoutCredentials() throws Exception { | ||
| TaskListener listener = StreamTaskListener.fromStdout(); | ||
| JGitAPIImpl gitClient = new JGitAPIImpl(tempDir, listener); | ||
|
|
||
| URIish urlWithoutCredentials = new URIish("https://example.com/repo.git"); | ||
|
|
||
| assertDoesNotThrow(() -> { | ||
| gitClient.addCredentials(urlWithoutCredentials.toString(), createTestCredentials("user", "pass")); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Test that URLs with only username (no password) are handled correctly | ||
| */ | ||
| @Test | ||
| void testUrlWithOnlyUsername() throws Exception { | ||
| TaskListener listener = StreamTaskListener.fromStdout(); | ||
| JGitAPIImpl gitClient = new JGitAPIImpl(tempDir, listener); | ||
|
|
||
| URIish urlWithOnlyUser = new URIish("https://testuser@example.com/repo.git"); | ||
|
|
||
| assertDoesNotThrow(() -> { | ||
| gitClient.addCredentials(urlWithOnlyUser.toString(), createTestCredentials("testuser", "pass")); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Test the scenario described in JENKINS-69507: credentials should be extracted | ||
| * when a remote name is resolved to a URL with embedded credentials. | ||
| * This simulates the case where: | ||
| * 1. First fetch works with URL containing credentials | ||
| * 2. URL is stored in git config as remote "origin" | ||
| * 3. Second fetch using remote name "origin" should extract credentials from the stored URL | ||
| */ | ||
| @Test | ||
| void testRemoteNameResolutionWithEmbeddedCredentials() throws Exception { | ||
| TaskListener listener = StreamTaskListener.fromStdout(); | ||
| JGitAPIImpl gitClient = new JGitAPIImpl(tempDir, listener); | ||
|
|
||
| gitClient.init_().workspace(tempDir.getAbsolutePath()).execute(); | ||
|
|
||
| String urlWithCreds = "https://testuser:testpass@example.com/repo.git"; | ||
| gitClient.setRemoteUrl("origin", urlWithCreds); | ||
|
|
||
| String storedUrl = gitClient.getRemoteUrl("origin"); | ||
| assertNotNull(storedUrl, "Remote URL should be stored"); | ||
| assertEquals(urlWithCreds, storedUrl, "Stored URL should match"); | ||
|
|
||
| URIish resolvedUrl = new URIish(storedUrl); | ||
| assertEquals("testuser", resolvedUrl.getUser(), "Username should be in URL"); | ||
| assertEquals("testpass", resolvedUrl.getPass(), "Password should be in URL"); | ||
| } | ||
|
Comment on lines
+89
to
+106
|
||
|
|
||
| /** | ||
| * Static inner class for test credentials to avoid SpotBugs warnings. | ||
| */ | ||
| private static class TestCredentials implements StandardUsernamePasswordCredentials { | ||
| private final String username; | ||
| private final String password; | ||
|
|
||
| TestCredentials(String username, String password) { | ||
| this.username = username; | ||
| this.password = password; | ||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return "Test credentials"; | ||
| } | ||
|
|
||
| @Override | ||
| public String getId() { | ||
| return "test-id"; | ||
| } | ||
|
|
||
| @Override | ||
| public com.cloudbees.plugins.credentials.CredentialsScope getScope() { | ||
| return com.cloudbees.plugins.credentials.CredentialsScope.GLOBAL; | ||
| } | ||
|
|
||
| @Override | ||
| public com.cloudbees.plugins.credentials.CredentialsDescriptor getDescriptor() { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getUsername() { | ||
| return username; | ||
| } | ||
|
|
||
| @Override | ||
| public hudson.util.Secret getPassword() { | ||
| return hudson.util.Secret.fromString(password); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Helper method to create test credentials | ||
| */ | ||
| private StandardUsernamePasswordCredentials createTestCredentials(String username, String password) { | ||
| return new TestCredentials(username, password); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getDescriptor() method throws UnsupportedOperationException. While this is unlikely to be called for embedded credentials, if any code in the Jenkins ecosystem attempts to retrieve the descriptor (e.g., for serialization or UI purposes), it will fail. Consider implementing a minimal descriptor or documenting why this is safe to throw an exception here.