Merge pull request #1162 from nspadaccino/nspadaccino/view-shared-zones

Allow All Users to View Any Shared Zone

modules/api/src/main/scala/vinyldns/api/domain/access/AccessValidations.scala

@@ -34,7 +34,7 @@ class AccessValidations(
     ensuring(
       NotAuthorizedError(s"User ${auth.signedInUser.userName} cannot access zone '${zone.name}'")
     )(
-      auth.isSystemAdmin || auth
+      auth.isSystemAdmin || zone.shared || auth
         .isGroupMember(zone.adminGroupId) || userHasAclRules(auth, zone)
     )
 

modules/api/src/test/functional/tests/zones/get_zone_test.py

@@ -34,12 +34,17 @@ def test_get_zone_shared_by_id_as_owner(shared_zone_test_context):
 
 def test_get_zone_shared_by_id_non_owner(shared_zone_test_context):
     """
-    Test get an existing shared zone by id as a zone owner
+    Test get an existing shared zone by id as a non-zone-owner. Non-owner should have read-only access
     """
     client = shared_zone_test_context.dummy_vinyldns_client
+    group_name = shared_zone_test_context.shared_record_group["name"]
+    result = client.get_zone(shared_zone_test_context.shared_zone["id"], status=200)
+    retrieved = result["zone"]
 
-    client.get_zone(shared_zone_test_context.shared_zone["id"], status=403)
+    assert_that(retrieved["id"], is_(shared_zone_test_context.shared_zone["id"]))
-
+    assert_that(retrieved["adminGroupName"], is_(group_name))
+    assert_that(retrieved["shared"], is_(True))
+    assert_that(retrieved["accessLevel"], is_("Read"))
 
 def test_get_zone_private_by_id_fails_without_access(shared_zone_test_context):
     """

modules/api/src/test/functional/tests/zones/list_zones_test.py

@@ -216,11 +216,11 @@ def test_list_zones_ignore_access_success(shared_zone_test_context):
 
 def test_list_zones_ignore_access_success_with_name_filter(shared_zone_test_context):
     """
-    Test that we can retrieve a list of all zones with a name filter
+    Test that we can retrieve a list of all zones with a name filter. Should have Read access to shared zone
     """
     result = shared_zone_test_context.list_zones_client.list_zones(name_filter=shared_zone_test_context.shared_zone["name"].rstrip("."), ignore_access=True, status=200)
     retrieved = result["zones"]
 
     assert_that(result["ignoreAccess"], is_(True))
     assert_that(retrieved, has_item(has_entry("name", shared_zone_test_context.shared_zone["name"])))
-    assert_that(retrieved, has_item(has_entry("accessLevel", "NoAccess")))
+    assert_that(retrieved, has_item(has_entry("accessLevel", "Read")))

modules/api/src/test/scala/vinyldns/api/domain/access/AccessValidationsSpec.scala

@@ -126,9 +126,8 @@ class AccessValidationsSpec
       accessValidationTest.canSeeZone(supportAuth, okZone) should be(right)
     }
 
-    "return false if the zone is shared and user does not have other access" in {
+    "return true if the zone is shared and user does not have other access" in {
-      val error = leftValue(accessValidationTest.canSeeZone(okAuth, sharedZone))
+      accessValidationTest.canSeeZone(okAuth, sharedZone) should be(right)
-      error shouldBe a[NotAuthorizedError]
     }
   }
 
@@ -1201,8 +1200,8 @@ class AccessValidationsSpec
       accessValidationTest.getZoneAccess(supportUserAuth, abcZone) should be(AccessLevel.Read)
     }
 
-    "return access level NoAccess if zone is shared and user is not an admin" in {
+    "return access level Read if zone is shared and user is not an admin" in {
-      accessValidationTest.getZoneAccess(okAuth, sharedZone) should be(AccessLevel.NoAccess)
+      accessValidationTest.getZoneAccess(okAuth, sharedZone) should be(AccessLevel.Read)
     }
 
     "return access level Read if zone is private and user is an ACL rule" in {

modules/portal/app/views/zones/zoneTabs/manageRecords.scala.html

@@ -66,7 +66,7 @@
       <div class="vinyldns-panel-top">
         <div class="btn-group">
             <button id="refresh-records-button" class="btn btn-default" ng-click="refreshRecords()"><span class="fa fa-refresh"></span> Refresh</button>
-            <button id="create-record-button" class="btn btn-default" ng-if="canCreateRecords" ng-click="createRecord(defaultTtl)"><span class="fa fa-plus"></span> Create Record Set</button>
+            <button id="create-record-button" class="btn btn-default" ng-if="canReadZone && (zoneInfo.accessLevel == 'Delete' || canCreateRecordsViaAcl())" ng-click="createRecord(defaultTtl)"><span class="fa fa-plus"></span> Create Record Set</button>
             <button id="zone-sync-button" class="btn btn-default mb-control" ng-if="zoneInfo.accessLevel=='Delete'" data-toggle="modal" data-target="#mb-sync"><span class="fa fa-exchange"></span> Sync Zone</button>
         </div>
 
@@ -324,7 +324,7 @@
                         </td>
                     }
                     <td>
-                        <span ng-if="(record.canBeEdited && record.accessLevel != 'NoAccess' && record.accessLevel != 'Read')">
+                        <span ng-if="(canReadZone && record.canBeEdited && record.accessLevel != 'NoAccess' && record.accessLevel != 'Read')">
                             <div class="table-form-group">
                                 <span><button class="btn btn-info btn-sm" ng-click="editRecord(record)">Update</button></span>
                                 <span ng-if="record.accessLevel == 'Delete'"><button id="delete-record-{{record.name}}-button" class="btn btn-danger btn-sm btn-rounded" ng-click="deleteRecord(record)">Delete</button></span>

modules/portal/public/lib/controllers/controller.records.js

@@ -369,8 +369,7 @@ angular.module('controller.records', [])
     function determineAdmin(){
         $scope.isZoneAdmin = $scope.profile.isSuper || isInAdminGroup();
         $scope.canReadZone = canReadZone();
-        $scope.canCreateRecords = $scope.zoneInfo.accessLevel == 'Delete' || $scope.zoneInfo.shared ||
+        $scope.canCreateRecords = $scope.zoneInfo.accessLevel == 'Delete' || canCreateRecordsViaAcl() || $scope.zoneInfo.shared;
-            canCreateRecordsViaAcl();
 
         function canCreateRecordsViaAcl() {
             return $scope.zoneInfo.acl.rules.some(b => b.accessLevel == "Write" || b.accessLevel == "Delete")

modules/portal/public/templates/record-modal.html

@@ -445,15 +445,21 @@
                 </modal-invalid>
             </modal-element>
 
-            <modal-element label="Record Owner Group" ng-if="recordModal.sharedDisplayEnabled && recordModal.sharedZone">
+            <modal-element label="Record Owner Group"
-                <select class="form-control"
+                           invalid-when="addRecordForm.$submitted && addRecordForm.ownerGroupId.$invalid"
+                           ng-if="recordModal.sharedDisplayEnabled && recordModal.sharedZone">
+                <select name="ownerGroupId"
+                        class="form-control"
                         ng-model="currentRecord.ownerGroupId"
-                        ng-disabled="recordModal.details.readOnly">
+                        ng-disabled="recordModal.details.readOnly"
-                    <option value=""></option>
+                        ng-class="recordModal.details.class"
-                    <option ng-repeat="group in myGroups | orderBy: 'name'" value="{{ group.id }}"
+                        ng-options="group.id as group.name for group in myGroups | orderBy: 'name'"
-                            ng-selected="currentRecord.ownerGroupId == group.id">
+                        required>
-                        {{group.name}}</option>
+                    <option value="" selected="selected">Please choose a record owner group</option>
                 </select>
+                <modal-invalid>
+                    Record Owner Group is required for records in shared zones
+                </modal-invalid>
             </modal-element>
 
         </modal-body>