Skip to content

Commit 3e3a3ce

Browse files
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
1 parent 83fb4b8 commit 3e3a3ce

7 files changed

Lines changed: 910 additions & 306 deletions

File tree

‎docs/03-apis/api-admin/permissions.md‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ for the group is returned.
3939
permissions for a project. As a response, all `default_object_acces_permissions` of a
4040
project are returned.
4141

42-
### Creating New Permissions:
42+
### Creating New Administrative Permissions:
4343

4444
- `POST: /admin/permissions/ap`: create a new administrative permission. The type of
4545
permissions, the project and group to which the permission should be added must be
@@ -79,11 +79,40 @@ As a response, the created administrative permission and its IRI are returned as
7979
}
8080
}
8181
```
82+
`hasPermissions` contains permission types that must be granted. See [the complete description of administrative
83+
permission types](../../05-internals/design/api-admin/administration.md#administrative-permissions).
84+
In summary, each permission should contain followings:
85+
- `name` : indicates the type of the permission that can be one of the followings:
86+
- `ProjectAdminAllPermission`: gives the user the permission to do anything
87+
on project level, i.e. create new groups, modify all
88+
existing groups
89+
- `ProjectAdminGroupAllPermission`: gives the user the permission to modify
90+
*group info* and *group membership* on *all* groups
91+
belonging to the project.
92+
- `ProjectAdminGroupRestrictedPermission`: gives the user the permission to modify
93+
*group info* and *group membership* on *certain* groups
94+
belonging to the project.
95+
- `ProjectAdminRightsAllPermission`: gives the user the permission to change the
96+
*permissions* on all objects belonging to the project
97+
(e.g., default permissions attached to groups and
98+
permissions on objects).
99+
- `ProjectResourceCreateAllPermission`: gives the permission to create resources
100+
inside the project.
101+
- `ProjectResourceCreateRestrictedPermission`: gives restricted resource creation permission
102+
inside the project.
103+
104+
- `additionalInformation`: should be left empty, otherwise will be ignored.
105+
- `permissionCode`: should be left empty, otherwise will be ignored.
106+
107+
82108
Note that during the creation of a new project, a default set of administrative permissions are added to its ProjectAdmin and
83109
ProjectMember groups (See [Default set of permissions for a new project](./projects.md#default-set-of-permissions-for-a-new-project)).
84110
Therefore, it is not possible to create new administrative permissions for the ProjectAdmin and ProjectMember groups of
85111
a project. However, the default permissions set for these groups can be modified (See [update permission](./permissions.md#updating-a-permissions-scope)).
86112

113+
114+
### Creating New Default Object Access Permissions:
115+
87116
- `POST: /admin/permissions/doap` : create a new default object access permission.
88117
A single instance of `knora-admin:DefaultObjectAccessPermission` must
89118
always reference a project, but can only reference **either** a group
@@ -101,6 +130,25 @@ default object access permission for a group of a project the request body would
101130
"hasPermissions":[{"additionalInformation":"http://www.knora.org/ontology/knora-admin#ProjectMember","name":"D","permissionCode":7}]
102131
}
103132
```
133+
`hasPermissions` contains permission types that must be granted. See [a complete description of object access
134+
permission types](../../05-internals/design/api-admin/administration.md#default-object-access-permissions).
135+
In summary, each permission should contain followings:
136+
- `additionalInformation`: To whom the permission should be granted: project members, known users, unknown users, etc.
137+
- `name` : indicates the type of the permission that can be one of the followings.
138+
- `RV`: restricted view permission (least privileged)
139+
- `V`: view permission
140+
- `M` modify permission
141+
- `D`: delete permission
142+
- `CR`: change rights permission (most privileged)
143+
- `permissionCode`: The code assigned to a permission indicating its hierarchical level. These codes are as below:
144+
- `1`: for restricted view permission (least privileged)
145+
- `2`: for view permission
146+
- `6`: for modify permission
147+
- `7`: for delete permission
148+
- `8`: for change rights permission (most privileged)
149+
150+
Note that, at least either `name` or `permissionCode` must be provided. If one is missing, it will be extrapolated from the other.
151+
For example, if `permissionCode= 1` is given but `name` was left empty, its value will be set to `name = RV`.
104152

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

170223
### Updating a Default Object Access Permission's Resource Class:
171224
- `PUT: /admin/permissions/<doap_permissionIri>/resourceClass` to change the resource class for which a default object

‎webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
package org.knora.webapi.messages.admin.responder.permissionsmessages
2121

2222
import java.util.UUID
23-
2423
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport
2524
import org.knora.webapi._
2625
import org.knora.webapi.exceptions.{BadRequestException, ForbiddenException, InconsistentRepositoryDataException}
2726
import org.knora.webapi.feature.FeatureFactoryConfig
27+
import org.knora.webapi.messages.OntologyConstants.KnoraBase.EntityPermissionAbbreviations
2828
import org.knora.webapi.messages.{OntologyConstants, StringFormatter}
2929
import org.knora.webapi.messages.admin.responder.permissionsmessages.PermissionDataType.PermissionProfileType
30+
import org.knora.webapi.messages.admin.responder.permissionsmessages.PermissionsMessagesUtilADM.PermissionTypeAndCodes
3031
import org.knora.webapi.messages.admin.responder.projectsmessages.ProjectsADMJsonProtocol
3132
import org.knora.webapi.messages.admin.responder.usersmessages.UserADM
3233
import org.knora.webapi.messages.admin.responder.{KnoraRequestADM, KnoraResponseADM}
@@ -59,9 +60,16 @@ case class CreateAdministrativePermissionAPIRequestADM(id: Option[IRI] = None,
5960
id,
6061
throw BadRequestException(s"Invalid permission IRI ${id.get} is given."))
6162
if (hasPermissions.isEmpty) throw BadRequestException("Permissions needs to be supplied.")
63+
6264
if (!OntologyConstants.KnoraAdmin.BuiltInGroups.contains(forGroup)) {
6365
stringFormatter.validateGroupIri(forGroup, throw BadRequestException(s"Invalid group IRI $forGroup"))
6466
}
67+
68+
def prepareHasPermissions: CreateAdministrativePermissionAPIRequestADM = {
69+
copy(
70+
hasPermissions = PermissionsMessagesUtilADM.verifyHasPermissionsAP(hasPermissions)
71+
)
72+
}
6573
}
6674

6775
/**
@@ -123,8 +131,14 @@ case class CreateDefaultObjectAccessPermissionAPIRequestADM(id: Option[IRI] = No
123131
}
124132
case None => None
125133
}
134+
126135
if (hasPermissions.isEmpty) throw BadRequestException("Permissions needs to be supplied.")
127136

137+
def prepareHasPermissions: CreateDefaultObjectAccessPermissionAPIRequestADM = {
138+
copy(
139+
hasPermissions = PermissionsMessagesUtilADM.verifyHasPermissionsDOAP(hasPermissions)
140+
)
141+
}
128142
}
129143

130144
/**
@@ -155,6 +169,7 @@ case class ChangePermissionHasPermissionsApiRequestADM(hasPermissions: Set[Permi
155169
}
156170

157171
def toJsValue: JsValue = changePermissionHasPermissionsApiRequestADMFormat.write(this)
172+
158173
}
159174

160175
/**
@@ -1138,6 +1153,7 @@ object PermissionADM {
11381153
permissionCode = Some(1)
11391154
)
11401155
}
1156+
11411157
}
11421158

11431159
/**

‎webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesUtilADM.scala‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,16 @@
2020
package org.knora.webapi.messages.admin.responder.permissionsmessages
2121

2222
import org.knora.webapi.IRI
23-
import org.knora.webapi.exceptions.ApplicationCacheException
23+
import org.knora.webapi.exceptions.{ApplicationCacheException, BadRequestException}
24+
import org.knora.webapi.messages.OntologyConstants.KnoraAdmin.AdministrativePermissionAbbreviations
25+
import org.knora.webapi.messages.OntologyConstants.KnoraBase.{
26+
ChangeRightsPermission,
27+
DeletePermission,
28+
EntityPermissionAbbreviations,
29+
ModifyPermission,
30+
RestrictedViewPermission,
31+
ViewPermission
32+
}
2433
import org.knora.webapi.util.cache.CacheUtil
2534

2635
/**
@@ -30,6 +39,14 @@ object PermissionsMessagesUtilADM {
3039

3140
val PermissionsCacheName = "permissionsCache"
3241

42+
val PermissionTypeAndCodes: Map[String, Int] = Map(
43+
RestrictedViewPermission -> 1,
44+
ViewPermission -> 2,
45+
ModifyPermission -> 6,
46+
DeletePermission -> 7,
47+
ChangeRightsPermission -> 8
48+
)
49+
3350
////////////////////
3451
// Helper Methods //
3552
////////////////////
@@ -88,4 +105,92 @@ object PermissionsMessagesUtilADM {
88105

89106
CacheUtil.remove(PermissionsCacheName, key)
90107
}
108+
109+
/**
110+
* Validates the parameters of the `hasPermissions` collections of a DOAP.
111+
*
112+
* @param hasPermissions Set of the permissions.
113+
*/
114+
def validateDOAPHasPermissions(hasPermissions: Set[PermissionADM]) = {
115+
116+
hasPermissions.foreach { permission =>
117+
if (permission.additionalInformation.isEmpty) {
118+
throw BadRequestException(s"additionalInformation of a default object access permission type cannot be empty.")
119+
}
120+
if (permission.name.nonEmpty && !EntityPermissionAbbreviations.contains(permission.name))
121+
throw BadRequestException(
122+
s"Invalid value for name parameter of hasPermissions: ${permission.name}, it should be one of " +
123+
s"${EntityPermissionAbbreviations.toString}")
124+
if (permission.permissionCode.nonEmpty) {
125+
val code = permission.permissionCode.get
126+
if (!PermissionTypeAndCodes.values.toSet.contains(code)) {
127+
throw BadRequestException(
128+
s"Invalid value for permissionCode parameter of hasPermissions: $code, it should be one of " +
129+
s"${PermissionTypeAndCodes.values.toString}")
130+
}
131+
}
132+
if (permission.permissionCode.isEmpty && permission.name.isEmpty) {
133+
throw BadRequestException(
134+
s"One of permission code or permission name must be provided for a default object access permission.")
135+
}
136+
if (permission.permissionCode.nonEmpty && permission.name.nonEmpty) {
137+
val code = permission.permissionCode.get
138+
if (PermissionTypeAndCodes(permission.name) != code) {
139+
throw BadRequestException(
140+
s"Given permission code $code and permission name ${permission.name} are not consistent.")
141+
}
142+
}
143+
}
144+
}
145+
146+
/**
147+
* For administrative permission we only need the name parameter of each PermissionADM given in hasPermissions collection.
148+
* This method, validates the content of hasPermissions collection by only keeping the values of name params.
149+
* @param hasPermissions Set of the permissions.
150+
*/
151+
def verifyHasPermissionsAP(hasPermissions: Set[PermissionADM]): Set[PermissionADM] = {
152+
val updatedPermissions = hasPermissions.map { permission =>
153+
if (!AdministrativePermissionAbbreviations.contains(permission.name))
154+
throw BadRequestException(
155+
s"Invalid value for name parameter of hasPermissions: ${permission.name}, it should be one of " +
156+
s"${AdministrativePermissionAbbreviations.toString}")
157+
PermissionADM(
158+
name = permission.name,
159+
additionalInformation = None,
160+
permissionCode = None
161+
)
162+
}
163+
updatedPermissions
164+
}
165+
166+
/**
167+
* For default object access permission, we need to make sure that the value given for the permissionCode matches
168+
* the value of name parameter.
169+
* This method, validates the content of hasPermissions collection by verifying that both permissionCode and name
170+
* indicate the same type of permission.
171+
*
172+
* @param hasPermissions Set of the permissions.
173+
*/
174+
def verifyHasPermissionsDOAP(hasPermissions: Set[PermissionADM]): Set[PermissionADM] = {
175+
validateDOAPHasPermissions(hasPermissions)
176+
hasPermissions.map { permission =>
177+
val code: Int = permission.permissionCode match {
178+
case None => PermissionTypeAndCodes(permission.name)
179+
case Some(code) => code
180+
}
181+
val name = permission.name.isEmpty match {
182+
case true =>
183+
val nameCodeSet: Option[(String, Int)] = PermissionTypeAndCodes.find {
184+
case (name, code) => code == permission.permissionCode.get
185+
}
186+
nameCodeSet.get._1
187+
case false => permission.name
188+
}
189+
PermissionADM(
190+
name = name,
191+
additionalInformation = permission.additionalInformation,
192+
permissionCode = Some(code)
193+
)
194+
}
195+
}
91196
}

‎webapi/src/main/scala/org/knora/webapi/responders/admin/PermissionsResponderADM.scala‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
8282
featureFactoryConfig,
8383
requestingUser,
8484
apiRequestID) =>
85-
administrativePermissionCreateRequestADM(newAdministrativePermission,
85+
administrativePermissionCreateRequestADM(newAdministrativePermission.prepareHasPermissions,
8686
featureFactoryConfig,
8787
requestingUser,
8888
apiRequestID)
@@ -127,7 +127,10 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
127127
featureFactoryConfig,
128128
requestingUser,
129129
apiRequestID) =>
130-
defaultObjectAccessPermissionCreateRequestADM(createRequest, featureFactoryConfig, requestingUser, apiRequestID)
130+
defaultObjectAccessPermissionCreateRequestADM(createRequest.prepareHasPermissions,
131+
featureFactoryConfig,
132+
requestingUser,
133+
apiRequestID)
131134
case PermissionsForProjectGetRequestADM(projectIri, groupIri, featureFactoryConfig, requestingUser) =>
132135
permissionsForProjectGetRequestADM(projectIri, groupIri, featureFactoryConfig, requestingUser)
133136
case PermissionByIriGetRequestADM(permissionIri, requestingUser) =>
@@ -1688,18 +1691,18 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
16881691
apiRequestID: UUID): Future[PermissionGetResponseADM] = {
16891692

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

16951698
/*Verify that update was successful*/
16961699
_ = updatedPermission match {
16971700
case ap: AdministrativePermissionADM =>
1698-
if (!ap.hasPermissions.equals(changeHasPermissionsRequest.hasPermissions))
1701+
if (!ap.hasPermissions.equals(expectedPermissions))
16991702
throw UpdateNotPerformedException(
17001703
s"The hasPermissions set of permission $permissionIri was not updated. Please report this as a bug.")
17011704
case doap: DefaultObjectAccessPermissionADM =>
1702-
if (!doap.hasPermissions.equals(changeHasPermissionsRequest.hasPermissions)) {
1705+
if (!doap.hasPermissions.equals(expectedPermissions)) {
17031706
throw UpdateNotPerformedException(
17041707
s"The hasPermissions set of permission $permissionIri was not updated. Please report this as a bug.")
17051708
}
@@ -1720,19 +1723,23 @@ class PermissionsResponderADM(responderData: ResponderData) extends Responder(re
17201723
// Is permission an administrative permission?
17211724
case ap: AdministrativePermissionADM =>
17221725
// Yes.
1726+
val verifiedPermissions =
1727+
PermissionsMessagesUtilADM.verifyHasPermissionsAP(changeHasPermissionsRequest.hasPermissions)
17231728
for {
17241729
formattedPermissions <- Future(
1725-
PermissionUtilADM.formatPermissionADMs(changeHasPermissionsRequest.hasPermissions, PermissionType.AP))
1730+
PermissionUtilADM.formatPermissionADMs(verifiedPermissions, PermissionType.AP))
17261731
_ <- updatePermission(permissionIri = ap.iri, maybeHasPermissions = Some(formattedPermissions))
1727-
updatedPermission <- verifyUpdateOfHasPermissions
1732+
updatedPermission <- verifyUpdateOfHasPermissions(verifiedPermissions)
17281733
} yield AdministrativePermissionGetResponseADM(updatedPermission.asInstanceOf[AdministrativePermissionADM])
17291734
case doap: DefaultObjectAccessPermissionADM =>
17301735
//No. It is a default object access permission.
1736+
val verifiedPermissions =
1737+
PermissionsMessagesUtilADM.verifyHasPermissionsDOAP(changeHasPermissionsRequest.hasPermissions)
17311738
for {
17321739
formattedPermissions <- Future(
1733-
PermissionUtilADM.formatPermissionADMs(changeHasPermissionsRequest.hasPermissions, PermissionType.OAP))
1740+
PermissionUtilADM.formatPermissionADMs(verifiedPermissions, PermissionType.OAP))
17341741
_ <- updatePermission(permissionIri = doap.iri, maybeHasPermissions = Some(formattedPermissions))
1735-
updatedPermission <- verifyUpdateOfHasPermissions
1742+
updatedPermission <- verifyUpdateOfHasPermissions(verifiedPermissions)
17361743
} yield
17371744
DefaultObjectAccessPermissionGetResponseADM(
17381745
updatedPermission.asInstanceOf[DefaultObjectAccessPermissionADM])

‎webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -991,7 +991,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
991991
forGroup = OntologyConstants.KnoraAdmin.ProjectAdmin,
992992
hasPermissions =
993993
Set(PermissionADM.ProjectAdminAllPermission, PermissionADM.ProjectResourceCreateAllPermission)
994-
),
994+
).prepareHasPermissions,
995995
featureFactoryConfig = featureFactoryConfig,
996996
requestingUser = requestingUser,
997997
apiRequestID = UUID.randomUUID()
@@ -1003,7 +1003,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
10031003
forProject = projectIri,
10041004
forGroup = OntologyConstants.KnoraAdmin.ProjectMember,
10051005
hasPermissions = Set(PermissionADM.ProjectResourceCreateAllPermission)
1006-
),
1006+
).prepareHasPermissions,
10071007
featureFactoryConfig = featureFactoryConfig,
10081008
requestingUser = requestingUser,
10091009
apiRequestID = UUID.randomUUID()
@@ -1022,7 +1022,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
10221022
PermissionADM.viewPermission(OntologyConstants.KnoraAdmin.ProjectAdmin),
10231023
PermissionADM.restrictedViewPermission(OntologyConstants.KnoraAdmin.ProjectAdmin)
10241024
)
1025-
),
1025+
).prepareHasPermissions,
10261026
featureFactoryConfig = featureFactoryConfig,
10271027
requestingUser = requestingUser,
10281028
apiRequestID = UUID.randomUUID()
@@ -1039,7 +1039,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo
10391039
PermissionADM.viewPermission(OntologyConstants.KnoraAdmin.ProjectMember),
10401040
PermissionADM.restrictedViewPermission(OntologyConstants.KnoraAdmin.ProjectMember)
10411041
)
1042-
),
1042+
).prepareHasPermissions,
10431043
featureFactoryConfig = featureFactoryConfig,
10441044
requestingUser = requestingUser,
10451045
apiRequestID = UUID.randomUUID()

0 commit comments

Comments
 (0)