From b3c2d5f82072d507d8cc5c2ab5df76c40d3de22d Mon Sep 17 00:00:00 2001 From: Sepideh Alassi Date: Wed, 30 Jun 2021 11:36:39 +0200 Subject: [PATCH] feat(projects)!: Change shortname to xsd:NCName forma, Escape special character in payloads of projects endpoints (DSP-1555 ) (#1886) * fix (projects): validate and escape project parameters * fix (projects): escape special characters before putting in triplestore, and unescape again before returning the response * docs (projects): update docs * fix (projects): fix the failing tests * fix (projects): fix the failing test in v1 --- docs/03-apis/api-admin/projects.md | 29 ++-- .../webapi/messages/StringFormatter.scala | 53 +++----- .../PermissionsMessagesADM.scala | 20 +-- .../ProjectsMessagesADM.scala | 128 +++++++++++++++++- .../admin/ProjectsResponderADM.scala | 61 ++++----- .../routing/admin/ProjectsRouteADM.scala | 4 +- .../webapi/messages/StringFormatterSpec.scala | 30 +++- .../PermissionsMessagesADMSpec.scala | 45 +++--- .../ProjectsMessagesADMSpec.scala | 36 ++++- .../admin/ProjectsResponderADMSpec.scala | 77 +++++------ 10 files changed, 319 insertions(+), 164 deletions(-) diff --git a/docs/03-apis/api-admin/projects.md b/docs/03-apis/api-admin/projects.md index fb1e533c4c..11113bc24a 100644 --- a/docs/03-apis/api-admin/projects.md +++ b/docs/03-apis/api-admin/projects.md @@ -58,18 +58,23 @@ License along with DSP. If not, see . ### Create a new project: - Required permission: SystemAdmin - - Required information: shortname (unique; used for named graphs), - status, selfjoin - - Optional information: longname, description, keywords, logo + - Required information: + - shortcode (unique, 4-digits) + - shortname (unique; it should be in the form of a + [xsd:NCNAME](https://www.w3.org/TR/xmlschema11-2/#NCName)) + - description (collection of descriptions as strings with language tag.) + - keywords (collection of keywords) + - status (true, if project is active. false, if project is inactive) + - selfjoin + - Optional information: longname, logo - Returns information about the newly created project - Remark: There are two distinct use cases / payload combination: (1) change ontology and data graph: ontologygraph, datagraph, - (2) basic project information: shortname, longname, description, + (2) basic project information: shortcode, shortname, longname, description, keywords, logo, institution, status, selfjoin - - TypeScript Docs: projectFormats - CreateProjectApiRequestV1 - POST: `/admin/projects/` - BODY: @@ -77,15 +82,16 @@ License along with DSP. If not, see . { "shortname": "newproject", "longname": "project longname", - "description": "project description", - "keywords": "keywords", + "description": [{"value": "project description", "language": "en"}], + "keywords": ["test project"], "logo": "/fu/bar/baz.jpg", "status": true, "selfjoin": false } ``` -Additionally, each project can have an optional custom IRI (of [Knora IRI](../api-v2/knora-iris.md#iris-for-data) form) specified by the `id` in the request body as below: +Additionally, each project can have an optional custom IRI (of [Knora IRI](../api-v2/knora-iris.md#iris-for-data) form) +specified by the `id` in the request body as below: ```json { @@ -100,6 +106,7 @@ Additionally, each project can have an optional custom IRI (of [Knora IRI](../ap "selfjoin": false } ``` + #### Default set of permissions for a new project: When a new project is created, following default permissions are added to its admins and members: - ProjectAdmin group receives an administrative permission to do all project level operations and to create resources @@ -123,7 +130,7 @@ belongs to the project. This default object access permission is retrievable thr - Required permission: SystemAdmin / ProjectAdmin - Changeable information: shortname, longname, description, - keywords, logo, status, selfjoin + keywords, logo, status, selfjoin. The payload must at least contain a new value for one of these properties. - TypeScript Docs: projectFormats - ChangeProjectApiRequestV1 - PUT: `/admin/projects/iri/` - BODY: @@ -132,8 +139,8 @@ belongs to the project. This default object access permission is retrievable thr { "shortname": "newproject", "longname": "project longname", - "description": "project description", - "keywords": "keywords", + "description": [{"value": "a new description", "language": "en"}], + "keywords": ["a new key"], "logo": "/fu/bar/baz.jpg", "status": true, "selfjoin": false diff --git a/webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala b/webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala index be2b5e38b2..57ab202182 100644 --- a/webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala +++ b/webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala @@ -885,14 +885,6 @@ class StringFormatter private (val maybeSettings: Option[KnoraSettingsImpl] = No private val UsernameRegex: Regex = """^(?=.{4,50}$)(?![_.])(?!.*[_.]{2})[a-zA-Z0-9._]+(? Nothing): IRI = { - if (isKnoraProjectIriStr(iri)) { - iri - } else { - errorFun - } - } - - /** - * Given the optional project IRI, checks if it is in a valid format. - * - * @param maybeIri the optional project's IRI to be checked. - * @return the same optional IRI. - */ - def validateOptionalProjectIri(maybeIri: Option[IRI], errorFun: => Nothing): Option[IRI] = { - maybeIri match { - case Some(iri) => Some(validateProjectIri(iri, errorFun)) - case None => None - } - } - /** * Check that the supplied IRI represents a valid project IRI. * @@ -2501,7 +2466,7 @@ class StringFormatter private (val maybeSettings: Option[KnoraSettingsImpl] = No * project IRI. * @return the same optional string but escaped. */ - def validateAndEscapeOptionalProjectIri(maybeString: Option[String], errorFun: => Nothing): Option[String] = { + def validateAndEscapeOptionalProjectIri(maybeString: Option[String], errorFun: => Nothing): Option[IRI] = { maybeString match { case Some(s) => Some(validateAndEscapeProjectIri(s, errorFun)) case None => None @@ -2517,7 +2482,7 @@ class StringFormatter private (val maybeSettings: Option[KnoraSettingsImpl] = No * @return the same string. */ def validateAndEscapeProjectShortname(value: String, errorFun: => Nothing): String = { - ProjectShortnameRegex.findFirstIn(value) match { + NCNameRegex.findFirstIn(value) match { case Some(shortname) => toSparqlEncodedString(shortname, errorFun) case None => errorFun } @@ -2594,6 +2559,14 @@ class StringFormatter private (val maybeSettings: Option[KnoraSettingsImpl] = No } } + def escapeOptionalString(maybeString: Option[String], errorFun: => Nothing): Option[String] = { + maybeString match { + case Some(s) => + Some(toSparqlEncodedString(s, errorFun)) + case None => None + } + } + /** * Given the list IRI, checks if it is in a valid format. * @@ -3263,4 +3236,10 @@ class StringFormatter private (val maybeSettings: Option[KnoraSettingsImpl] = No StringLiteralV2(value = fromSparqlEncodedString(stringLiteral.value), language = stringLiteral.language)) ) } + def unescapeOptionalString(optionalString: Option[String]): Option[String] = { + optionalString match { + case Some(s: String) => Some(fromSparqlEncodedString(s)) + case None => None + } + } } diff --git a/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala b/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala index 877c3a36c9..41cb620c28 100644 --- a/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala @@ -54,13 +54,13 @@ case class CreateAdministrativePermissionAPIRequestADM(id: Option[IRI] = None, def toJsValue: JsValue = createAdministrativePermissionAPIRequestADMFormat.write(this) implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(forProject, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(forProject, throw BadRequestException(s"Invalid project IRI $forProject")) stringFormatter.validateOptionalPermissionIri( 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}")) + stringFormatter.validateGroupIri(forGroup, throw BadRequestException(s"Invalid group IRI $forGroup")) } } @@ -84,7 +84,7 @@ case class CreateDefaultObjectAccessPermissionAPIRequestADM(id: Option[IRI] = No def toJsValue: JsValue = createDefaultObjectAccessPermissionAPIRequestADMFormat.write(this) implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(forProject, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(forProject, throw BadRequestException(s"Invalid project IRI $forProject")) stringFormatter.validateOptionalPermissionIri( id, throw BadRequestException(s"Invalid permission IRI ${id.get} is given.")) @@ -231,7 +231,7 @@ case class PermissionsForProjectGetRequestADM(projectIri: IRI, extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -345,7 +345,7 @@ case class AdministrativePermissionsForProjectGetRequestADM(projectIri: IRI, extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -386,7 +386,7 @@ case class AdministrativePermissionForProjectGroupGetADM(projectIri: IRI, groupI extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -488,7 +488,7 @@ case class DefaultObjectAccessPermissionsForProjectGetRequestADM(projectIri: IRI extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -516,7 +516,7 @@ case class DefaultObjectAccessPermissionGetRequestADM(projectIri: IRI, extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -592,7 +592,7 @@ case class DefaultObjectAccessPermissionsStringForResourceClassGetADM(projectIri extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin @@ -628,7 +628,7 @@ case class DefaultObjectAccessPermissionsStringForPropertyGetADM(projectIri: IRI extends PermissionsResponderRequestADM { implicit protected val stringFormatter: StringFormatter = StringFormatter.getInstanceForConstantOntologies - stringFormatter.validateProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI")) + stringFormatter.validateAndEscapeProjectIri(projectIri, throw BadRequestException(s"Invalid project IRI $projectIri")) // Check user's permission for the operation if (!requestingUser.isSystemAdmin diff --git a/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADM.scala b/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADM.scala index 16001f1016..d5e81296f8 100644 --- a/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADM.scala @@ -63,11 +63,59 @@ case class CreateProjectApiRequestADM(id: Option[IRI] = None, extends ProjectsADMJsonProtocol { implicit protected val stringFormatter: StringFormatter = StringFormatter.getGeneralInstance + // Check that collection parameters are not empty. + if (description.isEmpty) throw BadRequestException("Project description needs to be supplied.") + if (keywords.isEmpty) throw BadRequestException("At least one keyword must be supplied for a new project.") + if (shortcode.isEmpty) throw BadRequestException("Project shortcode must be supplied.") + if (shortname.isEmpty) throw BadRequestException("Project shortname must be supplied.") + + /* Convert to Json */ def toJsValue: JsValue = createProjectApiRequestADMFormat.write(this) - if (description.isEmpty) throw BadRequestException("Project description needs to be supplied.") + /* validates and escapes the given values.*/ + def validateAndEscape: CreateProjectApiRequestADM = { + val maybeProjectIri = + stringFormatter.validateAndEscapeOptionalProjectIri(id, throw BadRequestException(s"Invalid project IRI")) + val validatedShortcode = stringFormatter.validateAndEscapeProjectShortcode( + shortcode, + errorFun = throw BadRequestException(s"The supplied short code: '$shortcode' is not valid.")) + + val validatedShortname = stringFormatter.validateAndEscapeProjectShortname( + shortname, + errorFun = throw BadRequestException(s"The supplied short name: '$shortname' is not valid.")) + + val validatedLongName = stringFormatter.escapeOptionalString( + longname, + errorFun = throw BadRequestException(s"The supplied longname: '$longname' is not valid.")) + + val validatedLogo = stringFormatter.escapeOptionalString( + logo, + errorFun = throw BadRequestException(s"The supplied logo: '$logo' is not valid.")) + + val validatedDescriptions: Seq[StringLiteralV2] = description.map { des => + val escapedValue = + stringFormatter.toSparqlEncodedString( + des.value, + errorFun = throw BadRequestException(s"The supplied description: '${des.value}' is not valid.")) + StringLiteralV2(value = escapedValue, language = des.language) + } + + val validatedKeywords = keywords.map( + keyword => + stringFormatter.toSparqlEncodedString( + keyword, + errorFun = throw BadRequestException(s"The supplied keyword: '$keyword' is not valid."))) + copy( + id = maybeProjectIri, + shortcode = validatedShortcode, + shortname = validatedShortname, + longname = validatedLongName, + description = validatedDescriptions, + keywords = validatedKeywords, + logo = validatedLogo + ) + } - stringFormatter.validateOptionalProjectIri(id, throw BadRequestException(s"Invalid project IRI")) } /** @@ -89,6 +137,7 @@ case class ChangeProjectApiRequestADM(shortname: Option[String] = None, status: Option[Boolean] = None, selfjoin: Option[Boolean] = None) extends ProjectsADMJsonProtocol { + implicit protected val stringFormatter: StringFormatter = StringFormatter.getGeneralInstance val parametersCount: Int = List( shortname, @@ -104,6 +153,53 @@ case class ChangeProjectApiRequestADM(shortname: Option[String] = None, if (parametersCount == 0) throw BadRequestException("No data sent in API request.") def toJsValue: JsValue = changeProjectApiRequestADMFormat.write(this) + + /* validates and escapes the given values.*/ + def validateAndEscape: ChangeProjectApiRequestADM = { + + val validatedShortname: Option[String] = stringFormatter.validateAndEscapeOptionalProjectShortname( + shortname, + errorFun = throw BadRequestException(s"The supplied short name: '$shortname' is not valid.")) + + val validatedLongName: Option[String] = stringFormatter.escapeOptionalString( + longname, + errorFun = throw BadRequestException(s"The supplied longname: '$longname' is not valid.")) + + val validatedLogo: Option[String] = stringFormatter.escapeOptionalString( + logo, + errorFun = throw BadRequestException(s"The supplied logo: '$logo' is not valid.")) + + val validatedDescriptions: Option[Seq[StringLiteralV2]] = description match { + case Some(descriptions: Seq[StringLiteralV2]) => + val escapedDescriptions = descriptions.map { des => + val escapedValue = + stringFormatter.toSparqlEncodedString( + des.value, + errorFun = throw BadRequestException(s"The supplied description: '${des.value}' is not valid.")) + StringLiteralV2(value = escapedValue, language = des.language) + } + Some(escapedDescriptions) + case None => None + } + + val validatedKeywords: Option[Seq[String]] = keywords match { + case Some(givenKeywords: Seq[String]) => + val escapedKeywords = givenKeywords.map( + keyword => + stringFormatter.toSparqlEncodedString( + keyword, + errorFun = throw BadRequestException(s"The supplied keyword: '$keyword' is not valid."))) + Some(escapedKeywords) + case None => None + } + copy( + shortname = validatedShortname, + longname = validatedLongName, + description = validatedDescriptions, + keywords = validatedKeywords, + logo = validatedLogo + ) + } } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -464,6 +560,21 @@ case class ProjectADM(id: IRI, .append(selfjoin) .toHashCode } + + def unescape: ProjectADM = { + val stringFormatter: StringFormatter = StringFormatter.getGeneralInstance + val unescapedDescriptions: Seq[StringLiteralV2] = description.map(desc => + StringLiteralV2(value = stringFormatter.fromSparqlEncodedString(desc.value), language = desc.language)) + val unescapedKeywords: Seq[String] = keywords.map(key => stringFormatter.fromSparqlEncodedString(key)) + copy( + shortcode = stringFormatter.fromSparqlEncodedString(shortcode), + shortname = stringFormatter.fromSparqlEncodedString(shortname), + longname = stringFormatter.unescapeOptionalString(longname), + logo = stringFormatter.unescapeOptionalString(logo), + description = unescapedDescriptions, + keywords = unescapedKeywords + ) + } } /** @@ -636,7 +747,18 @@ trait ProjectsADMJsonProtocol extends SprayJsonSupport with DefaultJsonProtocol import org.knora.webapi.messages.admin.responder.usersmessages.UsersADMJsonProtocol._ - implicit val projectADMFormat: JsonFormat[ProjectADM] = lazyFormat(jsonFormat10(ProjectADM)) + implicit val projectADMFormat: JsonFormat[ProjectADM] = lazyFormat( + jsonFormat(ProjectADM, + "id", + "shortname", + "shortcode", + "longname", + "description", + "keywords", + "logo", + "ontologies", + "status", + "selfjoin")) implicit val projectsResponseADMFormat: RootJsonFormat[ProjectsGetResponseADM] = rootFormat( lazyFormat(jsonFormat(ProjectsGetResponseADM, "projects"))) implicit val projectResponseADMFormat: RootJsonFormat[ProjectGetResponseADM] = rootFormat( diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala index 920eb56c46..eca746d819 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala @@ -891,7 +891,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo // _ = log.debug(s"updateProjectADM - update query: {}", updateProjectSparqlString) - updateProjectResponse <- (storeManager ? SparqlUpdateRequest(updateProjectSparqlString)) + _ <- (storeManager ? SparqlUpdateRequest(updateProjectSparqlString)) .mapTo[SparqlUpdateResponse] /* Verify that the project was updated. */ @@ -910,34 +910,37 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo updatedProject) _ = if (projectUpdatePayload.shortname.isDefined) { - if (updatedProject.shortname != projectUpdatePayload.shortname.get) + val unescapedShortName: String = stringFormatter.fromSparqlEncodedString(projectUpdatePayload.shortname.get) + if (updatedProject.shortname != unescapedShortName) throw UpdateNotPerformedException( "Project's 'shortname' was not updated. Please report this as a possible bug.") } - _ = if (projectUpdatePayload.longname.isDefined) { - if (updatedProject.longname != projectUpdatePayload.longname) + val unescapedLongname: Option[String] = stringFormatter.unescapeOptionalString(projectUpdatePayload.longname) + if (updatedProject.longname != unescapedLongname) throw UpdateNotPerformedException( - "Project's 'longname' was not updated. Please report this as a possible bug.") + s"Project's 'longname' was not updated. Please report this as a possible bug.") } - _ = if (projectUpdatePayload.description.isDefined) { - if (updatedProject.description.diff(projectUpdatePayload.description.get).nonEmpty) + val unescapedDescriptions: Seq[StringLiteralV2] = projectUpdatePayload.description.get.map(desc => + StringLiteralV2(stringFormatter.fromSparqlEncodedString(desc.value), desc.language)) + if (updatedProject.description.diff(unescapedDescriptions).nonEmpty) throw UpdateNotPerformedException( "Project's 'description' was not updated. Please report this as a possible bug.") } _ = if (projectUpdatePayload.keywords.isDefined) { - if (updatedProject.keywords.sorted != projectUpdatePayload.keywords.get.sorted) + val unescapedKeywords: Seq[String] = + projectUpdatePayload.keywords.get.map(key => stringFormatter.fromSparqlEncodedString(key)) + if (updatedProject.keywords.sorted != unescapedKeywords.sorted) throw UpdateNotPerformedException( "Project's 'keywords' was not updated. Please report this as a possible bug.") } - _ = if (projectUpdatePayload.logo.isDefined) { - if (updatedProject.logo != projectUpdatePayload.logo) + val unescapedLogo: Option[String] = stringFormatter.unescapeOptionalString(projectUpdatePayload.logo) + if (updatedProject.logo != unescapedLogo) throw UpdateNotPerformedException("Project's 'logo' was not updated. Please report this as a possible bug.") } - _ = if (projectUpdatePayload.status.isDefined) { if (updatedProject.status != projectUpdatePayload.status.get) throw UpdateNotPerformedException("Project's 'status' was not updated. Please report this as a possible bug.") @@ -1046,15 +1049,6 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo def projectCreateTask(createProjectRequest: CreateProjectApiRequestADM, requestingUser: UserADM): Future[ProjectOperationResponseADM] = for { - // check if required properties are not empty - _ <- Future( - if (createProjectRequest.shortname.isEmpty) throw BadRequestException("'Shortname' cannot be empty")) - - // check if the requesting user is allowed to create project - _ = if (!requestingUser.permissions.isSystemAdmin) { - // not a system admin - throw ForbiddenException("A new project can only be created by a system admin.") - } // check if the supplied shortname is unique shortnameExists <- projectByShortnameExists(createProjectRequest.shortname) @@ -1063,24 +1057,25 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo s"Project with the shortname: '${createProjectRequest.shortname}' already exists") } - validatedShortcode: String = StringFormatter.getGeneralInstance.validateProjectShortcode( - createProjectRequest.shortcode, - errorFun = - throw BadRequestException(s"The supplied short code: '${createProjectRequest.shortcode}' is not valid.") - ) - // check if the optionally supplied shortcode is valid and unique - shortcodeExists <- projectByShortcodeExists(validatedShortcode) + shortcodeExists <- projectByShortcodeExists(createProjectRequest.shortcode) _ = if (shortcodeExists) { throw DuplicateValueException( s"Project with the shortcode: '${createProjectRequest.shortcode}' already exists") } + // check if the requesting user is allowed to create project + _ = if (!requestingUser.permissions.isSystemAdmin) { + // not a system admin + throw ForbiddenException("A new project can only be created by a system admin.") + } + // check the custom IRI; if not given, create an unused IRI customProjectIri: Option[SmartIri] = createProjectRequest.id.map(iri => iri.toSmartIri) - newProjectIRI: IRI <- checkOrCreateEntityIri(customProjectIri, - stringFormatter.makeRandomProjectIri(validatedShortcode)) + newProjectIRI: IRI <- checkOrCreateEntityIri( + customProjectIri, + stringFormatter.makeRandomProjectIri(createProjectRequest.shortcode)) createNewProjectSparqlString = org.knora.webapi.messages.twirl.queries.sparql.admin.txt .createNewProject( @@ -1089,7 +1084,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo projectIri = newProjectIRI, projectClassIri = OntologyConstants.KnoraAdmin.KnoraProject, shortname = createProjectRequest.shortname, - shortcode = validatedShortcode, + shortcode = createProjectRequest.shortcode, maybeLongname = createProjectRequest.longname, maybeDescriptions = if (createProjectRequest.description.nonEmpty) { Some(createProjectRequest.description) @@ -1122,7 +1117,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo // create permissions for admins and members of the new group _ <- createPermissionsForAdminsAndMembersOfNewProject(newProjectIRI, newProjectADM.shortcode) - } yield ProjectOperationResponseADM(project = newProjectADM) + } yield ProjectOperationResponseADM(project = newProjectADM.unescape) for { // run user creation with an global IRI lock @@ -1280,7 +1275,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo .head .asInstanceOf[BooleanLiteralV2] .value - ) + ).unescape } /** @@ -1374,7 +1369,7 @@ class ProjectsResponderADM(responderData: ResponderData) extends Responder(respo result.map { case Some(project) => log.debug("getProjectFromCache - cache hit for: {}", identifier) - Some(project) + Some(project.unescape) case None => log.debug("getUserProjectCache - no cache hit for: {}", identifier) None diff --git a/webapi/src/main/scala/org/knora/webapi/routing/admin/ProjectsRouteADM.scala b/webapi/src/main/scala/org/knora/webapi/routing/admin/ProjectsRouteADM.scala index 347ee34650..a32334b12a 100644 --- a/webapi/src/main/scala/org/knora/webapi/routing/admin/ProjectsRouteADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/routing/admin/ProjectsRouteADM.scala @@ -140,7 +140,7 @@ class ProjectsRouteADM(routeData: KnoraRouteData) ) } yield ProjectCreateRequestADM( - createRequest = apiRequest, + createRequest = apiRequest.validateAndEscape, featureFactoryConfig = featureFactoryConfig, requestingUser = requestingUser, apiRequestID = UUID.randomUUID() @@ -332,7 +332,7 @@ class ProjectsRouteADM(routeData: KnoraRouteData) } yield ProjectChangeRequestADM( projectIri = checkedProjectIri, - changeProjectRequest = apiRequest, + changeProjectRequest = apiRequest.validateAndEscape, featureFactoryConfig = featureFactoryConfig, requestingUser = requestingUser, apiRequestID = UUID.randomUUID() diff --git a/webapi/src/test/scala/org/knora/webapi/messages/StringFormatterSpec.scala b/webapi/src/test/scala/org/knora/webapi/messages/StringFormatterSpec.scala index 8d0cfe72b7..cd74c058b6 100644 --- a/webapi/src/test/scala/org/knora/webapi/messages/StringFormatterSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/messages/StringFormatterSpec.scala @@ -1181,12 +1181,32 @@ class StringFormatterSpec extends CoreSpec() { } "validate project shortname" in { - stringFormatter.validateAndEscapeProjectShortname("images", throw AssertionException("not valid")) should be( - "images") - - // to short + // shortname with dash is valid + stringFormatter.validateAndEscapeProjectShortname("valid-shortname", throw AssertionException("not valid")) should be( + "valid-shortname") + + // shortname with numbers + stringFormatter.validateAndEscapeProjectShortname("valid_1111", throw AssertionException("not valid")) should be( + "valid_1111") + // has special character colon + an[AssertionException] should be thrownBy { + stringFormatter.validateAndEscapeProjectShortname("invalid:shortname", throw AssertionException("not valid")) + } + // begins with dash + an[AssertionException] should be thrownBy { + stringFormatter.validateAndEscapeProjectShortname("-invalidshortname", throw AssertionException("not valid")) + } + // begins with dot + an[AssertionException] should be thrownBy { + stringFormatter.validateAndEscapeProjectShortname(".invalidshortname", throw AssertionException("not valid")) + } + // includes slash + an[AssertionException] should be thrownBy { + stringFormatter.validateAndEscapeProjectShortname("invalid/shortname", throw AssertionException("not valid")) + } + // includes @ an[AssertionException] should be thrownBy { - stringFormatter.validateAndEscapeProjectShortname("abc", throw AssertionException("not valid")) + stringFormatter.validateAndEscapeProjectShortname("invalid@shortname", throw AssertionException("not valid")) } } diff --git a/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADMSpec.scala b/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADMSpec.scala index 3ab7a5a238..71239c8552 100644 --- a/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADMSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADMSpec.scala @@ -35,14 +35,15 @@ class PermissionsMessagesADMSpec extends CoreSpec() { "Administrative Permission Get Requests" should { "return 'BadRequest' if the supplied project IRI for AdministrativePermissionsForProjectGetRequestADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( AdministrativePermissionsForProjectGetRequestADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, requestingUser = SharedTestDataADM.imagesUser01, apiRequestID = UUID.randomUUID() ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'ForbiddenException' if the user requesting AdministrativePermissionsForProjectGetRequestADM is not system or project Admin" in { @@ -80,14 +81,15 @@ class PermissionsMessagesADMSpec extends CoreSpec() { } "return 'BadRequest' if the supplied project IRI for AdministrativePermissionForProjectGroupGetADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( AdministrativePermissionForProjectGroupGetADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, groupIri = OntologyConstants.KnoraAdmin.ProjectMember, requestingUser = SharedTestDataADM.imagesUser01 ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'ForbiddenException' if the user requesting AdministrativePermissionForProjectGroupGetADM is not system or project Admin" in { @@ -104,10 +106,11 @@ class PermissionsMessagesADMSpec extends CoreSpec() { "Administrative Permission Create Requests" should { "return 'BadRequest' if the supplied project IRI for AdministrativePermissionCreateRequestADM is not valid" in { + val forProject = "invalid-project-IRI" val caught = intercept[BadRequestException]( AdministrativePermissionCreateRequestADM( createRequest = CreateAdministrativePermissionAPIRequestADM( - forProject = "invalid-project-IRI", + forProject = forProject, forGroup = OntologyConstants.KnoraAdmin.ProjectMember, hasPermissions = Set(PermissionADM.ProjectAdminAllPermission) ), @@ -116,7 +119,7 @@ class PermissionsMessagesADMSpec extends CoreSpec() { apiRequestID = UUID.randomUUID() ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $forProject") } "return 'BadRequest' if the supplied group IRI for AdministrativePermissionCreateRequestADM is not valid" in { @@ -215,14 +218,15 @@ class PermissionsMessagesADMSpec extends CoreSpec() { "Default Object Access Permission Get Requests" should { "return 'BadRequest' if the supplied project IRI for DefaultObjectAccessPermissionGetADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( DefaultObjectAccessPermissionGetRequestADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, groupIri = Some(OntologyConstants.KnoraAdmin.ProjectMember), requestingUser = SharedTestDataADM.imagesUser01 ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'BadRequest' if the supplied resourceClass IRI for DefaultObjectAccessPermissionGetADM is not valid" in { @@ -297,14 +301,15 @@ class PermissionsMessagesADMSpec extends CoreSpec() { } "return 'BadRequest' if the supplied project IRI for DefaultObjectAccessPermissionsForProjectGetRequestADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( DefaultObjectAccessPermissionsForProjectGetRequestADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, requestingUser = SharedTestDataADM.imagesUser01, apiRequestID = UUID.randomUUID() ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'ForbiddenException' if the user requesting DefaultObjectAccessPermissionsForProjectGetRequestADM is not System or project Admin" in { @@ -356,15 +361,16 @@ class PermissionsMessagesADMSpec extends CoreSpec() { } "return 'BadRequest' if the supplied project IRI DefaultObjectAccessPermissionsStringForResourceClassGetADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( DefaultObjectAccessPermissionsStringForResourceClassGetADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, resourceClassIri = SharedOntologyTestDataADM.IMAGES_BILD_RESOURCE_CLASS, targetUser = SharedTestDataADM.imagesUser02, requestingUser = SharedTestDataADM.imagesUser01 ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'BadRequest' if the target user of DefaultObjectAccessPermissionsStringForResourceClassGetADM is an Anonymous user" in { @@ -380,16 +386,17 @@ class PermissionsMessagesADMSpec extends CoreSpec() { } "return 'BadRequest' if the supplied project IRI DefaultObjectAccessPermissionsStringForPropertyGetADM is not valid" in { + val projectIri = "" val caught = intercept[BadRequestException]( DefaultObjectAccessPermissionsStringForPropertyGetADM( - projectIri = "", + projectIri = projectIri, resourceClassIri = SharedOntologyTestDataADM.IMAGES_BILD_RESOURCE_CLASS, propertyIri = SharedOntologyTestDataADM.IMAGES_TITEL_PROPERTY, targetUser = SharedTestDataADM.imagesUser02, requestingUser = SharedTestDataADM.imagesUser01 ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'BadRequest' if the supplied resourceClass IRI for DefaultObjectAccessPermissionsStringForPropertyGetADM is not valid" in { @@ -449,10 +456,11 @@ class PermissionsMessagesADMSpec extends CoreSpec() { "Default Object Access Permission Create Requests" should { "return 'BadRequest' if the supplied project IRI for DefaultObjectAccessPermissionCreateRequestADM is not valid" in { + val forProject = "invalid-project-IRI" val caught = intercept[BadRequestException]( DefaultObjectAccessPermissionCreateRequestADM( createRequest = CreateDefaultObjectAccessPermissionAPIRequestADM( - forProject = "invalid-project-IRI", + forProject = forProject, forGroup = Some(OntologyConstants.KnoraAdmin.ProjectMember), hasPermissions = Set(PermissionADM.changeRightsPermission(OntologyConstants.KnoraAdmin.ProjectMember)) ), @@ -461,7 +469,7 @@ class PermissionsMessagesADMSpec extends CoreSpec() { apiRequestID = UUID.randomUUID() ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $forProject") } "return 'BadRequest' if the supplied group IRI for DefaultObjectAccessPermissionCreateRequestADM is not valid" in { @@ -616,15 +624,16 @@ class PermissionsMessagesADMSpec extends CoreSpec() { "get all project permissions" should { "return 'BadRequest' if the supplied project IRI for PermissionsForProjectGetRequestADM is not valid" in { + val projectIri = "invalid-project-IRI" val caught = intercept[BadRequestException]( PermissionsForProjectGetRequestADM( - projectIri = "invalid-project-IRI", + projectIri = projectIri, featureFactoryConfig = defaultFeatureFactoryConfig, requestingUser = SharedTestDataADM.imagesUser01, apiRequestID = UUID.randomUUID() ) ) - assert(caught.getMessage === "Invalid project IRI") + assert(caught.getMessage === s"Invalid project IRI $projectIri") } "return 'ForbiddenException' if the user requesting PermissionsForProjectGetRequestADM is not system or project Admin" in { diff --git a/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADMSpec.scala b/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADMSpec.scala index acd705e9ec..6fd2f90199 100644 --- a/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADMSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/messages/admin/responder/projectsmessages/ProjectsMessagesADMSpec.scala @@ -70,10 +70,44 @@ class ProjectsMessagesADMSpec extends CoreSpec(ProjectsMessagesADMSpec.config) { logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ) + ).validateAndEscape ) assert(caught.getMessage === "Invalid project IRI") } + + "return 'BadRequestException' if project 'shortname' during creation is missing" in { + + val caught = intercept[BadRequestException]( + CreateProjectApiRequestADM( + shortname = "", + shortcode = "1114", + longname = Some("project longname"), + description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), + keywords = Seq("keywords"), + logo = Some("/fu/bar/baz.jpg"), + status = true, + selfjoin = false + ).validateAndEscape + ) + assert(caught.getMessage === s"Project shortname must be supplied.") + } + + "return 'BadRequestException' if project 'shortcode' during creation is missing" in { + + val caught = intercept[BadRequestException]( + CreateProjectApiRequestADM( + shortname = "newproject4", + shortcode = "", + longname = Some("project longname"), + description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), + keywords = Seq("keywords"), + logo = Some("/fu/bar/baz.jpg"), + status = true, + selfjoin = false + ).validateAndEscape + ) + assert(caught.getMessage === "Project shortcode must be supplied.") + } } "The ChangeProjectApiRequestADM case class" should { diff --git a/webapi/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala b/webapi/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala index 283b9794ef..a47603d7d2 100644 --- a/webapi/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala @@ -228,7 +228,7 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() @@ -315,7 +315,7 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() @@ -328,85 +328,74 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) received.project.description should be( Seq(StringLiteralV2(value = "project description", language = Some("en")))) - //println(s"newProjectIri: ${newProjectIri.get}") } - "return a 'DuplicateValueException' during creation if the supplied project shortname is not unique" in { - responderManager ! ProjectCreateRequestADM( - CreateProjectApiRequestADM( - shortname = "newproject", - shortcode = "111C", - longname = Some("project longname"), - description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), - keywords = Seq("keywords"), - logo = Some("/fu/bar/baz.jpg"), - status = true, - selfjoin = false - ), - featureFactoryConfig = defaultFeatureFactoryConfig, - SharedTestDataADM.rootUser, - UUID.randomUUID() - ) - expectMsg(Failure(DuplicateValueException(s"Project with the shortname: 'newproject' already exists"))) - } + "CREATE a project that its info has special characters" in { - "return a 'DuplicateValueException' during creation if the supplied project shortname is unique but the shortcode is not" in { + val longnameWithSpecialCharacter = "New \\\"Longname\\\"" + val descriptionWithSpecialCharacter = "project \\\"description\\\"" + val keywordWithSpecialCharacter = "new \\\"keyword\\\"" responderManager ! ProjectCreateRequestADM( CreateProjectApiRequestADM( - shortname = "newproject3", - shortcode = "111C", - longname = Some("project longname"), - description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), - keywords = Seq("keywords"), + shortname = "project_with_character", + shortcode = "1312", + longname = Some(longnameWithSpecialCharacter), + description = Seq(StringLiteralV2(value = descriptionWithSpecialCharacter, language = Some("en"))), + keywords = Seq(keywordWithSpecialCharacter), logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() ) - expectMsg(Failure(DuplicateValueException(s"Project with the shortcode: '111C' already exists"))) - } + val received: ProjectOperationResponseADM = expectMsgType[ProjectOperationResponseADM](timeout) - "return 'BadRequestException' if project 'shortname' during creation is missing" in { + received.project.longname should contain(stringFormatter.fromSparqlEncodedString(longnameWithSpecialCharacter)) + received.project.description should be( + Seq(StringLiteralV2(value = stringFormatter.fromSparqlEncodedString(descriptionWithSpecialCharacter), + language = Some("en")))) + received.project.keywords should contain(stringFormatter.fromSparqlEncodedString(keywordWithSpecialCharacter)) + + } + "return a 'DuplicateValueException' during creation if the supplied project shortname is not unique" in { responderManager ! ProjectCreateRequestADM( CreateProjectApiRequestADM( - shortname = "", - shortcode = "1114", + shortname = "newproject", + shortcode = "111C", longname = Some("project longname"), description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), keywords = Seq("keywords"), logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() ) - expectMsg(Failure(BadRequestException("'Shortname' cannot be empty"))) + expectMsg(Failure(DuplicateValueException(s"Project with the shortname: 'newproject' already exists"))) } - "return 'BadRequestException' if project 'shortcode' during creation is missing" in { - + "return a 'DuplicateValueException' during creation if the supplied project shortname is unique but the shortcode is not" in { responderManager ! ProjectCreateRequestADM( CreateProjectApiRequestADM( - shortname = "newproject4", - shortcode = "", + shortname = "newproject3", + shortcode = "111C", longname = Some("project longname"), description = Seq(StringLiteralV2(value = "project description", language = Some("en"))), keywords = Seq("keywords"), logo = Some("/fu/bar/baz.jpg"), status = true, selfjoin = false - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() ) - expectMsg(Failure(BadRequestException("The supplied short code: '' is not valid."))) + expectMsg(Failure(DuplicateValueException(s"Project with the shortcode: '111C' already exists"))) } "UPDATE a project" in { @@ -422,7 +411,7 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) logo = Some("/fu/bar/baz-updated.jpg"), status = Some(false), selfjoin = Some(true) - ), + ).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() @@ -443,7 +432,7 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) "return 'NotFound' if a not existing project IRI is submitted during update" in { responderManager ! ProjectChangeRequestADM( projectIri = "http://rdfh.ch/projects/notexisting", - changeProjectRequest = ChangeProjectApiRequestADM(longname = Some("new long name")), + changeProjectRequest = ChangeProjectApiRequestADM(longname = Some("new long name")).validateAndEscape, featureFactoryConfig = defaultFeatureFactoryConfig, SharedTestDataADM.rootUser, UUID.randomUUID() @@ -670,7 +659,7 @@ class ProjectsResponderADMSpec extends CoreSpec(ProjectsResponderADMSpec.config) SharedTestDataADM.rootUser ) val received: ProjectsKeywordsGetResponseADM = expectMsgType[ProjectsKeywordsGetResponseADM](timeout) - received.keywords.size should be(20) + received.keywords.size should be(21) } "return all keywords for a single project" in {