Skip to content

Commit f9ad399

Browse files
maheshpmaheshp
authored
Merge pull request #4185 from varshavaradarajan/stage-approval-validation
Validate the users and roles defined in stage authorization in the context of the pipeline using the template. (#4176)
2 parents e331a06 + 71d1dd4 commit f9ad399

File tree

6 files changed

+80
-12
lines changed

6 files changed

+80
-12
lines changed

‎common/src/test/java/com/thoughtworks/go/domain/ApprovalTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void shouldFailValidateWhenUsersWithoutOperatePermissionOnGroupAreAuthori
8181

8282
AdminUser user = approval.getAuthConfig().getUsers().get(0);
8383
assertThat(user.errors().isEmpty(), is(false));
84-
assertThat(user.errors().on("name"), is("User \"not-present\" who is not authorized to operate pipeline group can not be authorized to approve stage"));
84+
assertThat(user.errors().on("name"), is("User \"not-present\" who is not authorized to operate pipeline group `defaultGroup` can not be authorized to approve stage"));
8585
}
8686

8787
@Test
@@ -196,7 +196,7 @@ public void validate_shouldNotAllow_UserInApprovalListButNotInOperationList() {
196196

197197
AdminUser user = approval.getAuthConfig().getUsers().get(0);
198198
assertThat(user.errors().isEmpty(), is(false));
199-
assertThat(user.errors().on("name"), is("User \"not-present\" who is not authorized to operate pipeline group can not be authorized to approve stage"));
199+
assertThat(user.errors().on("name"), is("User \"not-present\" who is not authorized to operate pipeline group `defaultGroup` can not be authorized to approve stage"));
200200
}
201201

202202
@Test
@@ -215,7 +215,7 @@ public void validate_shouldNotAllowRoleInApprovalListButNotInOperationList() thr
215215

216216
AdminRole user = approval.getAuthConfig().getRoles().get(0);
217217
assertThat(user.errors().isEmpty(), is(false));
218-
assertThat(user.errors().on("name"), is("Role \"not-present\" who is not authorized to operate pipeline group can not be authorized to approve stage"));
218+
assertThat(user.errors().on("name"), is("Role \"not-present\" who is not authorized to operate pipeline group `defaultGroup` can not be authorized to approve stage"));
219219
}
220220

221221
@Test

‎config/config-api/src/main/java/com/thoughtworks/go/config/AdminRole.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,12 @@ public void validate(ValidationContext validationContext) {
9393
}
9494
SecurityConfig securityConfig = validationContext.getServerSecurityConfig();
9595
if (!securityConfig.isRoleExist(this.name)) {
96-
configErrors.add(NAME, String.format("Role \"%s\" does not exist.", this.name));
96+
if(validationContext.isWithinPipelines()) {
97+
configErrors.add(NAME, String.format("Role \"%s\" defined for `%s` does not exist.", this.name, validationContext.getPipeline().getName()));
98+
}
99+
else {
100+
configErrors.add(NAME, String.format("Role \"%s\" does not exist.", this.name));
101+
}
97102
}
98103
}
99104

‎config/config-api/src/main/java/com/thoughtworks/go/config/Approval.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ public void setType(String type) {
113113
public boolean validateTree(ValidationContext validationContext) {
114114
validate(validationContext);
115115
boolean isValid = errors.isEmpty();
116+
isValid = validateAuthConfig(validationContext, isValid);
117+
return isValid;
118+
}
119+
120+
private boolean validateAuthConfig(ValidationContext validationContext, boolean isValid) {
116121
for (Admin admin : authConfig) {
117122
admin.validate(validationContext);
118123
authConfig.errors().addAll(admin.errors());
@@ -125,6 +130,10 @@ public void validate(ValidationContext validationContext) {
125130
if (!isValidTypeValue()) {
126131
errors.add(TYPE, String.format("You have defined approval type as '%s'. Approval can only be of the type '%s' or '%s'.", type, MANUAL, SUCCESS));
127132
}
133+
validateOperatePermissions(validationContext);
134+
}
135+
136+
private void validateOperatePermissions(ValidationContext validationContext) {
128137
if (validationContext.isWithinPipelines()) {
129138
PipelineConfigs group = validationContext.getPipelineGroup();
130139
if (!group.hasOperationPermissionDefined()) {
@@ -143,7 +152,7 @@ public void validate(ValidationContext validationContext) {
143152
boolean approverIsNotAGroupOperator = !groupOperators.has(approver, roles.memberRoles(approver));
144153

145154
if (approverIsNotAnAdmin && approverIsNotAGroupOperator) {
146-
approver.addError(String.format("%s \"%s\" who is not authorized to operate pipeline group can not be authorized to approve stage", approver.describe(), approver, group.getGroup()));
155+
approver.addError(String.format("%s \"%s\" who is not authorized to operate pipeline group `%s` can not be authorized to approve stage", approver.describe(), approver, group.getGroup()));
147156
}
148157
}
149158
}
@@ -164,6 +173,10 @@ public ConfigErrors errors() {
164173
return errors;
165174
}
166175

176+
public List<ConfigErrors> getAllErrors() {
177+
return ErrorCollector.getAllErrors(this);
178+
}
179+
167180
public void addError(String fieldName, String message) {
168181
errors.add(fieldName, message);
169182
}

‎config/config-api/src/main/java/com/thoughtworks/go/config/PipelineTemplateConfig.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,10 @@ private void validateDependencies(CruiseConfig preprocessedConfig) {
8585
ParamsConfig paramsConfig = this.referredParams();
8686
for (CaseInsensitiveString pipelineName : pipelineNames) {
8787
PipelineConfig pipelineConfig = preprocessedConfig.getPipelineConfigByName(pipelineName);
88-
PipelineConfigSaveValidationContext contextForStages = PipelineConfigSaveValidationContext.forChain(false, "", preprocessedConfig, pipelineConfig);
88+
PipelineConfigs pipelineGroup = preprocessedConfig.findGroupOfPipeline(pipelineConfig);
89+
PipelineConfigSaveValidationContext contextForStages = PipelineConfigSaveValidationContext.forChain(false, pipelineGroup.getGroup(), preprocessedConfig, pipelineConfig);
8990
validateParams(pipelineConfig, paramsConfig);
90-
validateFetchTasksAndElasticProfileId(pipelineConfig, contextForStages);
91+
validatePartsOfPipelineConfig(pipelineConfig, contextForStages);
9192
validateDependenciesOfDownstreams(pipelineConfig, contextForStages);
9293
}
9394
}
@@ -98,17 +99,27 @@ private void validateDependenciesOfDownstreams(PipelineConfig pipelineConfig, Pi
9899
this.errors().addAll(pipelineConfig.errors());
99100
}
100101

101-
private void validateFetchTasksAndElasticProfileId(PipelineConfig pipelineConfig, PipelineConfigSaveValidationContext contextForStages) {
102+
private void validatePartsOfPipelineConfig(PipelineConfig pipelineConfig, PipelineConfigSaveValidationContext contextForStages) {
102103
for (StageConfig stageConfig : pipelineConfig.getStages()) {
103-
PipelineConfigSaveValidationContext contextForJobs = contextForStages.withParent(stageConfig);
104+
PipelineConfigSaveValidationContext contextForChildren = contextForStages.withParent(stageConfig);
105+
validateStageApprovalAuthorization(stageConfig, contextForChildren);
104106
for (JobConfig jobConfig : stageConfig.getJobs()) {
105-
PipelineConfigSaveValidationContext contextForTasks = contextForJobs.withParent(jobConfig);
107+
PipelineConfigSaveValidationContext contextForTasks = contextForChildren.withParent(jobConfig);
106108
validateFetchTasks(jobConfig, contextForTasks);
107109
validateElasticProfileId(jobConfig, contextForTasks);
108110
}
109111
}
110112
}
111113

114+
private void validateStageApprovalAuthorization(StageConfig stageConfig, PipelineConfigSaveValidationContext contextForChildren) {
115+
Approval approval = stageConfig.getApproval();
116+
if (!approval.validateTree(contextForChildren)) {
117+
for (ConfigErrors errors : approval.getAllErrors()) {
118+
this.errors().addAll(errors);
119+
}
120+
}
121+
}
122+
112123
private void validateElasticProfileId(JobConfig jobConfig, PipelineConfigSaveValidationContext preprocessedConfig) {
113124
String elasticProfileId = jobConfig.getElasticProfileId();
114125
if (elasticProfileId != null && !preprocessedConfig.isValidProfileId(elasticProfileId)) {

‎config/config-api/src/test/java/com/thoughtworks/go/config/AdminRoleTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,16 @@ public void shouldThrowExceptionIfRoleNameInPipelinesAuthorizationDoesNotExist_C
5454
}
5555

5656
@Test
57-
public void shouldAddValidationErrorIfRoleNameInPipelinesAuthorizationDoesNotExist_PipelineConfigSaveValidationContext() {
57+
public void shouldAddValidationErrorWithPipelineNameIfRoleNameInPipelinesAuthorizationDoesNotExist_PipelineConfigSaveValidationContext() {
5858
AdminRole role = new AdminRole(new CaseInsensitiveString("role2"));
5959
PipelineConfig pipelineConfig = new PipelineConfig();
60+
pipelineConfig.setName("foo");
6061
PipelineConfigs pipelinesConfig = new BasicPipelineConfigs(new Authorization(new ViewConfig(role)), pipelineConfig);
6162
CruiseConfig config = new BasicCruiseConfig(pipelinesConfig);
6263
role.validate(PipelineConfigSaveValidationContext.forChain(true, "group",config, pipelineConfig));
6364
ConfigErrors errors = role.errors();
6465
assertThat(errors.isEmpty(), is(false));
65-
assertThat(errors.on(AdminRole.NAME), is("Role \"role2\" does not exist."));
66+
assertThat(errors.on(AdminRole.NAME), is("Role \"role2\" defined for `foo` does not exist."));
6667
}
6768

6869
@Test

‎config/config-api/src/test/java/com/thoughtworks/go/config/PipelineTemplateConfigTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,44 @@ public void shouldValidateFetchTasksOfATemplateInTheContextOfPipelinesUsingTheTe
214214
assertThat(template.errors().getAllOn("pipeline"), is(Arrays.asList("\"pipeline :: stage :: defaultJob\" tries to fetch artifact from pipeline \"non-existent-pipeline\" which does not exist.")));
215215
}
216216

217+
@Test
218+
public void shouldValidateStageApprovalAuthorizationOfATemplateInTheContextOfPipelinesUsingTheTemplate() throws Exception {
219+
JobConfig jobConfig = new JobConfig(new CaseInsensitiveString("defaultJob"));
220+
JobConfigs jobConfigs = new JobConfigs(jobConfig);
221+
StageConfig stageConfig = StageConfigMother.custom("stage", jobConfigs);
222+
stageConfig.setApproval(new Approval(new AuthConfig(new AdminRole(new CaseInsensitiveString("non-existent-role")))));
223+
PipelineTemplateConfig template = PipelineTemplateConfigMother.createTemplate("template", stageConfig);
224+
PipelineConfig pipelineConfig = PipelineConfigMother.pipelineConfigWithTemplate("pipeline", "template");
225+
pipelineConfig.usingTemplate(template);
226+
BasicCruiseConfig cruiseConfig = GoConfigMother.defaultCruiseConfig();
227+
cruiseConfig.addTemplate(template);
228+
cruiseConfig.addPipelineWithoutValidation("group", pipelineConfig);
229+
230+
template.validateTree(ConfigSaveValidationContext.forChain(cruiseConfig), cruiseConfig, false);
231+
232+
assertThat(template.errors().getAllOn("name"), is(Arrays.asList("Role \"non-existent-role\" defined for `pipeline` does not exist.")));
233+
}
234+
235+
@Test
236+
public void shouldValidateStagePermissionsOfATemplateStageInTheContextOfPipelineUsingTheTemplate() {
237+
StageConfig stageConfig = StageConfigMother.custom("stage", new JobConfigs(new JobConfig(new CaseInsensitiveString("defaultJob"))));
238+
stageConfig.setApproval(new Approval(new AuthConfig(new AdminUser(new CaseInsensitiveString("non-admin-non-operate")))));
239+
PipelineTemplateConfig template = PipelineTemplateConfigMother.createTemplate("template", stageConfig);
240+
PipelineConfig pipelineConfig = PipelineConfigMother.pipelineConfigWithTemplate("pipeline", "template");
241+
pipelineConfig.usingTemplate(template);
242+
BasicCruiseConfig cruiseConfig = GoConfigMother.defaultCruiseConfig();
243+
cruiseConfig.addTemplate(template);
244+
cruiseConfig.addPipelineWithoutValidation("group", pipelineConfig);
245+
PipelineConfigs group = cruiseConfig.findGroup("group");
246+
group.setAuthorization(new Authorization(new ViewConfig(), new OperationConfig(new AdminUser(new CaseInsensitiveString("foo"))), new AdminsConfig()));
247+
cruiseConfig.server().security().securityAuthConfigs().add(new SecurityAuthConfig());
248+
cruiseConfig.server().security().adminsConfig().add(new AdminUser(new CaseInsensitiveString("super-admin")));
249+
250+
template.validateTree(ConfigSaveValidationContext.forChain(cruiseConfig), cruiseConfig, false);
251+
252+
assertThat(template.errors().getAllOn("name"), is(Arrays.asList("User \"non-admin-non-operate\" who is not authorized to operate pipeline group `group` can not be authorized to approve stage")));
253+
}
254+
217255
@Test
218256
public void shouldValidateTemplateStageUsedInDownstreamPipelines() {
219257
JobConfig jobConfigWithExecTask = new JobConfig(new CaseInsensitiveString("defaultJob"));

0 commit comments

Comments
 (0)