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)

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
xavileon authored and mergify[bot] committed Nov 22, 2019
1 parent d77c99a commit 2bf1472
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
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
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
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 2bf1472

Please sign in to comment.