Add ERROR_COMMITTER and ERROR_AUTHOR codes

src/main/java/org/eclipsefoundation/git/eca/namespace/APIStatusCode.java

@@ -13,7 +13,7 @@ import com.fasterxml.jackson.annotation.JsonValue;
 
 public enum APIStatusCode {
 	SUCCESS_DEFAULT(200), SUCCESS_COMMITTER(201), SUCCESS_CONTRIBUTOR(202), ERROR_DEFAULT(-401), ERROR_SIGN_OFF(-402),
-	ERROR_SPEC_PROJECT(-403);
+	ERROR_SPEC_PROJECT(-403), ERROR_AUTHOR(-404), ERROR_COMMITTER(-405), ERROR_PROXY_PUSH(-406);
 
 	private int code;
 

src/main/java/org/eclipsefoundation/git/eca/resource/ValidationResource.java

@@ -199,7 +199,7 @@ public class ValidationResource {
               "Could not find an Eclipse user with mail '%1$s' for author of commit %2$s",
               author.getMail(), c.getHash()),
           c.getHash());
-      addError(response, "Author must have an Eclipse Account", c.getHash());
+      addError(response, "Author must have an Eclipse Account", c.getHash(), APIStatusCode.ERROR_AUTHOR);
       return true;
     }
 
@@ -221,57 +221,76 @@ public class ValidationResource {
               "Could not find an Eclipse user with mail '%1$s' for committer of commit %2$s",
               committer.getMail(), c.getHash()),
           c.getHash());
-      addError(response, "Committing user must have an Eclipse Account", c.getHash());
+      addError(response, "Committing user must have an Eclipse Account", c.getHash(), APIStatusCode.ERROR_COMMITTER);
       return true;
     }
     // validate author access to the current repo
-    validateAuthorAccess(response, c, eclipseAuthor, filteredProjects, provider);
+    validateUserAccess(response, c, eclipseAuthor, filteredProjects, APIStatusCode.ERROR_AUTHOR);
+
+    // check committer general access
+    boolean isCommittingUserCommitter = isCommitter(response, eclipseCommitter, c.getHash(), filteredProjects);
+    validateUserAccessPartial(response, c, eclipseCommitter, isCommittingUserCommitter, APIStatusCode.ERROR_COMMITTER);
 
     // only committers can push on behalf of other users
-    if (response.isTrackedProject()
-        && !eclipseAuthor.equals(eclipseCommitter)
-        && !isCommitter(response, eclipseCommitter, c.getHash(), filteredProjects, provider)) {
+    if (response.isTrackedProject() && !eclipseAuthor.equals(eclipseCommitter) && !isCommittingUserCommitter) {
       addMessage(response, "You are not a project committer.", c.getHash());
       addMessage(response, "Only project committers can push on behalf of others.", c.getHash());
-      addError(response, "You must be a committer to push on behalf of others.", c.getHash());
+      addError(response, "You must be a committer to push on behalf of others.", c.getHash(), APIStatusCode.ERROR_PROXY_PUSH);
     }
     return true;
   }
 
+
+
   /**
    * Validates author access for the current commit. If there are errors, they are recorded in the
    * response for the current request to be returned once all validation checks are completed.
    *
    * @param r the current response object for the request
    * @param c the commit that is being validated
-   * @param eclipseAuthor the user to validate on a branch
+   * @param eclipseUser the user to validate on a branch
    * @param filteredProjects tracked projects for the current request
-   * @param provider the provider set for the current request
+   * @param errorCode the error code to display if the user does not have access
    */
-  private void validateAuthorAccess(
+  private void validateUserAccess(
       ValidationResponse r,
       Commit c,
-      EclipseUser eclipseAuthor,
-      List<Project> filteredProjects,
-      ProviderType provider) {
-    // check if the author matches to an eclipse user and is a committer
-    if (isCommitter(r, eclipseAuthor, c.getHash(), filteredProjects, provider)) {
-      addMessage(r, "The author is a committer on the project.", c.getHash());
+      EclipseUser eclipseUser,
+      List<Project> filteredProjects, APIStatusCode errorCode) {
+    // call isCommitter inline and pass to partial call 
+    validateUserAccessPartial(r, c, eclipseUser, isCommitter(r, eclipseUser, c.getHash(), filteredProjects), errorCode);
+  }
+
+  /**
+   * Allows for isCommitter to be called external to this method. This was extracted to ensure that isCommitter isn't 
+   * called twice for the same user when checking committer proxy push rules and committer general access.
+   * 
+   * @param r the current response object for the request
+   * @param c the commit that is being validated
+   * @param eclipseUser the user to validate on a branch
+   * @param isCommitter the results of the isCommitter call from this class.
+   * @param errorCode the error code to display if the user does not have access
+   */
+  private void validateUserAccessPartial(ValidationResponse r, Commit c, EclipseUser eclipseUser, 
+        boolean isCommitter, APIStatusCode errorCode) {
+    if (isCommitter) {
+      addMessage(r, String.format("Eclipse user '%s' is a committer on the project.", eclipseUser.getName()), c.getHash());
     } else {
-      addMessage(r, "The author is not a committer on the project.", c.getHash());
+      addMessage(r, String.format("Eclipse user '%s' is not a committer on the project.", eclipseUser.getName()), c.getHash());
       // check if the author is signed off if not a committer
-      if (eclipseAuthor.getEca().isSigned()) {
+      if (eclipseUser.getEca().isSigned()) {
         addMessage(
             r,
-            "The author has a current Eclipse Contributor Agreement (ECA) on file.",
+            String.format("Eclipse user '%s' has a current Eclipse Contributor Agreement (ECA) on file.", eclipseUser.getName()),
             c.getHash());
       } else {
         addMessage(
             r,
-            "The author does not have a current Eclipse Contributor Agreement (ECA) on file.\n"
-                + "If there are multiple commits, please ensure that each author has a ECA.",
+            String.format("Eclipse user '%s' does not have a current Eclipse Contributor Agreement (ECA) on file.\n"
+                + "If there are multiple commits, please ensure that each author has a ECA.", eclipseUser.getName()),
             c.getHash());
-        addError(r, "An Eclipse Contributor Agreement is required.", c.getHash());
+        addError(r, String.format("An Eclipse Contributor Agreement is required for Eclipse user '%s'.", eclipseUser.getName()),
+            c.getHash(), errorCode);
       }
     }
   }
@@ -288,15 +307,13 @@ public class ValidationResource {
    * @param user the user to validate on a branch
    * @param hash the hash of the commit that is being validated
    * @param filteredProjects tracked projects for the current request
-   * @param provider the provider set for the current request
    * @return true if user is considered a committer, false otherwise.
    */
   private boolean isCommitter(
       ValidationResponse r,
       EclipseUser user,
       String hash,
-      List<Project> filteredProjects,
-      ProviderType provider) {
+      List<Project> filteredProjects) {
     // iterate over filtered projects
     for (Project p : filteredProjects) {
       LOGGER.debug("Checking project '{}' for user '{}'", p.getName(), user.getName());

src/test/java/org/eclipsefoundation/git/eca/api/MockAccountsAPI.java

@@ -11,7 +11,6 @@ package org.eclipsefoundation.git.eca.api;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
 import javax.annotation.PostConstruct;

src/test/java/org/eclipsefoundation/git/eca/resource/ValidationResourceTest.java

@@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test;
 
 import io.quarkus.test.junit.QuarkusTest;
 import io.restassured.http.ContentType;
+import io.restassured.response.ValidatableResponse;
 
 /**
  * Tests for verifying end to end validation via the endpoint. Uses restassured to create pseudo
@@ -441,6 +442,7 @@ class ValidationResourceTest {
 
     // test output w/ assertions
     // Should be invalid as Grunt does not have spec project write access
+    // Should have 2 errors, as both users get validated
     given()
         .body(vr)
         .contentType(ContentType.JSON)
@@ -452,7 +454,7 @@ class ValidationResourceTest {
             "passed",
             is(false),
             "errorCount",
-            is(1),
+            is(2),
             "commits.123456789abcdefghijklmnop.errors[0].code",
             is(APIStatusCode.ERROR_SPEC_PROJECT.getValue()));
   }
@@ -526,17 +528,21 @@ class ValidationResourceTest {
   }
 
   @Test
-  void validateNoECA() throws URISyntaxException {
+  void validateNoECA_author() throws URISyntaxException {
     // set up test users
     GitUser g1 = new GitUser();
     g1.setName("Newbie Anon");
     g1.setMail("newbie@important.co");
 
+    GitUser g2 = new GitUser();
+    g2.setName("The Wizard");
+    g2.setMail("code.wiz@important.co");
+
     List<Commit> commits = new ArrayList<>();
     // create sample commits
     Commit c1 = new Commit();
     c1.setAuthor(g1);
-    c1.setCommitter(g1);
+    c1.setCommitter(g2);
     c1.setBody(String.format("Signed-off-by: %s <%s>", g1.getName(), g1.getMail()));
     c1.setHash("123456789abcdefghijklmnop");
     c1.setSubject("All of the things");
@@ -560,6 +566,79 @@ class ValidationResourceTest {
         .body("passed", is(false), "errorCount", is(1));
   }
 
+  @Test
+  void validateNoECA_committer() throws URISyntaxException {
+    // set up test users
+    GitUser g1 = new GitUser();
+    g1.setName("Newbie Anon");
+    g1.setMail("newbie@important.co");
+
+    GitUser g2 = new GitUser();
+    g2.setName("The Wizard");
+    g2.setMail("code.wiz@important.co");
+
+    List<Commit> commits = new ArrayList<>();
+    // create sample commits
+    Commit c1 = new Commit();
+    c1.setAuthor(g2);
+    c1.setCommitter(g1);
+    c1.setBody(String.format("Signed-off-by: %s <%s>", g2.getName(), g2.getMail()));
+    c1.setHash("123456789abcdefghijklmnop");
+    c1.setSubject("All of the things");
+    c1.setParents(Arrays.asList("46bb69bf6aa4ed26b2bf8c322ae05bef0bcc5c10"));
+    commits.add(c1);
+
+    ValidationRequest vr = new ValidationRequest();
+    vr.setProvider(ProviderType.GITHUB);
+    vr.setRepoUrl(new URI("http://www.github.com/eclipsefdn/sample"));
+    vr.setCommits(commits);
+    // test output w/ assertions
+    // Error count should be 2 - first is committer access, then proxy push violation
+    // Status 403 (forbidden) is the standard return for invalid requests
+    given()
+        .body(vr)
+        .contentType(ContentType.JSON)
+        .when()
+        .post("/eca")
+        .then()
+        .statusCode(403)
+        .body("passed", is(false), "errorCount", is(2));
+  }
+  @Test
+  void validateNoECA_both() throws URISyntaxException {
+    // set up test users
+    GitUser g1 = new GitUser();
+    g1.setName("Newbie Anon");
+    g1.setMail("newbie@important.co");
+
+    List<Commit> commits = new ArrayList<>();
+    // create sample commits
+    Commit c1 = new Commit();
+    c1.setAuthor(g1);
+    c1.setCommitter(g1);
+    c1.setBody(String.format("Signed-off-by: %s <%s>", g1.getName(), g1.getMail()));
+    c1.setHash("123456789abcdefghijklmnop");
+    c1.setSubject("All of the things");
+    c1.setParents(Arrays.asList("46bb69bf6aa4ed26b2bf8c322ae05bef0bcc5c10"));
+    commits.add(c1);
+
+    ValidationRequest vr = new ValidationRequest();
+    vr.setProvider(ProviderType.GITHUB);
+    vr.setRepoUrl(new URI("http://www.github.com/eclipsefdn/sample"));
+    vr.setCommits(commits);
+    // test output w/ assertions
+    // Should have 2 errors, 1 for author entry and 1 for committer entry
+    // Status 403 (forbidden) is the standard return for invalid requests
+    given()
+        .body(vr)
+        .contentType(ContentType.JSON)
+        .when()
+        .post("/eca")
+        .then()
+        .statusCode(403)
+        .body("passed", is(false), "errorCount", is(2));
+  }
+
   @Test
   void validateAuthorNoEclipseAccount() throws URISyntaxException {
     // set up test users