Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix (webapi): Add enforcing of restrictions for username and email #1587

Merged
merged 4 commits into from
Feb 9, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/src/paradox/03-apis/api-admin/users.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,61 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with
)
expectMsg(Failure(DuplicateValueException(s"User with the email: '[email protected]' already exists")))
}

"return a 'BadRequestException' if the supplied 'username' contains invalid characters (@)" in {
responderManager ! UserCreateRequestADM(
createRequest = CreateUserApiRequestADM(
username = "[email protected]",
email = "[email protected]",
givenName = "Donal",
familyName = "Duck",
password = "test",
status = true,
lang = "en",
systemAdmin = false
),
SharedTestDataADM.anonymousUser,
UUID.randomUUID
)
expectMsg(Failure(BadRequestException(s"The username: '[email protected]' contains invalid characters")))
}

"return a 'BadRequestException' if the supplied 'username' contains invalid characters (-)" in {
responderManager ! UserCreateRequestADM(
createRequest = CreateUserApiRequestADM(
username = "donald-duck",
email = "[email protected]",
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("[email protected]")
),
requestingUser = SharedTestDataADM.superUser,
UUID.randomUUID()
)

expectMsg(timeout, Failure(BadRequestException(s"The username: '[email protected]' 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
Original file line number Diff line number Diff line change
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("[email protected]", 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("[email protected]", throw AssertionException("not valid")) should be ("[email protected]")

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