Commit 070fad17 authored by Martin Lowe's avatar Martin Lowe 🇨🇦 Committed by Martin Lowe
Browse files

Update to bot checking logic to check for project and any matching email



Previously, check was only looking if user is a bot, rather than
checking for specific project bots. Even then, only checked one email
address. As there is cross-commits/mirrors, we should allow any of the
alias emails for a registered bot.
Signed-off-by: Martin Lowe's avatarMartin Lowe <martin.lowe@eclipse-foundation.org>
parent 893aaa0e
......@@ -16,7 +16,8 @@ import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.eclipsefoundation.git.eca.model.BotUser;
import com.fasterxml.jackson.databind.JsonNode;
/**
* Interface for interacting with the Eclipse Foundation Bots API.
......@@ -35,5 +36,5 @@ public interface BotsAPI {
*/
@GET
@Produces("application/json")
List<BotUser> getBots();
List<JsonNode> getBots();
}
/**
* Copyright (C) 2020 Eclipse Foundation
*
* <p>This program and the accompanying materials are made available under the terms of the Eclipse
* Public License 2.0 which is available at https://www.eclipse.org/legal/epl-2.0/
*
* <p>SPDX-License-Identifier: EPL-2.0
*/
package org.eclipsefoundation.git.eca.model;
import com.fasterxml.jackson.annotation.JsonProperty;
/**
* Represents a bot user in the Eclipse API.
*
* @author Martin Lowe
*/
public class BotUser {
private String id;
private String projectId;
private String username;
private String email;
@JsonProperty("github.com")
private SiteSpecificBot githubBot;
@JsonProperty("gitlab.eclipse.org")
private SiteSpecificBot gitlabBot;
/** @return the id */
public String getId() {
return id;
}
/** @param id the id to set */
public void setId(String id) {
this.id = id;
}
/** @return the projectId */
public String getProjectId() {
return projectId;
}
/** @param projectId the projectId to set */
public void setProjectId(String projectId) {
this.projectId = projectId;
}
/** @return the username */
public String getUsername() {
return username;
}
/** @param username the username to set */
public void setUsername(String username) {
this.username = username;
}
/** @return the email */
public String getEmail() {
return email;
}
/** @return the githubBot */
public SiteSpecificBot getGithubBot() {
return githubBot;
}
/** @param githubBot the githubBot to set */
public void setGithubBot(SiteSpecificBot githubBot) {
this.githubBot = githubBot;
}
/** @return the gitlabBot */
public SiteSpecificBot getGitlabBot() {
return gitlabBot;
}
/** @param gitlabBot the gitlabBot to set */
public void setGitlabBot(SiteSpecificBot gitlabBot) {
this.gitlabBot = gitlabBot;
}
/** @param email the email to set */
public void setEmail(String email) {
this.email = email;
}
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("BotUser [id=");
builder.append(id);
builder.append(", projectId=");
builder.append(projectId);
builder.append(", username=");
builder.append(username);
builder.append(", email=");
builder.append(email);
builder.append(", githubBot=");
builder.append(githubBot);
builder.append(", gitlabBot=");
builder.append(gitlabBot);
builder.append("]");
return builder.toString();
}
public static class SiteSpecificBot {
private String username;
private String email;
/** @return the username */
public String getUsername() {
return username;
}
/** @param username the username to set */
public void setUsername(String username) {
this.username = username;
}
/** @return the email */
public String getEmail() {
return email;
}
/** @param email the email to set */
public void setEmail(String email) {
this.email = email;
}
}
}
......@@ -10,7 +10,9 @@ package org.eclipsefoundation.git.eca.resource;
import java.net.MalformedURLException;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.stream.Collectors;
......@@ -27,7 +29,6 @@ import org.eclipse.microprofile.rest.client.inject.RestClient;
import org.eclipsefoundation.git.eca.api.AccountsAPI;
import org.eclipsefoundation.git.eca.api.BotsAPI;
import org.eclipsefoundation.git.eca.helper.CommitHelper;
import org.eclipsefoundation.git.eca.model.BotUser;
import org.eclipsefoundation.git.eca.model.Commit;
import org.eclipsefoundation.git.eca.model.EclipseUser;
import org.eclipsefoundation.git.eca.model.GitUser;
......@@ -42,407 +43,395 @@ import org.eclipsefoundation.git.eca.service.ProjectsService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.fasterxml.jackson.databind.JsonNode;
/**
* ECA validation endpoint for Git commits. Will use information from the bots, projects, and
* accounts API to validate commits passed to this endpoint. Should be as system agnostic as
* possible to allow for any service to request validation with less reliance on services external
* to the Eclipse foundation.
* ECA validation endpoint for Git commits. Will use information from the bots, projects, and accounts API to validate
* commits passed to this endpoint. Should be as system agnostic as possible to allow for any service to request
* validation with less reliance on services external to the Eclipse foundation.
*
* @author Martin Lowe
*/
@Path("/eca")
@Consumes({MediaType.APPLICATION_JSON})
@Produces({MediaType.APPLICATION_JSON})
@Consumes({ MediaType.APPLICATION_JSON })
@Produces({ MediaType.APPLICATION_JSON })
public class ValidationResource {
private static final Logger LOGGER = LoggerFactory.getLogger(ValidationResource.class);
private static final Logger LOGGER = LoggerFactory.getLogger(ValidationResource.class);
// eclipse API rest client interfaces
@Inject @RestClient AccountsAPI accounts;
@Inject @RestClient BotsAPI bots;
// eclipse API rest client interfaces
@Inject
@RestClient
AccountsAPI accounts;
@Inject
@RestClient
BotsAPI bots;
// external API/service harnesses
@Inject OAuthService oauth;
@Inject CachingService cache;
@Inject ProjectsService projects;
// external API/service harnesses
@Inject
OAuthService oauth;
@Inject
CachingService cache;
@Inject
ProjectsService projects;
/**
* Consuming a JSON request, this method will validate all passed commits, using the repo URL and
* the repository provider. These commits will be validated to ensure that all users are covered
* either by an ECA, or are committers on the project. In the case of ECA-only contributors, an
* additional sign off footer is required in the body of the commit.
*
* @param req the request containing basic data plus the commits to be validated
* @return a web response indicating success or failure for each commit, along with standard
* messages that may be used to give users context on failure.
* @throws MalformedURLException
*/
@POST
public Response validate(ValidationRequest req) {
ValidationResponse r = new ValidationResponse();
r.setStrictMode(req.isStrictMode());
// check that we have commits to validate
if (req.getCommits() == null || req.getCommits().isEmpty()) {
addError(r, "A commit is required to validate", null);
}
// check that we have a repo set
if (req.getRepoUrl() == null) {
addError(r, "A base repo URL needs to be set in order to validate", null);
}
// check that we have a type set
if (req.getProvider() == null) {
addError(r, "A provider needs to be set to validate a request", null);
}
// only process if we have no errors
if (r.getErrorCount() == 0) {
LOGGER.debug("Processing: {}", req);
// filter the projects based on the repo URL. At least one repo in project must
// match the repo URL to be valid
List<Project> filteredProjects = retrieveProjectsForRequest(req);
// set whether this call has tracked projects
r.setTrackedProject(!filteredProjects.isEmpty());
for (Commit c : req.getCommits()) {
// process the request, capturing if we should continue processing
boolean continueProcessing = processCommit(c, r, filteredProjects, req.getProvider());
// if there is a reason to stop processing, break the loop
if (!continueProcessing) {
break;
/**
* Consuming a JSON request, this method will validate all passed commits, using the repo URL and the repository
* provider. These commits will be validated to ensure that all users are covered either by an ECA, or are
* committers on the project. In the case of ECA-only contributors, an additional sign off footer is required in the
* body of the commit.
*
* @param req the request containing basic data plus the commits to be validated
* @return a web response indicating success or failure for each commit, along with standard messages that may be
* used to give users context on failure.
* @throws MalformedURLException
*/
@POST
public Response validate(ValidationRequest req) {
ValidationResponse r = new ValidationResponse();
r.setStrictMode(req.isStrictMode());
// check that we have commits to validate
if (req.getCommits() == null || req.getCommits().isEmpty()) {
addError(r, "A commit is required to validate", null);
}
}
}
// depending on number of errors found, set response status
if (r.getErrorCount() == 0) {
r.setPassed(true);
// check that we have a repo set
if (req.getRepoUrl() == null) {
addError(r, "A base repo URL needs to be set in order to validate", null);
}
// check that we have a type set
if (req.getProvider() == null) {
addError(r, "A provider needs to be set to validate a request", null);
}
// only process if we have no errors
if (r.getErrorCount() == 0) {
LOGGER.debug("Processing: {}", req);
// filter the projects based on the repo URL. At least one repo in project must
// match the repo URL to be valid
List<Project> filteredProjects = retrieveProjectsForRequest(req);
// set whether this call has tracked projects
r.setTrackedProject(!filteredProjects.isEmpty());
for (Commit c : req.getCommits()) {
// process the request, capturing if we should continue processing
boolean continueProcessing = processCommit(c, r, filteredProjects);
// if there is a reason to stop processing, break the loop
if (!continueProcessing) {
break;
}
}
}
// depending on number of errors found, set response status
if (r.getErrorCount() == 0) {
r.setPassed(true);
}
return r.toResponse();
}
return r.toResponse();
}
/**
* Process the current request, validating that the passed commit is valid. The author and
* committers Eclipse Account is retrieved, which are then used to check if the current commit is
* valid for the current project.
*
* @param c the commit to process
* @param response the response container
* @param filteredProjects tracked projects for the current request
* @return true if we should continue processing, false otherwise.
*/
private boolean processCommit(
Commit c,
ValidationResponse response,
List<Project> filteredProjects,
ProviderType provider) {
// ensure the commit is valid, and has required fields
if (!CommitHelper.validateCommit(c)) {
addError(
response,
"One or more commits were invalid. Please check the payload and try again",
c.getHash());
return false;
}
// retrieve the author + committer for the current request
GitUser author = c.getAuthor();
GitUser committer = c.getCommitter();
/**
* Process the current request, validating that the passed commit is valid. The author and committers Eclipse
* Account is retrieved, which are then used to check if the current commit is valid for the current project.
*
* @param c the commit to process
* @param response the response container
* @param filteredProjects tracked projects for the current request
* @return true if we should continue processing, false otherwise.
*/
private boolean processCommit(Commit c, ValidationResponse response, List<Project> filteredProjects) {
// ensure the commit is valid, and has required fields
if (!CommitHelper.validateCommit(c)) {
addError(response, "One or more commits were invalid. Please check the payload and try again", c.getHash());
return false;
}
// retrieve the author + committer for the current request
GitUser author = c.getAuthor();
GitUser committer = c.getCommitter();
addMessage(response, String.format("Reviewing commit: %1$s", c.getHash()), c.getHash());
addMessage(
response,
String.format("Authored by: %1$s <%2$s>", author.getName(), author.getMail()),
c.getHash());
addMessage(response, String.format("Reviewing commit: %1$s", c.getHash()), c.getHash());
addMessage(response, String.format("Authored by: %1$s <%2$s>", author.getName(), author.getMail()),
c.getHash());
// skip processing if a merge commit
if (c.getParents().size() > 1) {
addMessage(
response,
String.format(
"Commit '%1$s' has multiple parents, merge commit detected, passing", c.getHash()),
c.getHash());
return true;
}
// skip processing if a merge commit
if (c.getParents().size() > 1) {
addMessage(response,
String.format("Commit '%1$s' has multiple parents, merge commit detected, passing", c.getHash()),
c.getHash());
return true;
}
// retrieve the eclipse account for the author
EclipseUser eclipseAuthor = getIdentifiedUser(author);
if (eclipseAuthor == null) {
// if the user is a bot, generate a stubbed user
if (!userIsABot(author.getMail(), provider)) {
addMessage(
response,
String.format(
"Could not find an Eclipse user with mail '%1$s' for author of commit %2$s",
committer.getMail(), c.getHash()),
c.getHash());
addError(response, "Author must have an Eclipse Account", c.getHash());
return true;
}
// set the Eclipse author as the basic committer w\ bot flag to pass information forward
eclipseAuthor = EclipseUser.createBotStub(author);
}
// retrieve the eclipse account for the author
EclipseUser eclipseAuthor = getIdentifiedUser(author);
if (eclipseAuthor == null) {
// if the user is a bot, generate a stubbed user
if (!userIsABot(author.getMail(), filteredProjects)) {
addMessage(response,
String.format("Could not find an Eclipse user with mail '%1$s' for author of commit %2$s",
committer.getMail(), c.getHash()),
c.getHash());
addError(response, "Author must have an Eclipse Account", c.getHash());
return true;
}
// set the Eclipse author as the basic committer w\ bot flag to pass information forward
eclipseAuthor = EclipseUser.createBotStub(author);
}
// retrieve the eclipse account for the committer
EclipseUser eclipseCommitter = getIdentifiedUser(committer);
if (eclipseCommitter == null) {
if (!userIsABot(committer.getMail(), provider)) {
addMessage(
response,
String.format(
"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());
return true;
}
// set the Eclipse committer as the basic committer w\ bot flag to pass information forward
eclipseCommitter = EclipseUser.createBotStub(committer);
}
// validate author access to the current repo
validateAuthorAccess(response, c, eclipseAuthor, filteredProjects, provider);
// retrieve the eclipse account for the committer
EclipseUser eclipseCommitter = getIdentifiedUser(committer);
if (eclipseCommitter == null) {
if (!userIsABot(committer.getMail(), filteredProjects)) {
addMessage(response,
String.format("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());
return true;
}
// set the Eclipse committer as the basic committer w\ bot flag to pass information forward
eclipseCommitter = EclipseUser.createBotStub(committer);
}
// validate author access to the current repo
validateAuthorAccess(response, c, eclipseAuthor, filteredProjects);
// only committers can push on behalf of other users
if (response.isTrackedProject()
&& !eclipseAuthor.equals(eclipseCommitter)
&& !isCommitter(response, eclipseCommitter, c.getHash(), filteredProjects, provider)) {
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());
// only committers can push on behalf of other users
if (response.isTrackedProject() && !eclipseAuthor.equals(eclipseCommitter)
&& !isCommitter(response, eclipseCommitter, c.getHash(), filteredProjects)) {
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());
}
return true;
}
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 filteredProjects tracked projects for the current request
* @param provider the provider set for the current request
*/
private void validateAuthorAccess(
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());
} else {
addMessage(r, "The author is not a committer on the project.", c.getHash());
// check if the author is signed off if not a committer
if (eclipseAuthor.getEca().isSigned()) {
addMessage(
r,
"The author has a current Eclipse Contributor Agreement (ECA) on file.",
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.",
c.getHash());
addError(r, "An Eclipse Contributor Agreement is required.", c.getHash());
}
/**
* 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 filteredProjects tracked projects for the current request
* @param provider the provider set for the current request
*/
private void validateAuthorAccess(ValidationResponse r, Commit c, EclipseUser eclipseAuthor,
List<Project> filteredProjects) {
// check if the author matches to an eclipse user and is a committer
if (isCommitter(r, eclipseAuthor, c.getHash(), filteredProjects)) {
addMessage(r, "The author is a committer on the project.", c.getHash());
} else {
addMessage(r, "The author is not a committer on the project.", c.getHash());
// check if the author is signed off if not a committer
if (eclipseAuthor.getEca().isSigned()) {
addMessage(r, "The author has a current Eclipse Contributor Agreement (ECA) on file.", 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.",
c.getHash());
addError(r, "An Eclipse Contributor Agreement is required.", c.getHash());
}
// check if one of the signed off by footer lines matches the author email.
if (CommitHelper.getSignedOffByEmail(c)) {
addMessage(r, "The author has signed-off on the contribution.", c.getHash());
} else {
addMessage(
r,
"The author has not signed-off on the contribution.\n"
+ "If there are multiple commits, please ensure that each commit is signed-off.",
c.getHash());
addError(
r,
"The contributor must sign-off on the contribution.",
c.getHash(),
APIStatusCode.ERROR_SIGN_OFF);
}
// check if one of the signed off by footer lines matches the author email.
if (CommitHelper.getSignedOffByEmail(c)) {
addMessage(r, "The author has signed-off on the contribution.", c.getHash());
} else {
addMessage(r,
"The author has not signed-off on the contribution.\n"
+ "If there are multiple commits, please ensure that each commit is signed-off.",
c.getHash());
addError(r, "The contributor must sign-off on the contribution.", c.getHash(),
APIStatusCode.ERROR_SIGN_OFF);
}
}
}
}
/**
* Checks whether the given user is a committer on the project. If they are and the project is
* also a specification for a working group, an additional access check is made against the user.
*
* <p>Additionally, a check is made to see if the user is a registered bot user for the given
* project. If they match for the given project, they are granted committer-like access to the
* repository.
*
* @param r the current response object for the request
* @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.