Skip to content

Commit

Permalink
fix(serviceAccount): Filter non-valid roles when converting to UserPe…
Browse files Browse the repository at this point in the history
…rmission (#513) (#514)

Roles can't be empty. This leads to unexpected behaviour. However, we
were allowing creating empty roles like "" or "   " on the pipeline
triggersi (via API), which made service users to contain invalid roles
and thus failing on every sync request that tries to map roles to accounts.
This rendered FIAT unusable to get permissions and subsequently not
allowing any authorization operation.

This patch sanitizes the input on ServiceAccounts so we make sure
that the roles considered are valid.
  • Loading branch information
spinnakerbot authored and Travis Tomsu committed Dec 2, 2019
1 parent 47a6a00 commit c62d038
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import org.springframework.util.StringUtils;

@Data
@EqualsAndHashCode(of = "name")
Expand All @@ -46,7 +47,7 @@ public Role(String name) {
}

public Role setName(@Nonnull String name) {
if (name.isEmpty()) {
if (!StringUtils.hasText(name)) {
throw new IllegalArgumentException("name cannot be empty");
}
this.name = name.toLowerCase();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import lombok.val;
import org.springframework.util.StringUtils;

@Data
@EqualsAndHashCode(callSuper = false)
Expand All @@ -38,6 +39,7 @@ public class ServiceAccount implements Resource, Viewable {
public UserPermission toUserPermission() {
val roles =
memberOf.stream()
.filter(StringUtils::hasText)
.map(membership -> new Role(membership).setSource(Role.Source.EXTERNAL))
.collect(Collectors.toSet());
return new UserPermission().setId(name).setRoles(roles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import spock.lang.Specification

class ServiceAccountSpec extends Specification {

def 'should convert to UserPermission'() {
def 'should convert to UserPermission, filtering non-text strings'() {
setup:
ServiceAccount acct = new ServiceAccount().setName("my-svc-acct")
.setMemberOf(["foo", "bar"])
.setMemberOf(["foo", "bar", "", " "])

when:
def result = acct.toUserPermission()
Expand Down

0 comments on commit c62d038

Please sign in to comment.