From 156c6ea4ac0ca5a1b9283b54cfe562bf13c37ab3 Mon Sep 17 00:00:00 2001
From: Costi Muraru <costimuraru@gmail.com>
Date: Fri, 18 Oct 2019 19:10:31 +0200
Subject: [PATCH] fix(fiat): Fix access denied for new apps with no roles
 (#597)

Signed-off-by: Constantin Muraru <cmuraru@adobe.com>
---
 .../controllers/PermissionsController.groovy  |  2 +-
 .../PermissionsControllerSpec.groovy          | 75 +++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PermissionsControllerSpec.groovy

diff --git a/front50-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PermissionsController.groovy b/front50-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PermissionsController.groovy
index 186e6c00d..75f22b5bf 100644
--- a/front50-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PermissionsController.groovy
+++ b/front50-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PermissionsController.groovy
@@ -124,7 +124,7 @@ public class PermissionsController {
       roles += oldPermission.permissions.allGroups()
     }
 
-    if (!roles.isEmpty() && roleSync) {
+    if (roleSync) {
       try {
         fiatService.sync(roles as List)
       } catch (RetrofitError re) {
diff --git a/front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PermissionsControllerSpec.groovy b/front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PermissionsControllerSpec.groovy
new file mode 100644
index 000000000..8b54899c4
--- /dev/null
+++ b/front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PermissionsControllerSpec.groovy
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2019 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package com.netflix.spinnaker.front50.controllers
+
+import com.netflix.spinnaker.fiat.model.Authorization
+import com.netflix.spinnaker.fiat.model.resources.Permissions
+import com.netflix.spinnaker.fiat.shared.FiatClientConfigurationProperties
+import com.netflix.spinnaker.fiat.shared.FiatService
+import com.netflix.spinnaker.front50.model.application.Application
+import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO
+import spock.lang.Specification
+import spock.lang.Unroll
+
+public class PermissionsControllerSpec extends Specification {
+
+  @Unroll
+  def "test application creation will sync roles in fiat"(permission, expectedSyncedRoles) {
+    given:
+    def fiatService = Mock(FiatService)
+    PermissionsController permissionsController = createPermissionController(fiatService)
+    permissionsController.applicationPermissionDAO = Optional.of(Mock(ApplicationPermissionDAO) {
+      create(_, _) >> permission
+    })
+
+    when:
+    permissionsController.createApplicationPermission(permission)
+
+    then:
+    1 * fiatService.sync(expectedSyncedRoles)
+
+    where:
+    permission                                                  | expectedSyncedRoles
+    appPermission(null)                                         | []
+    appPermission(Permissions.EMPTY)                            | []
+    appPermission(permissions(Authorization.WRITE, "my_group")) | ["my_group"]
+  }
+
+  private Application.Permission appPermission(Permissions permissions) {
+    def permission = new Application.Permission()
+    permission.name = "testName"
+    permission.permissions = permissions
+    permission
+  }
+
+  private static Permissions permissions(Authorization authorization, String group) {
+    new Permissions.Builder()
+      .add(authorization, group)
+      .build()
+  }
+
+  private PermissionsController createPermissionController(FiatService fiatService) {
+    def permissionsController = new PermissionsController()
+    permissionsController.roleSync = true
+    permissionsController.fiatService = fiatService
+    permissionsController.fiatClientConfigurationProperties = Mock(FiatClientConfigurationProperties) {
+      isEnabled() >> true
+    }
+    permissionsController
+  }
+}