Skip to content

Commit

Permalink
fix (webapi): Add enforcing of restrictions for username and email (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
subotic committed Feb 9, 2020
1 parent faa2e55 commit ccb64bb
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 4 deletions.
5 changes: 5 additions & 0 deletions docs/src/paradox/03-apis/api-admin/users.md
Expand Up @@ -68,6 +68,11 @@ License along with Knora. If not, see <http://www.gnu.org/licenses/>.
- 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`
Expand Down
Expand Up @@ -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)) {
Expand All @@ -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)) {
Expand Down Expand Up @@ -1167,9 +1174,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")
Expand All @@ -1195,7 +1208,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,
Expand Down
19 changes: 17 additions & 2 deletions webapi/src/main/scala/org/knora/webapi/util/StringFormatter.scala
Expand Up @@ -2658,12 +2658,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 {
Expand Down
Expand Up @@ -277,6 +277,61 @@ 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 = "donald.duck2@example.com",
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.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 {
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 {
Expand Down Expand Up @@ -334,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,
Expand Down
Expand Up @@ -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"))
Expand All @@ -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)
Expand Down

0 comments on commit ccb64bb

Please sign in to comment.