Merge pull request #503 from jenkinsci/revert-502-JENKINS-72611

Revert "[JENKINS-72611] Forbid editing credentials ID"
jenkinsci · Jan 26, 2024 · 7eb51b3 · 7eb51b3
2 parents 0ce519a + fc557c1
commit 7eb51b3
Show file tree

Hide file tree

Showing 6 changed files with 6 additions and 44 deletions.
diff --git a/src/main/java/com/cloudbees/plugins/credentials/SystemCredentialsProvider.java b/src/main/java/com/cloudbees/plugins/credentials/SystemCredentialsProvider.java
@@ -323,14 +323,6 @@ private synchronized boolean removeCredentials(@NonNull Domain domain, @NonNull
     private synchronized boolean updateCredentials(@NonNull Domain domain, @NonNull Credentials current,
                                                    @NonNull Credentials replacement) throws IOException {
         checkPermission(CredentialsProvider.UPDATE);
-
-        // See IdCredentials.Helper#equals.
-        // Note: We probably should not assume that all credentials are IdCredentials. We also do this a few lines below in list.indexOf(current) call.
-        // If this one breaks, that one breaks as well.
-        if (!current.equals(replacement)) {
-            throw new IllegalArgumentException("Credentials' IDs do not match, will not update.");
-        }
-
         Map<Domain, List<Credentials>> domainCredentialsMap = getDomainCredentialsMap();
         if (domainCredentialsMap.containsKey(domain)) {
             List<Credentials> list = domainCredentialsMap.get(domain);

diff --git a/src/main/java/com/cloudbees/plugins/credentials/UserCredentialsProvider.java b/src/main/java/com/cloudbees/plugins/credentials/UserCredentialsProvider.java
@@ -392,14 +392,6 @@ private synchronized boolean removeCredentials(@NonNull Domain domain, @NonNull
         private synchronized boolean updateCredentials(@NonNull Domain domain, @NonNull Credentials current,
                                                        @NonNull Credentials replacement) throws IOException {
             checkPermission(CredentialsProvider.UPDATE);
-
-            // See IdCredentials.Helper#equals.
-            // Note: We probably should not assume that all credentials are IdCredentials. We also do this a few lines below in list.indexOf(current) call.
-            // If this one breaks, that one breaks as well.
-            if (!current.equals(replacement)) {
-                throw new IllegalArgumentException("Credentials' IDs do not match, will not update.");
-            }
-
             Map<Domain, List<Credentials>> domainCredentialsMap = getDomainCredentialsMap();
             if (domainCredentialsMap.containsKey(domain)) {
                 List<Credentials> list = domainCredentialsMap.get(domain);

diff --git a/src/test/java/com/cloudbees/plugins/credentials/CredentialsProviderTest.java b/src/test/java/com/cloudbees/plugins/credentials/CredentialsProviderTest.java
@@ -41,7 +41,6 @@
 import hudson.security.ACL;
 import hudson.util.ListBoxModel;
 import jenkins.model.Jenkins;
-import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
@@ -256,7 +255,7 @@ public void testManageUserCredentials() throws IOException {
         final User alice = User.getById("alice", true);
         DummyCredentials aliceCred1 = new DummyCredentials(CredentialsScope.USER, "aliceCred1", "pwd");
         DummyCredentials aliceCred2 = new DummyCredentials(CredentialsScope.USER, "aliceCred2", "pwd");
-        DummyCredentials aliceCred3 = new DummyCredentials(CredentialsScope.USER, aliceCred1.getId(), aliceCred1.getDescription(), "aliceCred3", "pwd");
+        DummyCredentials aliceCred3 = new DummyCredentials(CredentialsScope.USER, "aliceCred3", "pwd");
 
         r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
 
@@ -290,7 +289,7 @@ public void testUpdateAndDeleteCredentials() throws IOException {
         DummyCredentials systemCred = new DummyCredentials(CredentialsScope.SYSTEM, "systemCred", "pwd");
         DummyCredentials systemCred2 = new DummyCredentials(CredentialsScope.SYSTEM, "systemCred2", "pwd");
         DummyCredentials globalCred = new DummyCredentials(CredentialsScope.GLOBAL, "globalCred", "pwd");
-        DummyCredentials modCredential = new DummyCredentials(CredentialsScope.GLOBAL, globalCred.getId(), globalCred.getDescription(), "modCredential", "pwd");
+        DummyCredentials modCredential = new DummyCredentials(CredentialsScope.GLOBAL, "modCredential", "pwd");
 
         CredentialsStore store = CredentialsProvider.lookupStores(Jenkins.get()).iterator().next();
 
@@ -458,14 +457,4 @@ public void credentialsSortedByNameInUI() {
         assertThat(options.get(0).value, is("2"));
         assertThat(options.get(1).value, is("1"));
     }
-
-    @Test
-    @Issue("SECURITY-3252")
-    public void cannotUpdateCredentialsId() {
-        DummyCredentials cred1 = new DummyCredentials(CredentialsScope.GLOBAL, "cred1", "pwd");
-        DummyCredentials cred2 = new DummyCredentials(CredentialsScope.GLOBAL, "cred2", "pwd");
-        CredentialsStore store = CredentialsProvider.lookupStores(Jenkins.get()).iterator().next();
-
-        Assert.assertThrows(IllegalArgumentException.class, () -> store.updateCredentials(Domain.global(), cred1, cred2));
-    }
 }
diff --git a/src/test/java/com/cloudbees/plugins/credentials/MockFolderCredentialsProvider.java b/src/test/java/com/cloudbees/plugins/credentials/MockFolderCredentialsProvider.java
@@ -330,14 +330,6 @@ private synchronized boolean removeCredentials(@NonNull Domain domain, @NonNull
         private synchronized boolean updateCredentials(@NonNull Domain domain, @NonNull Credentials current,
                                                        @NonNull Credentials replacement) throws IOException {
             checkPermission(CredentialsProvider.UPDATE);
-
-            // See IdCredentials.Helper#equals.
-            // Note: We probably should not assume that all credentials are IdCredentials. We also do this a few lines below in list.indexOf(current) call.
-            // If this one breaks, that one breaks as well.
-            if (!current.equals(replacement)) {
-                throw new IllegalArgumentException("Credentials' IDs do not match, will not update.");
-            }
-
             Map<Domain, List<Credentials>> domainCredentialsMap = getDomainCredentialsMap();
             if (domainCredentialsMap.containsKey(domain)) {
                 List<Credentials> list = domainCredentialsMap.get(domain);

diff --git a/src/test/java/com/cloudbees/plugins/credentials/domains/DomainTest.java b/src/test/java/com/cloudbees/plugins/credentials/domains/DomainTest.java
@@ -84,7 +84,7 @@ public void testCredentialsInCustomDomains() throws IOException {
         Domain domainBar = new Domain("domainBar", "Path domain", Arrays.asList(new DomainSpecification[] { new HostnameSpecification("bar.com", "") }));
         DummyCredentials systemCred = new DummyCredentials(CredentialsScope.SYSTEM, "systemCred", "pwd");
         DummyCredentials systemCred1 = new DummyCredentials(CredentialsScope.SYSTEM, "systemCred1", "pwd");
-        DummyCredentials systemCredMod = new DummyCredentials(CredentialsScope.SYSTEM, systemCred.getId(), systemCred.getDescription(), "systemCredMod", "pwd");
+        DummyCredentials systemCredMod = new DummyCredentials(CredentialsScope.SYSTEM, "systemCredMod", "pwd");
 
         CredentialsStore store = CredentialsProvider.lookupStores(Jenkins.get()).iterator().next();
 

diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/DummyCredentials.java b/src/test/java/com/cloudbees/plugins/credentials/impl/DummyCredentials.java
@@ -23,6 +23,7 @@
  */
 package com.cloudbees.plugins.credentials.impl;
 
+import com.cloudbees.plugins.credentials.BaseCredentials;
 import com.cloudbees.plugins.credentials.CredentialsDescriptor;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials;
@@ -34,19 +35,15 @@
 /**
  * A test credentials impl.
  */
-public class DummyCredentials extends BaseStandardCredentials implements UsernamePasswordCredentials {
+public class DummyCredentials extends BaseCredentials implements UsernamePasswordCredentials {
 
     private final String username;
 
     private final Secret password;
 
     @DataBoundConstructor
     public DummyCredentials(CredentialsScope scope, String username, String password) {
-        this(scope, null, null, username, password);
-    }
-
-    public DummyCredentials(CredentialsScope scope, String id, String description, String username, String password) {
-        super(scope, id, description);
+        super(scope);
         this.username = username;
         this.password = Secret.fromString(password);
     }