Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(permissions): reject malformed doap and ap create/update request …
…(DSP-1328) (#1890)

* fix (premissions): don't allow malformed hasPermissions for updating DOAP

* docs(permissions): start adding some info about content of hasPermissions

* fix (permissions): reject malformed hasPermissions, correct the faulty values + tests

* clean up
  • Loading branch information
SepidehAlassi committed Jul 20, 2021
1 parent 83fb4b8 commit 3e3a3ce
Show file tree
Hide file tree
Showing 7 changed files with 910 additions and 306 deletions.
55 changes: 54 additions & 1 deletion docs/03-apis/api-admin/permissions.md
Expand Up @@ -39,7 +39,7 @@ for the group is returned.
permissions for a project. As a response, all `default_object_acces_permissions` of a
project are returned.

### Creating New Permissions:
### Creating New Administrative Permissions:

- `POST: /admin/permissions/ap`: create a new administrative permission. The type of
permissions, the project and group to which the permission should be added must be
Expand Down Expand Up @@ -79,11 +79,40 @@ As a response, the created administrative permission and its IRI are returned as
}
}
```
`hasPermissions` contains permission types that must be granted. See [the complete description of administrative
permission types](../../05-internals/design/api-admin/administration.md#administrative-permissions).
In summary, each permission should contain followings:
- `name` : indicates the type of the permission that can be one of the followings:
- `ProjectAdminAllPermission`: gives the user the permission to do anything
on project level, i.e. create new groups, modify all
existing groups
- `ProjectAdminGroupAllPermission`: gives the user the permission to modify
*group info* and *group membership* on *all* groups
belonging to the project.
- `ProjectAdminGroupRestrictedPermission`: gives the user the permission to modify
*group info* and *group membership* on *certain* groups
belonging to the project.
- `ProjectAdminRightsAllPermission`: gives the user the permission to change the
*permissions* on all objects belonging to the project
(e.g., default permissions attached to groups and
permissions on objects).
- `ProjectResourceCreateAllPermission`: gives the permission to create resources
inside the project.
- `ProjectResourceCreateRestrictedPermission`: gives restricted resource creation permission
inside the project.

- `additionalInformation`: should be left empty, otherwise will be ignored.
- `permissionCode`: should be left empty, otherwise will be ignored.


Note that during the creation of a new project, a default set of administrative permissions are added to its ProjectAdmin and
ProjectMember groups (See [Default set of permissions for a new project](./projects.md#default-set-of-permissions-for-a-new-project)).
Therefore, it is not possible to create new administrative permissions for the ProjectAdmin and ProjectMember groups of
a project. However, the default permissions set for these groups can be modified (See [update permission](./permissions.md#updating-a-permissions-scope)).


### Creating New Default Object Access Permissions:

- `POST: /admin/permissions/doap` : create a new default object access permission.
A single instance of `knora-admin:DefaultObjectAccessPermission` must
always reference a project, but can only reference **either** a group
Expand All @@ -101,6 +130,25 @@ default object access permission for a group of a project the request body would
"hasPermissions":[{"additionalInformation":"http://www.knora.org/ontology/knora-admin#ProjectMember","name":"D","permissionCode":7}]
}
```
`hasPermissions` contains permission types that must be granted. See [a complete description of object access
permission types](../../05-internals/design/api-admin/administration.md#default-object-access-permissions).
In summary, each permission should contain followings:
- `additionalInformation`: To whom the permission should be granted: project members, known users, unknown users, etc.
- `name` : indicates the type of the permission that can be one of the followings.
- `RV`: restricted view permission (least privileged)
- `V`: view permission
- `M` modify permission
- `D`: delete permission
- `CR`: change rights permission (most privileged)
- `permissionCode`: The code assigned to a permission indicating its hierarchical level. These codes are as below:
- `1`: for restricted view permission (least privileged)
- `2`: for view permission
- `6`: for modify permission
- `7`: for delete permission
- `8`: for change rights permission (most privileged)

Note that, at least either `name` or `permissionCode` must be provided. If one is missing, it will be extrapolated from the other.
For example, if `permissionCode= 1` is given but `name` was left empty, its value will be set to `name = RV`.

Similar to the previous case a custom IRI can be assigned to a permission specified by the `id` in the request body.
The example below shows the request body to create a new default object access permission with a custom IRI defined for
Expand Down Expand Up @@ -166,6 +214,11 @@ the combination of both, the permission will be defined for the newly specified
"hasPermissions":[{"additionalInformation":"http://www.knora.org/ontology/knora-admin#ProjectMember","name":"D","permissionCode":7}]
}
```
Each permission item given in `hasPermissions`, must contain the necessary parameters with respect to the type of the
permission. For example, if you wish to change the scope of an administrative permission, follow the
[guidelines](#creating-new-administrative-permissions) for the
content of its `hasPermissions` property. Similarly, if you wish to change the scope of a default object access permission,
follow the [guidelines](#creating-new-default-object-access-permissions) given about the content of its `hasPermissions` property.

### Updating a Default Object Access Permission's Resource Class:
- `PUT: /admin/permissions/<doap_permissionIri>/resourceClass` to change the resource class for which a default object
Expand Down
Expand Up @@ -20,13 +20,14 @@
package org.knora.webapi.messages.admin.responder.permissionsmessages

import java.util.UUID

import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport
import org.knora.webapi._
import org.knora.webapi.exceptions.{BadRequestException, ForbiddenException, InconsistentRepositoryDataException}
import org.knora.webapi.feature.FeatureFactoryConfig
import org.knora.webapi.messages.OntologyConstants.KnoraBase.EntityPermissionAbbreviations
import org.knora.webapi.messages.{OntologyConstants, StringFormatter}
import org.knora.webapi.messages.admin.responder.permissionsmessages.PermissionDataType.PermissionProfileType
import org.knora.webapi.messages.admin.responder.permissionsmessages.PermissionsMessagesUtilADM.PermissionTypeAndCodes
import org.knora.webapi.messages.admin.responder.projectsmessages.ProjectsADMJsonProtocol
import org.knora.webapi.messages.admin.responder.usersmessages.UserADM
import org.knora.webapi.messages.admin.responder.{KnoraRequestADM, KnoraResponseADM}
Expand Down Expand Up @@ -59,9 +60,16 @@ case class CreateAdministrativePermissionAPIRequestADM(id: Option[IRI] = None,
id,
throw BadRequestException(s"Invalid permission IRI ${id.get} is given."))
if (hasPermissions.isEmpty) throw BadRequestException("Permissions needs to be supplied.")

if (!OntologyConstants.KnoraAdmin.BuiltInGroups.contains(forGroup)) {
stringFormatter.validateGroupIri(forGroup, throw BadRequestException(s"Invalid group IRI $forGroup"))
}

def prepareHasPermissions: CreateAdministrativePermissionAPIRequestADM = {
copy(
hasPermissions = PermissionsMessagesUtilADM.verifyHasPermissionsAP(hasPermissions)
)
}
}

/**
Expand Down Expand Up @@ -123,8 +131,14 @@ case class CreateDefaultObjectAccessPermissionAPIRequestADM(id: Option[IRI] = No
}
case None => None
}

if (hasPermissions.isEmpty) throw BadRequestException("Permissions needs to be supplied.")

def prepareHasPermissions: CreateDefaultObjectAccessPermissionAPIRequestADM = {
copy(
hasPermissions = PermissionsMessagesUtilADM.verifyHasPermissionsDOAP(hasPermissions)
)
}
}

/**
Expand Down Expand Up @@ -155,6 +169,7 @@ case class ChangePermissionHasPermissionsApiRequestADM(hasPermissions: Set[Permi
}

def toJsValue: JsValue = changePermissionHasPermissionsApiRequestADMFormat.write(this)

}

/**
Expand Down Expand Up @@ -1138,6 +1153,7 @@ object PermissionADM {
permissionCode = Some(1)
)
}

}

/**
Expand Down
Expand Up @@ -20,7 +20,16 @@
package org.knora.webapi.messages.admin.responder.permissionsmessages

import org.knora.webapi.IRI
import org.knora.webapi.exceptions.ApplicationCacheException
import org.knora.webapi.exceptions.{ApplicationCacheException, BadRequestException}
import org.knora.webapi.messages.OntologyConstants.KnoraAdmin.AdministrativePermissionAbbreviations
import org.knora.webapi.messages.OntologyConstants.KnoraBase.{
ChangeRightsPermission,
DeletePermission,
EntityPermissionAbbreviations,
ModifyPermission,
RestrictedViewPermission,
ViewPermission
}
import org.knora.webapi.util.cache.CacheUtil

/**
Expand All @@ -30,6 +39,14 @@ object PermissionsMessagesUtilADM {

val PermissionsCacheName = "permissionsCache"

val PermissionTypeAndCodes: Map[String, Int] = Map(
RestrictedViewPermission -> 1,
ViewPermission -> 2,
ModifyPermission -> 6,
DeletePermission -> 7,
ChangeRightsPermission -> 8
)

////////////////////
// Helper Methods //
////////////////////
Expand Down Expand Up @@ -88,4 +105,92 @@ object PermissionsMessagesUtilADM {

CacheUtil.remove(PermissionsCacheName, key)
}

/**
* Validates the parameters of the `hasPermissions` collections of a DOAP.
*
* @param hasPermissions Set of the permissions.
*/
def validateDOAPHasPermissions(hasPermissions: Set[PermissionADM]) = {

hasPermissions.foreach { permission =>
if (permission.additionalInformation.isEmpty) {
throw BadRequestException(s"additionalInformation of a default object access permission type cannot be empty.")
}
if (permission.name.nonEmpty && !EntityPermissionAbbreviations.contains(permission.name))
throw BadRequestException(
s"Invalid value for name parameter of hasPermissions: ${permission.name}, it should be one of " +
s"${EntityPermissionAbbreviations.toString}")
if (permission.permissionCode.nonEmpty) {
val code = permission.permissionCode.get
if (!PermissionTypeAndCodes.values.toSet.contains(code)) {
throw BadRequestException(
s"Invalid value for permissionCode parameter of hasPermissions: $code, it should be one of " +
s"${PermissionTypeAndCodes.values.toString}")
}
}
if (permission.permissionCode.isEmpty && permission.name.isEmpty) {
throw BadRequestException(
s"One of permission code or permission name must be provided for a default object access permission.")
}
if (permission.permissionCode.nonEmpty && permission.name.nonEmpty) {
val code = permission.permissionCode.get
if (PermissionTypeAndCodes(permission.name) != code) {
throw BadRequestException(
s"Given permission code $code and permission name ${permission.name} are not consistent.")
}
}
}
}

/**
* For administrative permission we only need the name parameter of each PermissionADM given in hasPermissions collection.
* This method, validates the content of hasPermissions collection by only keeping the values of name params.
* @param hasPermissions Set of the permissions.
*/
def verifyHasPermissionsAP(hasPermissions: Set[PermissionADM]): Set[PermissionADM] = {
val updatedPermissions = hasPermissions.map { permission =>
if (!AdministrativePermissionAbbreviations.contains(permission.name))
throw BadRequestException(
s"Invalid value for name parameter of hasPermissions: ${permission.name}, it should be one of " +
s"${AdministrativePermissionAbbreviations.toString}")
PermissionADM(
name = permission.name,
additionalInformation = None,
permissionCode = None
)
}
updatedPermissions
}

/**
* For default object access permission, we need to make sure that the value given for the permissionCode matches
* the value of name parameter.
* This method, validates the content of hasPermissions collection by verifying that both permissionCode and name
* indicate the same type of permission.
*
* @param hasPermissions Set of the permissions.
*/
def verifyHasPermissionsDOAP(hasPermissions: Set[PermissionADM]): Set[PermissionADM] = {
validateDOAPHasPermissions(hasPermissions)
hasPermissions.map { permission =>
val code: Int = permission.permissionCode match {
case None => PermissionTypeAndCodes(permission.name)
case Some(code) => code
}
val name = permission.name.isEmpty match {
case true =>
val nameCodeSet: Option[(String, Int)] = PermissionTypeAndCodes.find {
case (name, code) => code == permission.permissionCode.get
}
nameCodeSet.get._1
case false => permission.name
}
PermissionADM(
name = name,
additionalInformation = permission.additionalInformation,
permissionCode = Some(code)
)
}
}
}
Expand Up @@ -82,7 +82,7 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
featureFactoryConfig,
requestingUser,
apiRequestID) =>
administrativePermissionCreateRequestADM(newAdministrativePermission,
administrativePermissionCreateRequestADM(newAdministrativePermission.prepareHasPermissions,
featureFactoryConfig,
requestingUser,
apiRequestID)
Expand Down Expand Up @@ -127,7 +127,10 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
featureFactoryConfig,
requestingUser,
apiRequestID) =>
defaultObjectAccessPermissionCreateRequestADM(createRequest, featureFactoryConfig, requestingUser, apiRequestID)
defaultObjectAccessPermissionCreateRequestADM(createRequest.prepareHasPermissions,
featureFactoryConfig,
requestingUser,
apiRequestID)
case PermissionsForProjectGetRequestADM(projectIri, groupIri, featureFactoryConfig, requestingUser) =>
permissionsForProjectGetRequestADM(projectIri, groupIri, featureFactoryConfig, requestingUser)
case PermissionByIriGetRequestADM(permissionIri, requestingUser) =>
Expand Down Expand Up @@ -1688,18 +1691,18 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
apiRequestID: UUID): Future[PermissionGetResponseADM] = {

/*Verify that hasPermissions is updated successfully*/
def verifyUpdateOfHasPermissions: Future[PermissionItemADM] =
def verifyUpdateOfHasPermissions(expectedPermissions: Set[PermissionADM]): Future[PermissionItemADM] =
for {
updatedPermission <- permissionGetADM(permissionIri, requestingUser)

/*Verify that update was successful*/
_ = updatedPermission match {
case ap: AdministrativePermissionADM =>
if (!ap.hasPermissions.equals(changeHasPermissionsRequest.hasPermissions))
if (!ap.hasPermissions.equals(expectedPermissions))
throw UpdateNotPerformedException(
s"The hasPermissions set of permission $permissionIri was not updated. Please report this as a bug.")
case doap: DefaultObjectAccessPermissionADM =>
if (!doap.hasPermissions.equals(changeHasPermissionsRequest.hasPermissions)) {
if (!doap.hasPermissions.equals(expectedPermissions)) {
throw UpdateNotPerformedException(
s"The hasPermissions set of permission $permissionIri was not updated. Please report this as a bug.")
}
Expand All @@ -1720,19 +1723,23 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
// Is permission an administrative permission?
case ap: AdministrativePermissionADM =>
// Yes.
val verifiedPermissions =
PermissionsMessagesUtilADM.verifyHasPermissionsAP(changeHasPermissionsRequest.hasPermissions)
for {
formattedPermissions <- Future(
PermissionUtilADM.formatPermissionADMs(changeHasPermissionsRequest.hasPermissions, PermissionType.AP))
PermissionUtilADM.formatPermissionADMs(verifiedPermissions, PermissionType.AP))
_ <- updatePermission(permissionIri = ap.iri, maybeHasPermissions = Some(formattedPermissions))
updatedPermission <- verifyUpdateOfHasPermissions
updatedPermission <- verifyUpdateOfHasPermissions(verifiedPermissions)
} yield AdministrativePermissionGetResponseADM(updatedPermission.asInstanceOf[AdministrativePermissionADM])
case doap: DefaultObjectAccessPermissionADM =>
//No. It is a default object access permission.
val verifiedPermissions =
PermissionsMessagesUtilADM.verifyHasPermissionsDOAP(changeHasPermissionsRequest.hasPermissions)
for {
formattedPermissions <- Future(
PermissionUtilADM.formatPermissionADMs(changeHasPermissionsRequest.hasPermissions, PermissionType.OAP))
PermissionUtilADM.formatPermissionADMs(verifiedPermissions, PermissionType.OAP))
_ <- updatePermission(permissionIri = doap.iri, maybeHasPermissions = Some(formattedPermissions))
updatedPermission <- verifyUpdateOfHasPermissions
updatedPermission <- verifyUpdateOfHasPermissions(verifiedPermissions)
} yield
DefaultObjectAccessPermissionGetResponseADM(
updatedPermission.asInstanceOf[DefaultObjectAccessPermissionADM])
Expand Down
Expand Up @@ -991,7 +991,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
forGroup = OntologyConstants.KnoraAdmin.ProjectAdmin,
hasPermissions =
Set(PermissionADM.ProjectAdminAllPermission, PermissionADM.ProjectResourceCreateAllPermission)
),
).prepareHasPermissions,
featureFactoryConfig = featureFactoryConfig,
requestingUser = requestingUser,
apiRequestID = UUID.randomUUID()
Expand All @@ -1003,7 +1003,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
forProject = projectIri,
forGroup = OntologyConstants.KnoraAdmin.ProjectMember,
hasPermissions = Set(PermissionADM.ProjectResourceCreateAllPermission)
),
).prepareHasPermissions,
featureFactoryConfig = featureFactoryConfig,
requestingUser = requestingUser,
apiRequestID = UUID.randomUUID()
Expand All @@ -1022,7 +1022,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
PermissionADM.viewPermission(OntologyConstants.KnoraAdmin.ProjectAdmin),
PermissionADM.restrictedViewPermission(OntologyConstants.KnoraAdmin.ProjectAdmin)
)
),
).prepareHasPermissions,
featureFactoryConfig = featureFactoryConfig,
requestingUser = requestingUser,
apiRequestID = UUID.randomUUID()
Expand All @@ -1039,7 +1039,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
PermissionADM.viewPermission(OntologyConstants.KnoraAdmin.ProjectMember),
PermissionADM.restrictedViewPermission(OntologyConstants.KnoraAdmin.ProjectMember)
)
),
).prepareHasPermissions,
featureFactoryConfig = featureFactoryConfig,
requestingUser = requestingUser,
apiRequestID = UUID.randomUUID()
Expand Down

0 comments on commit 3e3a3ce

Please sign in to comment.