From 0b42ce4fdc58eb61b1413ce9b14ada3012445a7d Mon Sep 17 00:00:00 2001 From: Ivan Subotic <400790+subotic@users.noreply.github.com> Date: Tue, 28 Jan 2020 11:38:40 +0100 Subject: [PATCH 1/2] fix (webapi): Add enforcing of restrictions for username and email --- docs/src/paradox/03-apis/api-admin/users.md | 5 +++ .../responders/admin/UsersResponderADM.scala | 10 ++++- .../knora/webapi/util/StringFormatter.scala | 19 +++++++++- .../admin/UsersResponderADMSpec.scala | 37 +++++++++++++++++++ .../webapi/util/StringFormatterSpec.scala | 14 +++++++ 5 files changed, 81 insertions(+), 4 deletions(-) diff --git a/docs/src/paradox/03-apis/api-admin/users.md b/docs/src/paradox/03-apis/api-admin/users.md index c9b0656006..624ce4cb9a 100644 --- a/docs/src/paradox/03-apis/api-admin/users.md +++ b/docs/src/paradox/03-apis/api-admin/users.md @@ -68,6 +68,11 @@ License along with Knora. If not, see . - Required permission: none, self-registration is allowed - Required information: email (unique), given name, family name, password, password, status, systemAdmin + - Username restrictions: + - 4 - 50 characters long + - Only contains alphanumeric characters, underscore and dot. + - Underscore and dot can't be at the end or start of a username + - Underscore or dot can't be used multiple times in a row - Returns information about the newly created user - TypeScript Docs: userFormats - `CreateUserApiRequestV1` - POST: `/admin/users` diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala index 27b11817d9..ab88129530 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala @@ -1167,9 +1167,15 @@ class UsersResponderADM(responderData: ResponderData) extends Responder(responde * The actual task run with an IRI lock. */ def createNewUserTask(createRequest: CreateUserApiRequestADM, requestingUser: UserADM, apiRequestID: UUID) = for { - // check if required information is supplied + // check username _ <- Future(if (createRequest.username.isEmpty) throw BadRequestException("Username cannot be empty")) + _ = stringFormatter.validateUsername(createRequest.username, throw BadRequestException(s"The username: '${createRequest.username}' contains invalid characters")) + + // check email _ = if (createRequest.email.isEmpty) throw BadRequestException("Email cannot be empty") + _ = stringFormatter.validateEmailAndThrow(createRequest.email, throw BadRequestException(s"The email: '${createRequest.email}' is invalid")) + + // check other _ = if (createRequest.password.isEmpty) throw BadRequestException("Password cannot be empty") _ = if (createRequest.givenName.isEmpty) throw BadRequestException("Given name cannot be empty") _ = if (createRequest.familyName.isEmpty) throw BadRequestException("Family name cannot be empty") @@ -1195,7 +1201,7 @@ class UsersResponderADM(responderData: ResponderData) extends Responder(responde triplestore = settings.triplestoreType, userIri = userIri, userClassIri = OntologyConstants.KnoraAdmin.User, - username = createRequest.username, + username = stringFormatter.validateAndEscapeUsername(createRequest.username, throw BadRequestException(s"The username: '${createRequest.username}' contains invalid characters")), email = createRequest.email, password = hashedPassword, givenName = createRequest.givenName, diff --git a/webapi/src/main/scala/org/knora/webapi/util/StringFormatter.scala b/webapi/src/main/scala/org/knora/webapi/util/StringFormatter.scala index 835c81931a..b017e80b5a 100644 --- a/webapi/src/main/scala/org/knora/webapi/util/StringFormatter.scala +++ b/webapi/src/main/scala/org/knora/webapi/util/StringFormatter.scala @@ -2682,12 +2682,27 @@ class StringFormatter private(val maybeSettings: Option[SettingsImpl] = None, ma } /** - * Check that the string represents a valid username. + * Check that the string represents a valid username. + * + * @param value the string to be checked. + * @param errorFun a function that throws an exception. It will be called if the string does not represent a valid + * username. + * @return the same string. + */ + def validateUsername(value: String, errorFun: => Nothing): String = { + UsernameRegex.findFirstIn(value) match { + case Some(username) => username + case None => errorFun + } + } + + /** + * Check that the string represents a valid username and escape any special characters. * * @param value the string to be checked. * @param errorFun a function that throws an exception. It will be called if the string does not represent a valid * username. - * @return the same string. + * @return the same string with escaped special characters. */ def validateAndEscapeUsername(value: String, errorFun: => Nothing): String = { UsernameRegex.findFirstIn(value) match { diff --git a/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala b/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala index fa57827ff8..2f4ec5cd86 100644 --- a/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala @@ -277,6 +277,43 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with ) expectMsg(Failure(DuplicateValueException(s"User with the email: 'root@example.com' already exists"))) } + + "return a 'BadRequestException' if the supplied 'username' contains invalid characters" in { + responderManager ! UserCreateRequestADM( + createRequest = CreateUserApiRequestADM( + username = "root2@example.com", + email = "root2@example.com", + givenName = "Donal", + familyName = "Duck", + password = "test", + status = true, + lang = "en", + systemAdmin = false + ), + SharedTestDataADM.anonymousUser, + UUID.randomUUID + ) + expectMsg(Failure(BadRequestException(s"The username: 'root2@example.com' contains invalid characters"))) + } + + "return a 'BadRequestException' if the supplied 'email' is invalid" in { + responderManager ! UserCreateRequestADM( + createRequest = CreateUserApiRequestADM( + username = "root3", + email = "root3", + givenName = "Donal", + familyName = "Duck", + password = "test", + status = true, + lang = "en", + systemAdmin = false + ), + SharedTestDataADM.anonymousUser, + UUID.randomUUID + ) + expectMsg(Failure(BadRequestException(s"The email: 'root3' is invalid"))) + } + } "asked to update a user" should { diff --git a/webapi/src/test/scala/org/knora/webapi/util/StringFormatterSpec.scala b/webapi/src/test/scala/org/knora/webapi/util/StringFormatterSpec.scala index 342cc8a139..f4c34afed7 100644 --- a/webapi/src/test/scala/org/knora/webapi/util/StringFormatterSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/util/StringFormatterSpec.scala @@ -1158,6 +1158,11 @@ class StringFormatterSpec extends CoreSpec() { stringFormatter.validateAndEscapeUsername("a_2.3-4", throw AssertionException("not valid")) } + // not allow @ + an[AssertionException] should be thrownBy { + stringFormatter.validateUsername("donald.duck@example.com", throw AssertionException("not valid")) + } + // Underscore and dot can't be at the end or start of a username an[AssertionException] should be thrownBy { stringFormatter.validateAndEscapeUsername("_username", throw AssertionException("not valid")) @@ -1183,6 +1188,15 @@ class StringFormatterSpec extends CoreSpec() { } + "validate email" in { + + stringFormatter.validateEmailAndThrow("donald.duck@example.com", throw AssertionException("not valid")) should be ("donald.duck@example.com") + + an[AssertionException] should be thrownBy { + stringFormatter.validateEmailAndThrow("donald.duck", throw AssertionException("not valid")) + } + } + "convert a UUID to Base-64 encoding and back again" in { val uuid = UUID.randomUUID val base64EncodedUuid = stringFormatter.base64EncodeUuid(uuid) From 373778a897fcbfcf7570f14694c504677ca86f34 Mon Sep 17 00:00:00 2001 From: Ivan Subotic <400790+subotic@users.noreply.github.com> Date: Fri, 31 Jan 2020 13:42:11 +0100 Subject: [PATCH 2/2] fix (webapi): Add enforcing of restrictions for username and email --- .../responders/admin/UsersResponderADM.scala | 7 ++ .../admin/UsersResponderADMSpec.scala | 69 +++++++++++++++++-- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala index ab88129530..6966660de8 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala @@ -259,8 +259,14 @@ class UsersResponderADM(responderData: ResponderData) extends Responder(responde KnoraSystemInstances.Users.SystemUser ) + // check if user exists + _ = if (currentUserInformation.isEmpty) { + throw BadRequestException(s"User ${userIri} does not exist") + } + // check if we want to change the email _ = if (changeUserRequest.email.isDefined) { + stringFormatter.validateEmailAndThrow(changeUserRequest.email.get, throw BadRequestException(s"The email: '${changeUserRequest.email.get}' is invalid")) currentUserInformation.map { user => // check if current email differs from the one in the change request if (!user.email.equals(changeUserRequest.email.get)) { @@ -274,6 +280,7 @@ class UsersResponderADM(responderData: ResponderData) extends Responder(responde // check if we want to change the username _ = if (changeUserRequest.username.isDefined) { + stringFormatter.validateUsername(changeUserRequest.username.get, throw BadRequestException(s"The username: '${changeUserRequest.username.get}' contains invalid characters")) currentUserInformation.map { user => // check if the current username differs from the one in the change request if (!user.username.equals(changeUserRequest.username.get)) { diff --git a/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala b/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala index 2f4ec5cd86..89650d1627 100644 --- a/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala @@ -278,11 +278,11 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with expectMsg(Failure(DuplicateValueException(s"User with the email: 'root@example.com' already exists"))) } - "return a 'BadRequestException' if the supplied 'username' contains invalid characters" in { + "return a 'BadRequestException' if the supplied 'username' contains invalid characters (@)" in { responderManager ! UserCreateRequestADM( createRequest = CreateUserApiRequestADM( - username = "root2@example.com", - email = "root2@example.com", + username = "donald.duck2@example.com", + email = "donald.duck2@example.com", givenName = "Donal", familyName = "Duck", password = "test", @@ -293,7 +293,25 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with SharedTestDataADM.anonymousUser, UUID.randomUUID ) - expectMsg(Failure(BadRequestException(s"The username: 'root2@example.com' contains invalid characters"))) + expectMsg(Failure(BadRequestException(s"The username: 'donald.duck2@example.com' contains invalid characters"))) + } + + "return a 'BadRequestException' if the supplied 'username' contains invalid characters (-)" in { + responderManager ! UserCreateRequestADM( + createRequest = CreateUserApiRequestADM( + username = "donald-duck", + email = "donald.duck2@example.com", + givenName = "Donal", + familyName = "Duck", + password = "test", + status = true, + lang = "en", + systemAdmin = false + ), + SharedTestDataADM.anonymousUser, + UUID.randomUUID + ) + expectMsg(Failure(BadRequestException(s"The username: 'donald-duck' contains invalid characters"))) } "return a 'BadRequestException' if the supplied 'email' is invalid" in { @@ -371,6 +389,49 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with } + "return 'BadRequest' if the new 'username' contains invalid characters (@)" in { + + responderManager ! UserChangeBasicUserInformationRequestADM( + userIri = SharedTestDataADM.normalUser.id, + changeUserRequest = ChangeUserApiRequestADM( + username = Some("donald.duck2@example.com") + ), + requestingUser = SharedTestDataADM.superUser, + UUID.randomUUID() + ) + + expectMsg(timeout, Failure(BadRequestException(s"The username: 'donald.duck2@example.com' contains invalid characters"))) + } + + "return 'BadRequest' if the new 'username' contains invalid characters (-)" in { + + responderManager ! UserChangeBasicUserInformationRequestADM( + userIri = SharedTestDataADM.normalUser.id, + changeUserRequest = ChangeUserApiRequestADM( + username = Some("donald-duck") + ), + requestingUser = SharedTestDataADM.superUser, + UUID.randomUUID() + ) + + expectMsg(timeout, Failure(BadRequestException(s"The username: 'donald-duck' contains invalid characters"))) + } + + + "return 'BadRequest' if the new 'email' is invalid" in { + + responderManager ! UserChangeBasicUserInformationRequestADM( + userIri = SharedTestDataADM.normalUser.id, + changeUserRequest = ChangeUserApiRequestADM( + email = Some("root3") + ), + requestingUser = SharedTestDataADM.superUser, + UUID.randomUUID() + ) + + expectMsg(timeout, Failure(BadRequestException(s"The email: 'root3' is invalid"))) + } + "UPDATE the user's password (by himself)" in { responderManager ! UserChangePasswordRequestADM( userIri = SharedTestDataADM.normalUser.id,