Skip to content

Fix TestingBot credential resolution and Test Connection #40

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.

Sign up for GitHub

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

Merged
jochen-testingbot merged 2 commits into from
Jul 17, 2026
Merged

Fix TestingBot credential resolution and Test Connection #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6f8dc4
Fix TestingBot credential resolution and Test Connection
jochen-testingbot Jul 17, 2026
f9e5f94
Add regression tests for Test Connection result branches
jochen-testingbot Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 23 additions & 37 deletions src/main/java/testingbot/TestingBotCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Comment on lines 305 to 308

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Distinguish API/connectivity failures from invalid credentials.

TestingbotApiException also covers network/parse errors, but this branch labels all of them as authentication failures. Catch it separately and return a connection/API error; retain the authentication message for TestingbotUnauthorizedException.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/testingbot/TestingBotCredentials.java` around lines 305 - 308,
Update the exception handling around the TestingBot credential validation to
catch TestingbotUnauthorizedException separately and retain the
authentication-failed message only for that exception. Handle
TestingbotApiException in its own branch with a connection/API failure message,
preserving the existing exception details in both responses.

}
}

/**
* 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> {
Expand Down
59 changes: 59 additions & 0 deletions src/test/java/testingbot/TestingBotCredentialsVerificationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)");
}
}