Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Commit

Permalink
Allow all service account to bump version (#43)
Browse files Browse the repository at this point in the history 
* Allow all service account to bump version

* Add test case for commits where file changed is null

* Increment version to 3.3.3-Snapshot
  • Loading branch information
@chzhanpeng
chzhanpeng authored Jun 29, 2020
1 parent c2d6d4d commit c7931cc
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 116 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<artifactId>api-audit</artifactId>
<packaging>jar</packaging>
<name>${project.groupId}:${project.artifactId}</name>
<version>3.3.2-SNAPSHOT</version>
<version>3.3.3-SNAPSHOT</version>
<description>Hygieia Audit Rest API Layer</description>
<url>https://github.com/Hygieia/${repository.name}</url>

Expand Down
9 changes: 9 additions & 0 deletions src/main/java/com/capitalone/dashboard/ApiSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class ApiSettings {
@Value("${criticalLicenseVulnerabilitiesAge:0}")
private int criticalLicenseVulnerabilitiesAge;
private List<String> buildStageRegEx;
private List<String> directCommitWhitelistedFiles;

public String getKey() {
return key;
Expand Down Expand Up @@ -223,5 +224,13 @@ public void setBuildStageRegEx(List<String> buildStageRegEx) {
this.buildStageRegEx = buildStageRegEx;
}

public List<String> getDirectCommitWhitelistedFiles() {
return directCommitWhitelistedFiles;
}

public void setDirectCommitWhitelistedFiles(List<String> directCommitWhitelistedFiles) {
this.directCommitWhitelistedFiles = directCommitWhitelistedFiles;
}


}
37 changes: 16 additions & 21 deletions src/main/java/com/capitalone/dashboard/common/CommonCodeReview.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import javax.naming.InvalidNameException;
import javax.naming.ldap.LdapName;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
Expand All @@ -41,6 +42,7 @@
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class CommonCodeReview {

Expand Down Expand Up @@ -73,7 +75,7 @@ public static boolean computePeerReviewStatus(GitRequest pr, ApiSettings setting
if (!CollectionUtils.isEmpty(reviews)) {
for (Review review : reviews) {
if (StringUtils.equalsIgnoreCase("approved", review.getState())) {
if (!StringUtils.isEmpty(review.getAuthorLDAPDN()) && checkForServiceAccount(review.getAuthorLDAPDN(), settings,accounts,review.getAuthor(),null,false,auditReviewResponse)) {
if (!StringUtils.isEmpty(review.getAuthorLDAPDN()) && isServiceAccount(review.getAuthorLDAPDN(), settings)) {
auditReviewResponse.addAuditStatus(CodeReviewAuditStatus.PEER_REVIEW_BY_SERVICEACCOUNT);
}
//review done using GitHub Review workflow
Expand Down Expand Up @@ -121,7 +123,7 @@ public static boolean computePeerReviewStatus(GitRequest pr, ApiSettings setting
if (!CollectionUtils.isEmpty(settings.getServiceAccountOU()) && !StringUtils.isEmpty(settings.getPeerReviewApprovalText()) && !StringUtils.isEmpty(description) &&
description.startsWith(settings.getPeerReviewApprovalText())) {
String user = description.replace(settings.getPeerReviewApprovalText(), "").trim();
if (!StringUtils.isEmpty(actors.get(user)) && checkForServiceAccount(actors.get(user), settings,accounts,user,null,false,auditReviewResponse)) {
if (!StringUtils.isEmpty(actors.get(user)) && isServiceAccount(actors.get(user), settings)) {
auditReviewResponse.addAuditStatus(CodeReviewAuditStatus.PEER_REVIEW_BY_SERVICEACCOUNT);
}
}
Expand Down Expand Up @@ -157,16 +159,10 @@ public static boolean computePeerReviewStatus(GitRequest pr, ApiSettings setting
* @param settings
* @return
*/
public static boolean checkForServiceAccount(String userLdapDN, ApiSettings settings,Map<String,String> allowedUsers,String author,List<String> commitFiles,boolean isCommit,AuditReviewResponse auditReviewResponse) {
public static boolean isServiceAccount(String userLdapDN, ApiSettings settings) {
List<String> serviceAccountOU = settings.getServiceAccountOU();
boolean isValid = false;
if(!MapUtils.isEmpty(allowedUsers) && isCommit){
isValid = isValidServiceAccount(author,allowedUsers,commitFiles);
if(isValid){
auditReviewResponse.addAuditStatus(CodeReviewAuditStatus.DIRECT_COMMIT_CHANGE_WHITELISTED_ACCOUNT);
}
}
if (!CollectionUtils.isEmpty(serviceAccountOU) && StringUtils.isNotBlank(userLdapDN) && !isValid) {
if (!CollectionUtils.isEmpty(serviceAccountOU) && StringUtils.isNotBlank(userLdapDN)) {
try {
String userLdapDNParsed = LdapUtils.getStringValue(new LdapName(userLdapDN), "OU");
List<String> matches = serviceAccountOU.stream().filter(it -> it.contains(userLdapDNParsed)).collect(Collectors.toList());
Expand All @@ -181,19 +177,18 @@ public static boolean checkForServiceAccount(String userLdapDN, ApiSettings sett
return isValid;
}

private static boolean isValidServiceAccount(String author, Map<String,String> allowedServiceAccounts,List<String> commitFiles) {

boolean isValidServiceAccount = false;
if (MapUtils.isEmpty(allowedServiceAccounts)) return Boolean.FALSE;
for (String serviceAccount:allowedServiceAccounts.keySet()) {
String fileNames = allowedServiceAccounts.get(serviceAccount);
for (String s : fileNames.split(",")) {
if (serviceAccount.equalsIgnoreCase(author) && findFileMatch(s, commitFiles)){
isValidServiceAccount = true;
}
public static boolean isFileTypeWhitelisted(Commit commit, ApiSettings settings) {
List<String> whitelistedFiles = settings.getDirectCommitWhitelistedFiles();
Stream<String> combinedStream
= Stream.of(commit.getFilesAdded(), commit.getFilesModified(),commit.getFilesRemoved()).filter(Objects::nonNull).flatMap(Collection::stream);
Collection<String> updatedFiles = combinedStream.collect(Collectors.toList());
boolean isValid = true;
for (String file : updatedFiles) {
if(!findFileMatch(file, whitelistedFiles)) {
isValid = false;
}
}
return isValidServiceAccount;
return isValid;
}

private static boolean findFileMatch(String fileName, List<String> files){
Expand Down
16 changes: 7 additions & 9 deletions src/main/java/com/capitalone/dashboard/evaluator/CodeReviewEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,16 +433,14 @@ protected boolean checkIfCommitsMatch(Commit commit1, Commit commit2) {
}

protected void auditDirectCommits(CodeReviewAuditResponseV2 reviewAuditResponseV2, Commit commit) {
Stream<String> combinedStream
= Stream.of(commit.getFilesAdded(), commit.getFilesModified(),commit.getFilesRemoved()).filter(Objects::nonNull).flatMap(Collection::stream);
Collection<String> collectionCombined = combinedStream.collect(Collectors.toList());
if (CommonCodeReview.checkForServiceAccount(commit.getScmAuthorLDAPDN(), settings,getAllServiceAccounts(),commit.getScmAuthor(),collectionCombined.stream().collect(Collectors.toList()),true,reviewAuditResponseV2)) {
if (CommonCodeReview.isServiceAccount(commit.getScmAuthorLDAPDN(), settings) &&
CommonCodeReview.isFileTypeWhitelisted(commit, settings) &&
CommonCodeReview.matchIncrementVersionTag(commit.getScmCommitLog(), settings)
) {
reviewAuditResponseV2.addAuditStatus(CodeReviewAuditStatus.COMMITAUTHOR_EQ_SERVICEACCOUNT);
auditIncrementVersionTag(reviewAuditResponseV2, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_SERVICE_ACCOUNT);
} else if (StringUtils.isBlank(commit.getScmAuthorLDAPDN())) {
auditIncrementVersionTag(reviewAuditResponseV2, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE);
}else {
auditIncrementVersionTag(reviewAuditResponseV2, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_USER_ACCOUNT);
reviewAuditResponseV2.addAuditStatus(CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_SERVICE_ACCOUNT);
} else {
addDirectCommitsToBase(reviewAuditResponseV2,commit);
}
}

Expand Down
28 changes: 8 additions & 20 deletions src/main/java/com/capitalone/dashboard/evaluator/CodeReviewEvaluatorLegacy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ protected void auditPullRequest(CollectorItem repoItem, GitRequest pr, List<Comm
String sourceRepo = pr.getSourceRepo();
String targetRepo = pr.getTargetRepo();
codeReviewAuditResponse.addAuditStatus(sourceRepo == null ? CodeReviewAuditStatus.GIT_FORK_STRATEGY : sourceRepo.equalsIgnoreCase(targetRepo) ? CodeReviewAuditStatus.GIT_BRANCH_STRATEGY : CodeReviewAuditStatus.GIT_FORK_STRATEGY);
if (!StringUtils.isEmpty(pr.getMergeAuthorLDAPDN()) && (CommonCodeReview.checkForServiceAccount(pr.getMergeAuthorLDAPDN(), settings, getAllServiceAccounts(),pr.getMergeAuthor(),null,false,codeReviewAuditResponse))) {
if (!StringUtils.isEmpty(pr.getMergeAuthorLDAPDN()) && (CommonCodeReview.isServiceAccount(pr.getMergeAuthorLDAPDN(), settings))) {
codeReviewAuditResponse.addAuditStatus(CodeReviewAuditStatus.MERGECOMMITER_EQ_SERVICEACCOUNT);
}
allPeerReviews.add(codeReviewAuditResponse);
Expand Down Expand Up @@ -359,27 +359,15 @@ private void auditServiceAccountChecks(CodeReviewAuditResponse codeReviewAuditRe

@SuppressWarnings("Duplicates")
private void auditDirectCommits(CodeReviewAuditResponse codeReviewAuditResponse, Commit commit) {

Stream<String> combinedStream = Stream.of(commit.getFilesAdded(), commit.getFilesModified(),commit.getFilesRemoved()).filter(Objects::nonNull).flatMap(Collection::stream);
List<String> collectionCombined = combinedStream.collect(Collectors.toList());
if (CommonCodeReview.checkForServiceAccount(commit.getScmAuthorLDAPDN(), settings,getAllServiceAccounts(),commit.getScmAuthor(),collectionCombined,true,codeReviewAuditResponse)) {
if (CommonCodeReview.isServiceAccount(commit.getScmAuthorLDAPDN(), settings) &&
CommonCodeReview.isFileTypeWhitelisted(commit, settings) &&
CommonCodeReview.matchIncrementVersionTag(commit.getScmCommitLog(), settings)
) {
codeReviewAuditResponse.addAuditStatus(CodeReviewAuditStatus.COMMITAUTHOR_EQ_SERVICEACCOUNT);
// check for increment version tag and flag Direct commit by Service Account.
auditIncrementVersionTag(codeReviewAuditResponse, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_SERVICE_ACCOUNT);
}else if (StringUtils.isBlank(commit.getScmAuthorLDAPDN())) {
// Status for commits without login information.
auditIncrementVersionTag(codeReviewAuditResponse, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE);
} else {
// check for increment version tag and flag Direct commit by User Account.
auditIncrementVersionTag(codeReviewAuditResponse, commit, CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_USER_ACCOUNT);
}
}

private void auditIncrementVersionTag(CodeReviewAuditResponse codeReviewAuditResponse, Commit commit, CodeReviewAuditStatus directCommitIncrementVersionTagStatus) {
if (CommonCodeReview.matchIncrementVersionTag(commit.getScmCommitLog(), settings)) {
codeReviewAuditResponse.addAuditStatus(directCommitIncrementVersionTagStatus);
} else {
codeReviewAuditResponse.addAuditStatus(CodeReviewAuditStatus.DIRECT_COMMIT_NONCODE_CHANGE_SERVICE_ACCOUNT);
} else {
codeReviewAuditResponse.addAuditStatus(commit.isFirstEverCommit() ? CodeReviewAuditStatus.DIRECT_COMMITS_TO_BASE_FIRST_COMMIT : CodeReviewAuditStatus.DIRECT_COMMITS_TO_BASE);

}
}

Expand Down
83 changes: 48 additions & 35 deletions src/test/java/com/capitalone/dashboard/common/CommonCodeReviewTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,55 +35,44 @@ public class CommonCodeReviewTest {
private CommitRepository commitRepository;
@Mock
private ServiceAccountRepository serviceAccountRepository;
@Test
public void testCheckForServiceAccount() {

apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Assert.assertEquals(true, CommonCodeReview.checkForServiceAccount("CN=hygieiaUser,OU=Service Accounts,DC=basic,DC=ds,DC=industry,DC=com", apiSettings,null,null,null,false,new AuditReviewResponse()));
}

@Test
public void testCheckForServiceAccountForAllUsers() {
public void testIsServiceAccount() {
apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Assert.assertEquals(false, CommonCodeReview.checkForServiceAccount("CN=hygieiaUser,OU=Developers,OU=All Users,DC=basic,DC=ds,DC=industry,DC=com", apiSettings,null,null,null,false,new AuditReviewResponse()));
Assert.assertFalse(CommonCodeReview.isServiceAccount("CN=hygieiaUser,OU=Developers,OU=All Users,DC=basic,DC=ds,DC=industry,DC=com", apiSettings));
Assert.assertTrue(CommonCodeReview.isServiceAccount("CN=hygieiaUser,OU=Service Accounts,DC=basic,DC=ds,DC=industry,DC=com", apiSettings));
}


@Test
public void testCheckForServiceAccountForAllowedServiceAccountsMatch() {
apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Map<String,String> allowedUsers = Collections.unmodifiableMap(Stream.of(
new AbstractMap.SimpleEntry<>("allowedUser1", "pom.xml,test.json"),
new AbstractMap.SimpleEntry<>("allowedUser2", "test.java"))
.collect(Collectors.toMap((e) -> e.getKey(), (e) -> e.getValue())));
Assert.assertEquals(true, CommonCodeReview.checkForServiceAccount("CN=hygieiaUser,OU=Developers,OU=All Users,DC=basic,DC=ds,DC=industry,DC=com", apiSettings,allowedUsers,"allowedUser1",Stream.of("test.json").collect(Collectors.toList()), true,new AuditReviewResponse()));
public void testIsFileTypeWhitelistedPass() {
List<String> whitelistedFile = new ArrayList<>();
whitelistedFile.add("pom.xml");
whitelistedFile.add("readme.md");
apiSettings.setDirectCommitWhitelistedFiles(whitelistedFile);
Commit commit = makeCommitWhitelistedFiles();
Assert.assertTrue(CommonCodeReview.isFileTypeWhitelisted(commit, apiSettings));
}

@Test
public void testCheckForServiceAccountForAllowedServiceAccountsWildcardMatch() {
AuditReviewResponse<CodeReviewAuditStatus> auditStatusAuditReviewResponse = new AuditReviewResponse();
apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Map<String,String> allowedUsers = Collections.unmodifiableMap(Stream.of(
new AbstractMap.SimpleEntry<>("allowedUser1", "pom.xml,test.json"),
new AbstractMap.SimpleEntry<>("allowedUser2", "*.java"))
.collect(Collectors.toMap((e) -> e.getKey(), (e) -> e.getValue())));
Assert.assertEquals(true, CommonCodeReview.checkForServiceAccount("CN=hygieiaUser,OU=Developers,OU=All Users,DC=basic,DC=ds,DC=industry,DC=com", apiSettings,allowedUsers,"allowedUser2",Stream.of("test.java").collect(Collectors.toList()), true,auditStatusAuditReviewResponse));
Assert.assertEquals(true, auditStatusAuditReviewResponse.getAuditStatuses().toString().contains("DIRECT_COMMIT_CHANGE_WHITELISTED_ACCOUNT"));
public void testIsFileTypeWhitelistedNoFilesInCommit() {
List<String> whitelistedFile = new ArrayList<>();
whitelistedFile.add("pom.xml");
whitelistedFile.add("readme.md");
apiSettings.setDirectCommitWhitelistedFiles(whitelistedFile);
Commit commit = makeCommitNoFiles();
Assert.assertTrue(CommonCodeReview.isFileTypeWhitelisted(commit, apiSettings));
}

@Test
public void testCheckForServiceAccountForAllowedServiceAccountsNonMatch() {
AuditReviewResponse<CodeReviewAuditStatus> auditStatusAuditReviewResponse = new AuditReviewResponse();
apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Map<String,String> allowedUsers = Collections.unmodifiableMap(Stream.of(
new AbstractMap.SimpleEntry<>("allowedUser1", "pom.xml,test.json"),
new AbstractMap.SimpleEntry<>("allowedUser2", "*.java"))
.collect(Collectors.toMap((e) -> e.getKey(), (e) -> e.getValue())));
Assert.assertEquals(false, auditStatusAuditReviewResponse.getAuditStatuses().toString().contains("DIRECT_COMMIT_CHANGE_WHITELISTED_ACCOUNT"));
Assert.assertEquals(false, CommonCodeReview.checkForServiceAccount("CN=hygieiaUser,OU=Developers,OU=All Users,DC=basic,DC=ds,DC=industry,DC=com", apiSettings,allowedUsers,"allowedUser2",Stream.of("test.md").collect(Collectors.toList()), true,auditStatusAuditReviewResponse));
public void testIsFileTypeWhitelistedFail() {
List<String> whitelistedFile = new ArrayList<>();
whitelistedFile.add("pom.xml");
whitelistedFile.add("readme.md");
apiSettings.setDirectCommitWhitelistedFiles(whitelistedFile);
Commit commit = makeCommit();
Assert.assertFalse(CommonCodeReview.isFileTypeWhitelisted(commit, apiSettings));
}


@Test
public void testComputePeerReviewStatusForServiceAccount() {
apiSettings.setServiceAccountOU(TestConstants.SERVICE_ACCOUNTS);
Expand Down Expand Up @@ -140,6 +129,30 @@ private Commit makeCommit() {
c.setScmAuthorLDAPDN("CN=hygieiaUser,OU=Service Accounts,DC=basic,DC=ds,DC=industry,DC=com");
c.setScmCommitTimestamp(100000000);
c.setScmAuthorLogin("hygieiaUser");
c.setFilesAdded(Arrays.asList("source1.java", "source2.java"));
return c;
}

private Commit makeCommitNoFiles() {
Commit c = new Commit();
c.setId(ObjectId.get());
c.setScmCommitLog("Merge branch master into branch");
c.setScmAuthor("hygieiaUser");
c.setScmAuthorLDAPDN("CN=hygieiaUser,OU=Service Accounts,DC=basic,DC=ds,DC=industry,DC=com");
c.setScmCommitTimestamp(100000000);
c.setScmAuthorLogin("hygieiaUser");
return c;
}

private Commit makeCommitWhitelistedFiles() {
Commit c = new Commit();
c.setId(ObjectId.get());
c.setScmCommitLog("Merge branch master into branch");
c.setScmAuthor("hygieiaUser");
c.setScmAuthorLDAPDN("CN=hygieiaUser,OU=Service Accounts,DC=basic,DC=ds,DC=industry,DC=com");
c.setScmCommitTimestamp(100000000);
c.setScmAuthorLogin("hygieiaUser");
c.setFilesAdded(Arrays.asList("readme.md", "pom.xml"));
return c;
}

Expand Down
Loading

0 comments on commit c7931cc

Please sign in to comment.