fix(unrestricted): load permissions for unrestricted roles (#500)

The unrestricted user can have associated roles via
`UserRolesProvider.loadUnrestrictedRoles`, however this would
always short-circuit evaluation of permissions for those roles.

This resulted in the case where on the first authorization
(`POST /roles/:userId`) the user would not have access to any
resources that were granted solely via the unrestricted role.
However in a subsequent authentication - the users permissions
would get merged with an existing user entry and the unrestricted
roles would show up as if they were directly granted to the user
(due to thge way the `UserPermissions.merge` happens in the
`RedisPermissionRepository`) and eventually the resources protected
by unrestricted roles would show up.

fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java

@@ -117,7 +117,9 @@ private UserPermission getUserPermission(String userId, Set<Role> roles) {
       try {
         if (UnrestrictedResourceConfig.UNRESTRICTED_USERNAME.equalsIgnoreCase(userId)) {
           permission.addResources(provider.getAllUnrestricted());
-        } else if (!roles.isEmpty()) {
+        }
+
+        if (!roles.isEmpty()) {
           permission.addResources(provider.getAllRestricted(roles, permission.isAdmin()));
         }
       } catch (ProviderException pe) {

fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolverSpec.groovy

@@ -19,9 +19,11 @@ package com.netflix.spinnaker.fiat.permissions
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.fiat.config.FiatAdminConfig
 import com.netflix.spinnaker.fiat.config.FiatRoleConfig
+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.model.resources.Application
+import com.netflix.spinnaker.fiat.model.resources.Permissions
 import com.netflix.spinnaker.fiat.model.resources.Role
 import com.netflix.spinnaker.fiat.model.resources.ServiceAccount
 import com.netflix.spinnaker.fiat.providers.AccessControlledResourcePermissionSource
@@ -40,6 +42,9 @@ import spock.lang.Subject
 class DefaultPermissionsResolverSpec extends Specification {
   UserRolesProvider userRolesProvider = Mock(UserRolesProvider)
 
+  @Shared
+  Role anonymous = new Role("anonymous")
+
   @Shared
   Account noReqGroupsAcct = new Account().setName("noReqGroups")
 
@@ -50,9 +55,13 @@ class DefaultPermissionsResolverSpec extends Specification {
   Account reqGroup1and2Acct = new Account().setName("reqGroup1and2")
                                            .setRequiredGroupMembership(["group1", "GrouP2"]) // test case insensitivity.
 
+  @Shared
+  Account anonymousRead = new Account().setName("anonymousRead")
+                                       .setPermissions(Permissions.factory((Authorization.READ): [anonymous.name], (Authorization.WRITE): ["otherGroup"]))
+
   @Shared
   ClouddriverService clouddriverService = Mock(ClouddriverService) {
-    getAccounts() >> [noReqGroupsAcct, reqGroup1Acct, reqGroup1and2Acct]
+    getAccounts() >> [noReqGroupsAcct, reqGroup1Acct, reqGroup1and2Acct, anonymousRead]
   }
 
   @Shared
@@ -102,8 +111,8 @@ class DefaultPermissionsResolverSpec extends Specification {
     1 * userRolesProvider.loadUnrestrictedRoles() >> { return [new Role("anonymous")] }
 
     def expected = new UserPermission().setId("__unrestricted_user__")
-                                       .setAccounts([noReqGroupsAcct] as Set)
-                                       .setRoles([new Role("anonymous")] as Set)
+                                       .setAccounts([noReqGroupsAcct, anonymousRead] as Set)
+                                       .setRoles([anonymous] as Set)
     result == expected
   }
 
@@ -183,7 +192,7 @@ class DefaultPermissionsResolverSpec extends Specification {
     def expected = new UserPermission().setId("testUserId")
     expected.setRoles([role1] as Set).setAdmin(true)
             .setServiceAccounts([group1SvcAcct, group2SvcAcct] as Set)
-            .setAccounts([reqGroup1Acct, reqGroup1and2Acct] as Set)
+            .setAccounts([reqGroup1Acct, reqGroup1and2Acct, anonymousRead] as Set)
     result == expected
 
   }