Skip to content

Commit

Permalink
feat(auth): Propagate X-SPINNAKER-ACCOUNTS when triggering pipelines (
Browse files Browse the repository at this point in the history 
#530)

Echo and externally triggered (via `gate`) pipelines differ in that the
former does not propagate `X-SPINNAKER-ACCOUNTS`, even when a service
account is being used.

When `services.fiat.legacyFallback` is enabled, this PR will fetch
the allowed accounts for either the specified service account name
or `anonymous`.

The goal is to make fallbacks consistent regardless of how a pipeline
was triggered.
  • Loading branch information
@ajordens
ajordens authored Apr 23, 2019
1 parent dae6199 commit d96013b
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 15 deletions.
54 changes: 52 additions & 2 deletions ...ers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
import com.netflix.spinnaker.echo.model.Pipeline;
import com.netflix.spinnaker.echo.pipelinetriggers.QuietPeriodIndicator;
import com.netflix.spinnaker.echo.pipelinetriggers.orca.OrcaService.TriggerResponse;
import com.netflix.spinnaker.fiat.model.Authorization;
import com.netflix.spinnaker.fiat.model.UserPermission;
import com.netflix.spinnaker.fiat.model.resources.Account;
import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator;
import com.netflix.spinnaker.fiat.shared.FiatStatus;
import com.netflix.spinnaker.security.AuthenticatedRequest;
import com.netflix.spinnaker.security.User;
Expand All @@ -37,8 +41,12 @@
import rx.Observable;
import rx.functions.Func1;

import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

/**
* Triggers a {@link Pipeline} by invoking _Orca_.
Expand All @@ -49,7 +57,9 @@ public class PipelineInitiator {

private final Registry registry;
private final OrcaService orca;
private final FiatPermissionEvaluator fiatPermissionEvaluator;
private final FiatStatus fiatStatus;

private final ObjectMapper objectMapper;
private final QuietPeriodIndicator quietPeriodIndicator;
private final boolean enabled;
Expand All @@ -59,6 +69,7 @@ public class PipelineInitiator {
@Autowired
public PipelineInitiator(@NonNull Registry registry,
@NonNull OrcaService orca,
@NonNull Optional<FiatPermissionEvaluator> fiatPermissionEvaluator,
@NonNull FiatStatus fiatStatus,
ObjectMapper objectMapper,
@NonNull QuietPeriodIndicator quietPeriodIndicator,
Expand All @@ -67,6 +78,7 @@ public PipelineInitiator(@NonNull Registry registry,
@Value("${orca.pipelineInitiatorRetryDelayMillis:5000}") long retryDelayMillis) {
this.registry = registry;
this.orca = orca;
this.fiatPermissionEvaluator = fiatPermissionEvaluator.orElse(null);
this.fiatStatus = fiatStatus;
this.objectMapper = objectMapper;
this.quietPeriodIndicator = quietPeriodIndicator;
Expand Down Expand Up @@ -130,8 +142,15 @@ private void triggerPipeline(Pipeline pipeline) throws Exception {
} else {
// If we should not propagate authentication, create an empty User object for the request
User korkUser = new User();
if (fiatStatus.isEnabled() && pipeline.getTrigger() != null) {
korkUser.setEmail(pipeline.getTrigger().getRunAsUser());
if (fiatStatus.isEnabled()) {
if (pipeline.getTrigger() != null && pipeline.getTrigger().getRunAsUser() != null) {
korkUser.setEmail(pipeline.getTrigger().getRunAsUser());
} else {
// consistent with the existing pattern of `AuthenticatedRequest.getSpinnakerUser().orElse("anonymous")`
// and defaulting to `anonymous` throughout all Spinnaker services
korkUser.setEmail("anonymous");
}
korkUser.setAllowedAccounts(getAllowedAccountsForUser(korkUser.getEmail()));
}
AuthenticatedRequest.propagate(() -> orcaResponse.subscribe(), korkUser).call();
}
Expand All @@ -150,6 +169,37 @@ private void onOrcaError(Pipeline pipeline, Throwable error) {
log.error("Error triggering pipeline: {}", pipeline, error);
}

/**
* The set of accounts that a user has WRITE access to.
*
* Similar filtering can be found in `gate` (see AllowedAccountsSupport.java).
*
* @param user A service account name (or 'anonymous' if not specified)
* @return the allowed accounts for {@param user} as determined by fiat
*/
private Set<String> getAllowedAccountsForUser(String user) {
if (fiatPermissionEvaluator == null || !fiatStatus.isLegacyFallbackEnabled()) {
return Collections.emptySet();
}

UserPermission.View userPermission = null;
try {
userPermission = fiatPermissionEvaluator.getPermission(user);
} catch (Exception e) {
log.error("Unable to fetch permission for {}", user, e);
}

if (userPermission == null) {
return Collections.emptySet();
}

return userPermission.getAccounts()
.stream()
.filter(v -> v.getAuthorizations().contains(Authorization.WRITE))
.map(Account.View::getName)
.collect(Collectors.toSet());
}

private static boolean isRetryable(Throwable error) {
if (!(error instanceof RetrofitError)) {
return false;
Expand Down
113 changes: 100 additions & 13 deletions ...test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiatorSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,15 @@ package com.netflix.spinnaker.echo.pipelinetriggers.orca
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spectator.api.NoopRegistry
import com.netflix.spinnaker.echo.model.Pipeline
import com.netflix.spinnaker.echo.model.Trigger
import com.netflix.spinnaker.echo.pipelinetriggers.QuietPeriodIndicator
import com.netflix.spinnaker.fiat.model.Authorization
import com.netflix.spinnaker.fiat.model.UserPermission
import com.netflix.spinnaker.fiat.model.resources.Account
import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator
import com.netflix.spinnaker.fiat.shared.FiatStatus
import com.netflix.spinnaker.security.AuthenticatedRequest
import rx.functions.Action0
import spock.lang.Specification
import spock.lang.Unroll

Expand All @@ -13,54 +20,134 @@ import static rx.Observable.empty
class PipelineInitiatorSpec extends Specification {
def registry = new NoopRegistry()
def orca = Mock(OrcaService)
def fiatPermissionEvaluator = Mock(FiatPermissionEvaluator)
def fiatStatus = Mock(FiatStatus)
def objectMapper = Mock(ObjectMapper)
def quietPeriodIndicator = Mock(QuietPeriodIndicator)

Optional<String> capturedSpinnakerUser
Optional<String> capturedSpinnakerAccounts

def userPermissions = [
"anonymous": new UserPermission.View(
accounts: [
account("account1", ["READ"]),
account("account2", ["READ", "WRITE"]),
account("account3", ["READ", "WRITE"])
] as Set<Account.View>
),
"not-anonymous": new UserPermission.View(
accounts: [
account("account1", ["READ", "WRITE"]),
account("account2", ["READ", "WRITE"]),
account("account3", ["READ", "WRITE"])
] as Set<Account.View>
)
]

void setup() {
capturedSpinnakerUser = Optional.empty()
capturedSpinnakerAccounts = Optional.empty()
}

@Unroll
def "calls orca #orcaCalls times when enabled=#enabled flag"() {
given:
def pipelineInitiator = new PipelineInitiator(registry, orca, fiatStatus, objectMapper, quietPeriodIndicator, enabled, 5, 5000)
def pipeline = Pipeline.builder().application("application").name("name").id("id").type("pipeline").build()
def pipelineInitiator = new PipelineInitiator(
registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, objectMapper, quietPeriodIndicator, enabled, 5, 5000
)

def pipeline = Pipeline
.builder()
.application("application")
.name("name")
.id("id")
.type("pipeline")
.trigger(
new Trigger.TriggerBuilder().type("cron").runAsUser(user).build()
)
.build()

when:
pipelineInitiator.startPipeline(pipeline)

then:
_ * fiatStatus.isEnabled() >> { return enabled }
orcaCalls * orca.trigger(pipeline) >> empty()
_ * fiatStatus.isLegacyFallbackEnabled() >> { return legacyFallbackEnabled }

(legacyFallbackEnabled ? 1 : 0) * fiatPermissionEvaluator.getPermission(user ?: "anonymous") >> {
return userPermissions.get(user ?: "anonymous")
}

expectedTriggerCalls * orca.trigger(pipeline) >> {
return empty().doOnSubscribe(captureAuthorizationContext())
}

capturedSpinnakerUser.orElse(null) == expectedSpinnakerUser
capturedSpinnakerAccounts.orElse(null)?.split(",") as Set<String> == expectedSpinnakerAccounts?.split(",") as Set<String>

where:
enabled || orcaCalls
true || 1
false || 0
user | enabled | legacyFallbackEnabled || expectedTriggerCalls || expectedSpinnakerUser || expectedSpinnakerAccounts
"anonymous" | false | false || 0 || null || null // fiat is not enabled
"anonymous" | true | false || 1 || "anonymous" || null // fallback disabled (no accounts)
"anonymous" | true | true || 1 || "anonymous" || "account2,account3" // fallback enabled (all WRITE accounts)
"not-anonymous" | true | true || 1 || "not-anonymous" || "account1,account2,account3" // fallback enabled (all WRITE accounts)
null | true | true || 1 || "anonymous" || "account2,account3" // null trigger user should default to 'anonymous'
}

@Unroll
def "calls orca #orcaCalls to plan pipeline if templated"() {
given:
def pipelineInitiator = new PipelineInitiator(registry, orca, fiatStatus, objectMapper, quietPeriodIndicator, true, 5, 5000)
def pipelineInitiator = new PipelineInitiator(
registry, orca, Optional.empty(), fiatStatus, objectMapper, quietPeriodIndicator, true, 5, 5000
)

def pipeline = Pipeline.builder()
.application("application")
.name("name")
.id("id")
.type(type)
.build()

def pipelineMap = pipeline as Map

when:
pipelineInitiator.startPipeline(pipeline)

then:
1 * fiatStatus.isEnabled() >> { return true }
orcaCalls * orca.plan(_, true) >> pipelineMap
expectedPlanCalls * orca.plan(_, true) >> pipelineMap
objectMapper.convertValue(pipelineMap, Pipeline.class) >> pipeline
1 * orca.trigger(_) >> empty()
1 * orca.trigger(_) >> {
return empty().doOnSubscribe(captureAuthorizationContext())
}

capturedSpinnakerUser.orElse(null) == expectedSpinnakerUser
capturedSpinnakerAccounts.orElse(null) == expectedSpinnakerAccounts

where:
type || orcaCalls
"pipeline" || 0
"templatedPipeline" || 1
null || 0
type || expectedPlanCalls || expectedSpinnakerUser || expectedSpinnakerAccounts
"pipeline" || 0 || "anonymous" || null
"templatedPipeline" || 1 || "anonymous" || null
null || 0 || "anonymous" || null
}

private Action0 captureAuthorizationContext() {
new Action0() {
@Override
void call() {
capturedSpinnakerUser = AuthenticatedRequest.getSpinnakerUser()
capturedSpinnakerAccounts = AuthenticatedRequest.getSpinnakerAccounts()
}
}
}

private static Account.View account(String name, Collection<String> authorizations) {
def accountView = new Account.View()

accountView.name = name
accountView.authorizations = authorizations.collect { Authorization.valueOf(it) }

return accountView
}
}

0 comments on commit d96013b

Please sign in to comment.