Skip to content

Commit

Permalink
[WX-1764] Fix improper authorization for POST list method permissions (
Browse files Browse the repository at this point in the history
  • Loading branch information
salonishah11 authored Sep 11, 2024
1 parent 524716f commit 904ebc8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,10 @@ class PermissionBusiness(permissionsDataSource: PermissionsDataSource) {
} recover {
// AgoraEntityAuthorizationException means we don't have permissions to read the entity's acls,
// or the entity doesn't exist. For purposes of this method, call these non-fatal.
// we don't recover from any other exceptions.
case aeae: AgoraEntityAuthorizationException => EntityAccessControl(annotatedEntity, Seq.empty[AccessControl], Some(aeae.getMessage))
// we don't recover from any other exceptions. Additionally, for unauthorized access return entity information
// that was already part of request. Don't send additional information. This meets previous response expectations
// and fixes the issue mentioned in https://broadworkbench.atlassian.net/browse/WX-1764.
case aeae: AgoraEntityAuthorizationException => EntityAccessControl(entity, Seq.empty[AccessControl], Some(aeae.getMessage))
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ class PermissionIntegrationSpec extends AnyFlatSpec with ScalatestRouteTest with
val stubEntity = AgoraEntity(agoraEntity2.namespace, agoraEntity2.name, agoraEntity2.snapshotId)
val found = entityAclList.find(_.entity.toShortString == stubEntity.toShortString)
assert(found.isDefined, "second")
assertResult(Set(owner2.get), "second") {found.get.entity.managers.toSet}
assert(found.get.entity.managers.isEmpty, "second") // since user doesn't have permission they shouldn't see additional information about method
assert(found.get.message.isDefined, "second")
assert(found.get.message.get.contains("Authorization exception for user"), "second")
}
// check third - it doesn't exist in the db
{
Expand Down Expand Up @@ -434,21 +436,27 @@ class PermissionIntegrationSpec extends AnyFlatSpec with ScalatestRouteTest with
val stubEntity = AgoraEntity(agoraEntity2.namespace, agoraEntity2.name, agoraEntity2.snapshotId)
val found = entityAclList.find(_.entity.toShortString == stubEntity.toShortString)
assert(found.isDefined, "second")
assertResult(Some(true)) {found.get.entity.public}
// since user doesn't have permission they shouldn't see additional information about method
assertResult(None) {found.get.entity.public}
assert(found.get.message.get.contains("Authorization exception for user"), "second")
}
// check third - it doesn't exist in the db
{
val stubEntity = AgoraEntity(agoraEntity1.namespace, agoraEntity1.name, Some(12345))
val found = entityAclList.find(_.entity.toShortString == stubEntity.toShortString)
assert(found.isDefined, "third")
assertResult(Some(false)) {found.get.entity.public} // entity doesn't exist, so it's not public
// it seems when entity doesn't exist 'AgoraEntityAuthorizationException' is thrown and hence
// public information is now not available because of https://broadworkbench.atlassian.net/browse/WX-1764
assertResult(None) {found.get.entity.public}
}
// check fourth - it has been redacted, which resolves to us not having permissions to see it
{
val stubEntity = AgoraEntity(redactedEntity.namespace, redactedEntity.name, redactedEntity.snapshotId)
val found = entityAclList.find(_.entity.toShortString == stubEntity.toShortString)
assert(found.isDefined, "fourth")
assertResult(Some(false)) {found.get.entity.public} // when redacted, so it's not public
// it seems when entity is redacted 'AgoraEntityAuthorizationException' is thrown and hence
// public information is now not available because of https://broadworkbench.atlassian.net/browse/WX-1764
assertResult(None) {found.get.entity.public}
}
}
}
Expand Down

0 comments on commit 904ebc8

Please sign in to comment.