Skip to content

Commit

Permalink
fix(notifications): Propagate auth for application notifications (spi…
Browse files Browse the repository at this point in the history
…nnaker#2609)

Application and pipeline level notifications fails for protected apps, because orca is not propagating authentication to front50. 

That's sad if you want both security _and_ notifications. 

But I'm pleased to announce that with this PR, you can have your cake AND eat it too.
  • Loading branch information
jervi authored and ajordens committed Jan 21, 2019
1 parent e7b0755 commit f8dcac8
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import com.netflix.spinnaker.orca.listeners.ExecutionListener
import com.netflix.spinnaker.orca.listeners.Persister
import com.netflix.spinnaker.orca.pipeline.model.Execution
import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor
import com.netflix.spinnaker.security.AuthenticatedRequest
import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j

import static com.netflix.spinnaker.orca.pipeline.model.Execution.ExecutionType.PIPELINE

@Slf4j
Expand Down Expand Up @@ -102,7 +104,15 @@ class EchoNotifyingExecutionListener implements ExecutionListener {
* @param pipeline
*/
private void addApplicationNotifications(Execution pipeline) {
ApplicationNotifications notifications = front50Service.getApplicationNotifications(pipeline.application)
def user = pipeline.getAuthentication()?.toKorkUser()
ApplicationNotifications notifications
if (user?.isPresent()) {
notifications = AuthenticatedRequest.propagate({
front50Service.getApplicationNotifications(pipeline.application)
}, user.get()).call()
} else {
notifications = front50Service.getApplicationNotifications(pipeline.application)
}

if (notifications) {
notifications.getPipelineNotifications().each { appNotification ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ import com.netflix.spinnaker.orca.front50.model.ApplicationNotifications
import com.netflix.spinnaker.orca.front50.model.ApplicationNotifications.Notification
import com.netflix.spinnaker.orca.pipeline.model.Execution
import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor
import com.netflix.spinnaker.security.AuthenticatedRequest
import org.slf4j.MDC
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject

import static com.netflix.spinnaker.orca.pipeline.model.Execution.ExecutionType.PIPELINE

class EchoNotifyingExecutionListenerSpec extends Specification {

def echoService = Mock(EchoService)
Expand Down Expand Up @@ -173,4 +177,23 @@ class EchoNotifyingExecutionListenerSpec extends Specification {
1 * echoService.recordEvent(_)
0 * _
}

def "propagates authentication details to front50"() {
given:
def pipeline = new Execution(PIPELINE, "myapp")
pipeline.setAuthentication(new Execution.AuthenticationDetails("[email protected]", "someAccount", "anotherAccount"))

when:
echoListener.beforeExecution(null, pipeline)

then:
pipeline.notifications == [slackPipes]

1 * front50Service.getApplicationNotifications("myapp") >> {
assert MDC.get(AuthenticatedRequest.SPINNAKER_USER) == "[email protected]"
assert MDC.get(AuthenticatedRequest.SPINNAKER_ACCOUNTS) == "someAccount,anotherAccount"
return notifications
}
1 * echoService.recordEvent(_)
}
}

0 comments on commit f8dcac8

Please sign in to comment.