From c57316d8a37941d4c93e035855343784f9d331db Mon Sep 17 00:00:00 2001 From: Adam Jordens Date: Tue, 8 Dec 2020 11:48:06 -0800 Subject: [PATCH] feat(echo): Support for restricting who can perform a manual judgment This PR verifies the current user roles against `context.selectedStageRoles`. The operation is _only_ allowed if there is at least one overlapping role between `context.selectedStageRoles` and the current user roles. `context.selectedStageRoles` maps to what is currently being specified in the UI. This is a rewrite of what originally went in as https://github.com/spinnaker/orca/pull/3988. --- orca-echo/orca-echo.gradle | 1 + .../echo/pipeline/ManualJudgmentStage.groovy | 68 ++++--- .../util/ManualJudgmentAuthorization.java | 172 ++++++++++++++++++ .../pipeline/ManualJudgmentStageSpec.groovy | 71 +++++++- .../ManualJudgmentAuthorizationSpec.groovy | 57 ++++++ .../orca/web/config/WebConfiguration.groovy | 1 - 6 files changed, 340 insertions(+), 30 deletions(-) create mode 100644 orca-echo/src/main/java/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorization.java create mode 100644 orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorizationSpec.groovy diff --git a/orca-echo/orca-echo.gradle b/orca-echo/orca-echo.gradle index 15e37c6f3d5..0d553b085f1 100644 --- a/orca-echo/orca-echo.gradle +++ b/orca-echo/orca-echo.gradle @@ -27,6 +27,7 @@ dependencies { implementation("org.springframework.boot:spring-boot-autoconfigure") implementation("javax.validation:validation-api") implementation("com.netflix.spinnaker.fiat:fiat-core:$fiatVersion") + implementation("com.netflix.spinnaker.fiat:fiat-api:$fiatVersion") testImplementation("com.squareup.retrofit:retrofit-mock") } diff --git a/orca-echo/src/main/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStage.groovy b/orca-echo/src/main/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStage.groovy index b477f800034..2f406e219c0 100644 --- a/orca-echo/src/main/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStage.groovy +++ b/orca-echo/src/main/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStage.groovy @@ -23,6 +23,7 @@ import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus import com.netflix.spinnaker.orca.api.pipeline.OverridableTimeoutRetryableTask import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution import com.netflix.spinnaker.orca.api.pipeline.TaskResult +import com.netflix.spinnaker.orca.echo.util.ManualJudgmentAuthorization import javax.annotation.Nonnull import java.util.concurrent.TimeUnit @@ -42,7 +43,7 @@ class ManualJudgmentStage implements StageDefinitionBuilder, AuthenticatedStage @Override void taskGraph(@Nonnull StageExecution stage, @Nonnull TaskNode.Builder builder) { builder - .withTask("waitForJudgment", WaitForManualJudgmentTask.class) + .withTask("waitForJudgment", WaitForManualJudgmentTask.class) } @Override @@ -72,8 +73,15 @@ class ManualJudgmentStage implements StageDefinitionBuilder, AuthenticatedStage final long backoffPeriod = 15000 final long timeout = TimeUnit.DAYS.toMillis(3) - @Autowired(required = false) - EchoService echoService + private final EchoService echoService + private final ManualJudgmentAuthorization manualJudgmentAuthorization + + @Autowired + WaitForManualJudgmentTask(EchoService echoService, + ManualJudgmentAuthorization manualJudgmentAuthorization) { + this.echoService = echoService + this.manualJudgmentAuthorization = manualJudgmentAuthorization + } @Override TaskResult execute(StageExecution stage) { @@ -96,6 +104,17 @@ class ManualJudgmentStage implements StageDefinitionBuilder, AuthenticatedStage break } + if (stageData.state != StageData.State.UNKNOWN && !stageData.selectedStageRoles.isEmpty()) { + def application = stage.execution.application + def currentUser = stage.lastModified?.user + + if (!manualJudgmentAuthorization.isAuthorized(application, stageData.selectedStageRoles, currentUser)) { + notificationState = "manualJudgment" + executionStatus = ExecutionStatus.RUNNING + stage.context.put("judgmentStatus", "") + } + } + Map outputs = processNotifications(stage, stageData, notificationState) return TaskResult.builder(executionStatus).context(outputs).build() @@ -128,6 +147,7 @@ class ManualJudgmentStage implements StageDefinitionBuilder, AuthenticatedStage static class StageData { String judgmentStatus = "" List notifications = [] + List selectedStageRoles = [] boolean propagateAuthenticationContext State getState() { @@ -194,27 +214,27 @@ class ManualJudgmentStage implements StageDefinitionBuilder, AuthenticatedStage void notify(EchoService echoService, StageExecution stage, String notificationState) { echoService.create(new EchoService.Notification( - notificationType: EchoService.Notification.Type.valueOf(type.toUpperCase()), - to: address ? [address] : (publisherName ? [publisherName] : null), - cc: cc ? [cc] : null, - templateGroup: notificationState, - severity: EchoService.Notification.Severity.HIGH, - source: new EchoService.Notification.Source( - executionType: stage.execution.type.toString(), - executionId: stage.execution.id, - application: stage.execution.application - ), - additionalContext: [ - stageName: stage.name, - stageId: stage.refId, - restrictExecutionDuringTimeWindow: stage.context.restrictExecutionDuringTimeWindow, - execution: stage.execution, - instructions: stage.context.instructions ?: "", - message: message?.get(notificationState)?.text, - judgmentInputs: stage.context.judgmentInputs, - judgmentInput: stage.context.judgmentInput, - judgedBy: stage.context.lastModifiedBy - ] + notificationType: EchoService.Notification.Type.valueOf(type.toUpperCase()), + to: address ? [address] : (publisherName ? [publisherName] : null), + cc: cc ? [cc] : null, + templateGroup: notificationState, + severity: EchoService.Notification.Severity.HIGH, + source: new EchoService.Notification.Source( + executionType: stage.execution.type.toString(), + executionId: stage.execution.id, + application: stage.execution.application + ), + additionalContext: [ + stageName : stage.name, + stageId : stage.refId, + restrictExecutionDuringTimeWindow: stage.context.restrictExecutionDuringTimeWindow, + execution : stage.execution, + instructions : stage.context.instructions ?: "", + message : message?.get(notificationState)?.text, + judgmentInputs : stage.context.judgmentInputs, + judgmentInput : stage.context.judgmentInput, + judgedBy : stage.context.lastModifiedBy + ] )) lastNotifiedByNotificationState[notificationState] = new Date() } diff --git a/orca-echo/src/main/java/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorization.java b/orca-echo/src/main/java/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorization.java new file mode 100644 index 00000000000..6dd58782c1a --- /dev/null +++ b/orca-echo/src/main/java/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorization.java @@ -0,0 +1,172 @@ +/* + * Copyright 2020 OpsMx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.orca.echo.util; + +import static java.lang.String.format; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Strings; +import com.google.common.collect.Sets; +import com.netflix.spinnaker.fiat.model.UserPermission; +import com.netflix.spinnaker.fiat.model.resources.Role; +import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator; +import com.netflix.spinnaker.fiat.shared.FiatStatus; +import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import com.netflix.spinnaker.orca.front50.Front50Service; +import com.netflix.spinnaker.orca.front50.model.Application; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Component; +import retrofit.RetrofitError; + +@Component +public class ManualJudgmentAuthorization { + private final Logger log = LoggerFactory.getLogger(getClass()); + + private final Front50Service front50Service; + private final FiatPermissionEvaluator fiatPermissionEvaluator; + + private final FiatStatus fiatStatus; + private final ObjectMapper objectMapper; + + @Autowired + public ManualJudgmentAuthorization( + Optional front50Service, + Optional fiatPermissionEvaluator, + FiatStatus fiatStatus, + ObjectMapper objectMapper) { + this.front50Service = front50Service.orElse(null); + this.fiatPermissionEvaluator = fiatPermissionEvaluator.orElse(null); + + this.fiatStatus = fiatStatus; + this.objectMapper = objectMapper; + } + + /** + * A manual judgment will be considered "authorized" if the current user has at least one of the + * required judgment roles _and_ read/write/execute permissions on the application being judged. + * + * @param application Application being judged + * @param requiredJudgmentRoles Required judgment roles + * @param currentUser User that has attempted this judgment + * @return whether or not {@param currentUser} has authorization to judge + */ + public boolean isAuthorized( + String application, Collection requiredJudgmentRoles, String currentUser) { + if (!fiatStatus.isEnabled() || requiredJudgmentRoles.isEmpty()) { + return true; + } + + if (Strings.isNullOrEmpty(currentUser)) { + return false; + } + + UserPermission.View permission = fiatPermissionEvaluator.getPermission(currentUser); + if (permission == null) { // Should never happen? + log.warn("Attempted to get user permission for '{}' but none were found.", currentUser); + return false; + } + + return isAuthorized( + requiredJudgmentRoles, + permission.getRoles().stream().map(Role.View::getName).collect(Collectors.toList()), + getApplicationPermissions(application)); + } + + private boolean isAuthorized( + Collection requiredJudgmentRoles, + Collection currentUserRoles, + Map applicationPermissions) { + if (requiredJudgmentRoles == null || requiredJudgmentRoles.isEmpty()) { + return true; + } + + return Sets.intersection(new HashSet<>(requiredJudgmentRoles), new HashSet<>(currentUserRoles)) + .size() + > 0; + + // boolean isAuthorizedGroup = false; + // + // for (String role : currentUserRoles) { + // if (requiredJudgmentRoles.contains(role)) { + // for (Map.Entry entry : + // applicationPermissions.entrySet()) { // get the application permission roles. + // if (Authorization.CREATE.name().equals(entry.getKey()) + // || Authorization.EXECUTE.name().equals(entry.getKey()) + // || Authorization.WRITE.name().equals(entry.getKey())) { + // // If the application permission roles has 'CREATE, EXECUTE, WRITE', then user is + // // authorized. + // if (entry.getValue() != null && ((List) entry.getValue()).contains(role)) + // { + // return true; + // } + // } else if (Authorization.READ.name().equals(entry.getKey())) { + // // If the application permission roles has 'READ', then user is not authorized. + // if (entry.getValue() != null && ((List) entry.getValue()).contains(role)) + // { + // isAuthorizedGroup = false; + // } + // } + // } + // } + // } + // + // return isAuthorizedGroup; + } + + private Map getApplicationPermissions(String applicationName) { + return getApplication(applicationName) + .map( + application -> { + if (application.getPermission().getPermissions() != null) { + return objectMapper.convertValue( + application.getPermission().getPermissions(), + new TypeReference>() {}); + } + + return new HashMap(); + }) + .orElse(new HashMap<>()); + } + + private Optional getApplication(String applicationName) { + if (front50Service == null) { + return Optional.empty(); + } + + try { + return Optional.of(front50Service.get(applicationName)); + } catch (RetrofitError e) { + if (e.getResponse().getStatus() == HttpStatus.NOT_FOUND.value()) { + return Optional.empty(); + } + throw new SpinnakerException( + format("Failed to retrieve application '%s'", applicationName), e); + } catch (RuntimeException re) { + return Optional.empty(); + } + } +} diff --git a/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStageSpec.groovy b/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStageSpec.groovy index 6dba18c3172..7622efd5f08 100644 --- a/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStageSpec.groovy +++ b/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/pipeline/ManualJudgmentStageSpec.groovy @@ -16,9 +16,17 @@ package com.netflix.spinnaker.orca.echo.pipeline +import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.fiat.model.UserPermission +import com.netflix.spinnaker.fiat.model.resources.Role +import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator +import com.netflix.spinnaker.fiat.shared.FiatStatus import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution import com.netflix.spinnaker.orca.echo.EchoService +import com.netflix.spinnaker.orca.echo.util.ManualJudgmentAuthorization +import com.netflix.spinnaker.orca.front50.Front50Service +import com.netflix.spinnaker.orca.front50.model.Application import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl import spock.lang.Specification @@ -27,17 +35,42 @@ import static com.netflix.spinnaker.orca.echo.pipeline.ManualJudgmentStage.Notif import static com.netflix.spinnaker.orca.echo.pipeline.ManualJudgmentStage.WaitForManualJudgmentTask class ManualJudgmentStageSpec extends Specification { + EchoService echoService = Mock(EchoService) + + Front50Service front50Service = Mock(Front50Service) + + FiatPermissionEvaluator fiatPermissionEvaluator = Mock(FiatPermissionEvaluator) + + FiatStatus fiatStatus = Mock() { + _ * isEnabled() >> true + } + + ManualJudgmentAuthorization manualJudgmentAuthorization = new ManualJudgmentAuthorization( + Optional.of(front50Service), + Optional.of(fiatPermissionEvaluator), + fiatStatus, + new ObjectMapper() + ) + + def config = [ + application: [ + "name" : "orca", + "owner" : "owner", + "permissions" : [WRITE: ["foo"], READ: ["foo","baz"], EXECUTE: ["foo"]] + ], + user : "testUser" + ] + @Unroll void "should return execution status based on judgmentStatus"() { given: - def task = new WaitForManualJudgmentTask() + def task = new WaitForManualJudgmentTask(echoService, manualJudgmentAuthorization) when: def result = task.execute(new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", context)) then: result.status == expectedStatus - result.context.isEmpty() where: context || expectedStatus @@ -49,9 +82,37 @@ class ManualJudgmentStageSpec extends Specification { [judgmentStatus: "unknown"] || ExecutionStatus.RUNNING } + @Unroll + void "should return execution status based on authorizedGroups"() { + given: + 1 * fiatPermissionEvaluator.getPermission('abc@somedomain.io') >> { + new UserPermission().addResources([new Role('foo')]).view + } + 1 * front50Service.get("orca") >> new Application(config.application) + + def task = new WaitForManualJudgmentTask(echoService, manualJudgmentAuthorization) + + when: + def stage = new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", context) + stage.lastModified = new StageExecution.LastModifiedDetails(user: "abc@somedomain.io", allowedAccounts: ["group1"]) + def result = task.execute(stage) + + then: + result.status == expectedStatus + + where: + context || expectedStatus + [judgmentStatus: "continue", selectedStageRoles: ['foo']] || ExecutionStatus.SUCCEEDED + [judgmentStatus: "Continue", selectedStageRoles: ['foo']] || ExecutionStatus.SUCCEEDED + [judgmentStatus: "stop", selectedStageRoles: ['foo']] || ExecutionStatus.TERMINAL + [judgmentStatus: "STOP", selectedStageRoles: ['foo']] || ExecutionStatus.TERMINAL + [judgmentStatus: "Continue", selectedStageRoles: ['baz']] || ExecutionStatus.RUNNING + [judgmentStatus: "Stop", selectedStageRoles: ['baz']] || ExecutionStatus.RUNNING + } + void "should only send notifications for supported types"() { given: - def task = new WaitForManualJudgmentTask(echoService: Mock(EchoService)) + def task = new WaitForManualJudgmentTask(echoService, manualJudgmentAuthorization) when: def result = task.execute(new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", [notifications: [ @@ -72,7 +133,7 @@ class ManualJudgmentStageSpec extends Specification { @Unroll void "if deprecated notification configuration is in use, only send notifications for awaiting judgment state"() { given: - def task = new WaitForManualJudgmentTask(echoService: Mock(EchoService)) + def task = new WaitForManualJudgmentTask(echoService, manualJudgmentAuthorization) when: def result = task.execute(new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", [ @@ -153,7 +214,7 @@ class ManualJudgmentStageSpec extends Specification { @Unroll void "should retain unknown fields in the notification context"() { given: - def task = new WaitForManualJudgmentTask(echoService: Mock(EchoService)) + def task = new WaitForManualJudgmentTask(echoService, manualJudgmentAuthorization) def slackNotification = new Notification(type: "slack") slackNotification.setOther("customMessage", "hello slack") diff --git a/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorizationSpec.groovy b/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorizationSpec.groovy new file mode 100644 index 00000000000..61cda76a429 --- /dev/null +++ b/orca-echo/src/test/groovy/com/netflix/spinnaker/orca/echo/util/ManualJudgmentAuthorizationSpec.groovy @@ -0,0 +1,57 @@ +/* + * Copyright 2020 OpsMx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.orca.echo.util + +import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator +import com.netflix.spinnaker.fiat.shared.FiatStatus +import com.netflix.spinnaker.orca.front50.Front50Service +import spock.lang.Specification +import spock.lang.Subject +import spock.lang.Unroll + +class ManualJudgmentAuthorizationSpec extends Specification { + def front50Service = Mock(Front50Service) + def fiatPermissionEvaluator = Mock(FiatPermissionEvaluator) + def fiatStatus = Mock(FiatStatus) + + @Subject + def manualJudgmentAuthorization = new ManualJudgmentAuthorization( + Optional.of(front50Service), + Optional.of(fiatPermissionEvaluator), + fiatStatus, + new ObjectMapper() + ) + + @Unroll + void 'should determine authorization based on intersection of userRoles and stageRoles/permissions'() { + when: + def result = manualJudgmentAuthorization.isAuthorized(requiredJudgmentRoles, currentUserRoles, permissions) + + then: + result == isAuthorized + + where: + requiredJudgmentRoles | currentUserRoles | permissions || isAuthorized + ['foo', 'blaz'] | ['foo', 'baz'] | ["WRITE": ["foo"], "READ": ["foo", "baz"], "EXECUTE": ["foo"]] || true + [] | ['foo', 'baz'] | ["WRITE": ["foo"], "READ": ["foo", "baz"], "EXECUTE": ["foo"]] || true + ['foo'] | [] | ["WRITE": ["foo"], "READ": ["foo", "baz"], "EXECUTE": ["foo"]] || false + ['baz'] | ['foo', 'baz'] | ["WRITE": ["foo"], "READ": ["foo", "baz"], "EXECUTE": ["foo"]] || true + [] | ['foo', 'baz'] | ["": ""] || true + ['baz'] | ['foo', 'baz', 'bar'] | ["WRITE": ["bar"], "READ": ["bar"], "EXECUTE": ["bar"]] || true + } +} diff --git a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/web/config/WebConfiguration.groovy b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/web/config/WebConfiguration.groovy index 402b028430d..30beef99eff 100644 --- a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/web/config/WebConfiguration.groovy +++ b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/web/config/WebConfiguration.groovy @@ -33,7 +33,6 @@ import com.netflix.spinnaker.kork.web.interceptors.MetricsInterceptor import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper import groovy.transform.CompileStatic import org.springframework.beans.factory.annotation.Autowire -import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.web.servlet.FilterRegistrationBean import org.springframework.context.annotation.Bean import org.springframework.context.annotation.ComponentScan