diff --git a/modules/api/src/main/scala/vinyldns/api/domain/access/AccessValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/access/AccessValidations.scala index 4aef5a679..845b2c034 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/access/AccessValidations.scala +++ b/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) ) diff --git a/modules/api/src/test/functional/tests/zones/get_zone_test.py b/modules/api/src/test/functional/tests/zones/get_zone_test.py index 2e5ba57ff..0068c2bfa 100644 --- a/modules/api/src/test/functional/tests/zones/get_zone_test.py +++ b/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): """ diff --git a/modules/api/src/test/functional/tests/zones/list_zones_test.py b/modules/api/src/test/functional/tests/zones/list_zones_test.py index 1caf96e31..7ffb72ba2 100644 --- a/modules/api/src/test/functional/tests/zones/list_zones_test.py +++ b/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"))) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/access/AccessValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/access/AccessValidationsSpec.scala index 286cad05e..d32f41055 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/access/AccessValidationsSpec.scala +++ b/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 { - val error = leftValue(accessValidationTest.canSeeZone(okAuth, sharedZone)) - error shouldBe a[NotAuthorizedError] + "return true if the zone is shared and user does not have other access" in { + accessValidationTest.canSeeZone(okAuth, sharedZone) should be(right) } } @@ -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 { - accessValidationTest.getZoneAccess(okAuth, sharedZone) should be(AccessLevel.NoAccess) + "return access level Read if zone is shared and user is not an admin" in { + accessValidationTest.getZoneAccess(okAuth, sharedZone) should be(AccessLevel.Read) } "return access level Read if zone is private and user is an ACL rule" in { diff --git a/modules/portal/app/views/zones/zoneTabs/manageRecords.scala.html b/modules/portal/app/views/zones/zoneTabs/manageRecords.scala.html index 61272b245..1de4871e6 100644 --- a/modules/portal/app/views/zones/zoneTabs/manageRecords.scala.html +++ b/modules/portal/app/views/zones/zoneTabs/manageRecords.scala.html @@ -66,7 +66,7 @@
- +
@@ -324,7 +324,7 @@ } - +
diff --git a/modules/portal/public/lib/controllers/controller.records.js b/modules/portal/public/lib/controllers/controller.records.js index d9867a39d..27796f34c 100644 --- a/modules/portal/public/lib/controllers/controller.records.js +++ b/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 || - canCreateRecordsViaAcl(); + $scope.canCreateRecords = $scope.zoneInfo.accessLevel == 'Delete' || canCreateRecordsViaAcl() || $scope.zoneInfo.shared; function canCreateRecordsViaAcl() { return $scope.zoneInfo.acl.rules.some(b => b.accessLevel == "Write" || b.accessLevel == "Delete") diff --git a/modules/portal/public/templates/record-modal.html b/modules/portal/public/templates/record-modal.html index 03f3edf84..857776c69 100644 --- a/modules/portal/public/templates/record-modal.html +++ b/modules/portal/public/templates/record-modal.html @@ -445,15 +445,21 @@ - - - - + ng-disabled="recordModal.details.readOnly" + ng-class="recordModal.details.class" + ng-options="group.id as group.name for group in myGroups | orderBy: 'name'" + required> + + + Record Owner Group is required for records in shared zones +