Fix TestingBot credential resolution and Test Connection (#40)

* Fix TestingBot credential resolution and Test Connection

Two correctness fixes surfaced by an end-to-end run against a real account.

1. Credentials never resolved at build time.
   TestingBotCredentials extended BaseCredentials (implements StandardCredentials).
   CredentialsProvider.lookupCredentialsInItem(TestingBotCredentials.class, ...) —
   used by the build wrapper's getCredentials(), availableCredentials() and every
   credential dropdown — returned an empty list for it, even though the stored
   objects are instances of the class (a StandardCredentials.class lookup found
   them; a UsernamePasswordCredentialsImpl, which extends BaseStandardCredentials,
   is found by concrete class). As a result the freestyle wrapper silently
   resolved null credentials, so TB_KEY/TB_SECRET/TESTINGBOT_BUILD were never
   injected and the dropdowns were empty.

   Make TestingBotCredentials extend BaseStandardCredentials, which owns id and
   description (and the id-based equals/hashCode), so concrete-type lookups work.
   Old-format credentials.xml (scope/id/description/key/secret) still deserializes
   unchanged — field names are identical.

2. Test Connection failed for accounts without an email.
   doVerifyCredentials required a non-null user email; a valid account can return
   200 with a null email (e.g. sub-accounts), which produced a false 'Could not
   verify these credentials' for working key/secret. Treat any authenticated user
   as success, preferring the email and falling back to the account first name.

* Add regression tests for Test Connection result branches

Extract the user-facing result building of doVerifyCredentials into a pure,
package-private verificationResult(TestingbotUser) helper (behavior unchanged) so
it can be unit-tested without the network call or the Jenkins permission check
(a JenkinsRule test is not viable here — see the harness/Jetty conflict).

Cover: null user -> error; user with email -> success naming the email; user
without email but with a first name -> success naming the first name; user
missing email, first name and plan -> still success, with no identity or plan
fragment in the message.

Author: jochen-testingbot

src/main/java/testingbot/TestingBotCredentials.java

@@ -7,7 +7,6 @@
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.verb.POST;
 
-import com.cloudbees.plugins.credentials.BaseCredentials;
 import com.cloudbees.plugins.credentials.Credentials;
 import com.cloudbees.plugins.credentials.CredentialsDescriptor;
 import com.cloudbees.plugins.credentials.CredentialsMatcher;
@@ -28,7 +27,6 @@
 import com.testingbot.testingbotrest.TestingbotREST;
 import com.testingbot.testingbotrest.TestingbotUnauthorizedException;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
-import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.Util;
 import hudson.model.AbstractItem;
@@ -56,19 +54,15 @@
 import java.util.logging.Logger;
 
 @NameWith(value = TestingBotCredentials.NameProvider.class)
-public class TestingBotCredentials extends BaseCredentials implements StandardCredentials {
+public class TestingBotCredentials extends BaseStandardCredentials {
     private static final String CREDENTIAL_DISPLAY_NAME = "TestingBot";
 
-    private final String id;
-    private final String description;
     private final String key;
     private final Secret secret;
 
     @DataBoundConstructor
     public TestingBotCredentials(String id, String description, String key, String secret) {
-        super(CredentialsScope.GLOBAL);
-        this.id = IdCredentials.Helpers.fixEmptyId(id);
-        this.description = Util.fixNull(description);
+        super(CredentialsScope.GLOBAL, IdCredentials.Helpers.fixEmptyId(id), Util.fixNull(description));
         this.key = Util.fixNull(key);
         this.secret = Secret.fromString(secret);
     }
@@ -86,28 +80,6 @@ public String getDecryptedSecret() {
         return secret.getPlainText();
     }
 
-    @NonNull
-    @Exported
-    public String getDescription() {
-        return description;
-    }
-
-    @NonNull
-    @Exported
-    public String getId() {
-        return id;
-    }
-
-    @Override
-    public final boolean equals(Object o) {
-        return IdCredentials.Helpers.equals(this, o);
-    }
-
-    @Override
-    public final int hashCode() {
-        return IdCredentials.Helpers.hashCode(this);
-    }
-
     private static List<String> getLegacyCredentials() {
         DataInputStream in = null;
         BufferedReader br = null;
@@ -329,19 +301,33 @@ public FormValidation doVerifyCredentials(@QueryParameter String key, @QueryPara
             // fromString transparently yields the plaintext either way.
             String plainSecret = Secret.fromString(secret).getPlainText();
             try (TestingbotREST rest = new TestingbotREST(key.trim(), plainSecret)) {
-                TestingbotUser user = rest.getUserInfo();
-                if (user == null || Util.fixEmptyAndTrim(user.getEmail()) == null) {
-                    return FormValidation.error("Could not verify these credentials with TestingBot.");
-                }
-                String plan = Util.fixEmptyAndTrim(user.getPlan());
-                return FormValidation.ok("Connection successful — signed in as %s%s",
-                        user.getEmail(), plan != null ? " (" + plan + " plan)" : "");
+                return verificationResult(rest.getUserInfo());
             } catch (TestingbotUnauthorizedException | TestingbotApiException e) {
                 // The REST client's typed failures: bad key/secret, and network/parse errors (it wraps
                 // IOException into TestingbotApiException). Unexpected runtime exceptions propagate.
                 return FormValidation.error("TestingBot authentication failed: " + e.getMessage());
             }
         }
+
+        /**
+         * Builds the user-facing result for a fetched {@link TestingbotUser}. A null user means the
+         * credentials could not be verified; otherwise any authenticated user is a success. A valid
+         * account may not expose an email (e.g. sub-accounts), so prefer the email but fall back to
+         * the account's first name, and treat both the identity and the plan as optional.
+         */
+        static FormValidation verificationResult(TestingbotUser user) {
+            if (user == null) {
+                return FormValidation.error("Could not verify these credentials with TestingBot.");
+            }
+            String who = Util.fixEmptyAndTrim(user.getEmail());
+            if (who == null) {
+                who = Util.fixEmptyAndTrim(user.getFirstName());
+            }
+            String plan = Util.fixEmptyAndTrim(user.getPlan());
+            return FormValidation.ok("Connection successful%s%s",
+                    who != null ? " — signed in as " + who : "",
+                    plan != null ? " (" + plan + " plan)" : "");
+        }
     }
 
     public static class NameProvider extends CredentialsNameProvider<TestingBotCredentials> {

src/test/java/testingbot/TestingBotCredentialsVerificationTest.java

@@ -0,0 +1,59 @@
+package testingbot;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.testingbot.models.TestingbotUser;
+import hudson.util.FormValidation;
+import org.junit.Test;
+
+/**
+ * Covers {@link TestingBotCredentials.DescriptorImpl#verificationResult} — the Test Connection
+ * result branches — without touching the network or the Jenkins permission check: a null user
+ * fails, and any authenticated user succeeds even when the email, first name or plan are absent.
+ */
+public class TestingBotCredentialsVerificationTest {
+
+    @Test
+    public void nullUserFailsVerification() {
+        FormValidation r = TestingBotCredentials.DescriptorImpl.verificationResult(null);
+        assertThat(r.kind).isEqualTo(FormValidation.Kind.ERROR);
+        assertThat(r.getMessage()).contains("Could not verify");
+    }
+
+    @Test
+    public void userWithEmailSucceeds() {
+        TestingbotUser user = new TestingbotUser();
+        user.setEmail("jane@example.com");
+        user.setFirstName("Jane");
+        user.setPlan("Enterprise Plan");
+        FormValidation r = TestingBotCredentials.DescriptorImpl.verificationResult(user);
+        assertThat(r.kind).isEqualTo(FormValidation.Kind.OK);
+        assertThat(r.getMessage())
+                .contains("Connection successful")
+                .contains("jane@example.com")
+                .contains("Enterprise Plan plan");
+    }
+
+    @Test
+    public void userWithoutEmailFallsBackToFirstName() {
+        TestingbotUser user = new TestingbotUser();
+        user.setFirstName("demo");
+        user.setPlan("Free");
+        FormValidation r = TestingBotCredentials.DescriptorImpl.verificationResult(user);
+        assertThat(r.kind).isEqualTo(FormValidation.Kind.OK);
+        assertThat(r.getMessage())
+                .contains("signed in as demo")
+                .contains("Free plan")
+                .doesNotContain("@");
+    }
+
+    @Test
+    public void userMissingIdentityAndPlanStillSucceeds() {
+        TestingbotUser user = new TestingbotUser(); // no email, first name or plan
+        FormValidation r = TestingBotCredentials.DescriptorImpl.verificationResult(user);
+        assertThat(r.kind).isEqualTo(FormValidation.Kind.OK);
+        assertThat(r.getMessage()).contains("Connection successful");
+        assertThat(r.getMessage()).doesNotContain("signed in as");
+        assertThat(r.getMessage()).doesNotContain(" plan)");
+    }
+}