From bec70fc3240dc966b8e1fbc2952029c6a5bc3463 Mon Sep 17 00:00:00 2001 From: Simon Redfern Date: Sat, 31 Dec 2016 19:44:45 +0100 Subject: [PATCH] Nicer response from functions that check values --- src/main/scala/code/api/directlogin.scala | 15 ++++++++------- src/main/scala/code/api/util/APIUtil.scala | 18 ++++++++++++------ .../scala/code/model/dataAccess/OBPUser.scala | 13 +++++++------ src/main/scala/code/util/Helper.scala | 10 ++++++++++ 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/main/scala/code/api/directlogin.scala b/src/main/scala/code/api/directlogin.scala index 8cbe3aaff..ea5f57a7b 100644 --- a/src/main/scala/code/api/directlogin.scala +++ b/src/main/scala/code/api/directlogin.scala @@ -42,6 +42,7 @@ import net.liftweb.util.Helpers._ import scala.compat.Platform import code.api.util.{APIUtil, ErrorMessages} +import code.util.Helper.SILENCE_IS_GOLDEN /** * This object provides the API calls necessary to @@ -189,18 +190,18 @@ object DirectLogin extends RestHelper with Loggable { /**Validate user supplied Direct Login parameters before they are used further, * guard maximum length and content of strings (a-z,0-9 etc.) */ def validDirectLoginParameters(parameters: Map[String, String]): Iterable[String] = { - for( key <- parameters.keys )yield { + for (key <- parameters.keys) yield { val parameterValue = parameters.get(key).get key match { case "username" => - assertMediumString(parameterValue) + checkMediumString(parameterValue) case "password" => - assertMediumAlphaNumeric(parameterValue) + checkMediumAlphaNumeric(parameterValue) case "consumer_key" => - assertMediumAlphaNumeric(parameterValue) + checkMediumAlphaNumeric(parameterValue) case "token" => - assertMediumString(parameterValue) - case _ =>ErrorMessages.InvalidDirectLoginParameters + checkMediumString(parameterValue) + case _ => ErrorMessages.InvalidDirectLoginParameters } } } @@ -231,7 +232,7 @@ object DirectLogin extends RestHelper with Loggable { message = ErrorMessages.DirectLoginMissingParameters + missingParams.mkString(", ") httpCode = 400 } - else if("Success" != validParams.mkString("")){ + else if(SILENCE_IS_GOLDEN != validParams.mkString("")){ message = validParams.mkString("") httpCode = 400 } diff --git a/src/main/scala/code/api/util/APIUtil.scala b/src/main/scala/code/api/util/APIUtil.scala index abcda79dc..e570cb87e 100644 --- a/src/main/scala/code/api/util/APIUtil.scala +++ b/src/main/scala/code/api/util/APIUtil.scala @@ -54,6 +54,7 @@ import net.liftweb.util.{Helpers, Props, SecurityHelpers} import scala.xml.{Elem, XML} import scala.collection.mutable.ArrayBuffer import scala.collection.JavaConverters._ +import code.util.Helper.SILENCE_IS_GOLDEN object ErrorMessages { @@ -291,34 +292,39 @@ object APIUtil extends Loggable { } } + + /** These three functions check rather than assert. I.e. they are silent if OK and return an error message if not. + * They do not throw an exception on failure thus they are not assertions + */ + /** only A-Z ,a-z and max length <= 512 */ - def assertMediumAlpha(value:String): String ={ + def checkMediumAlpha(value:String): String ={ val valueLength = value.length val regex = """^([A-Za-z]+)$""".r value match { - case regex(e) if(valueLength <= 512) => "Success" + case regex(e) if(valueLength <= 512) => SILENCE_IS_GOLDEN case regex(e) if(valueLength > 512) => ErrorMessages.InvalidValueLength case _ => ErrorMessages.InvalidValueCharacters } } /** only A-Z ,a-z ,0-9 and max length <= 512 */ - def assertMediumAlphaNumeric(value:String): String ={ + def checkMediumAlphaNumeric(value:String): String ={ val valueLength = value.length val regex = """^([A-Za-z0-9]+)$""".r value match { - case regex(e) if(valueLength <= 512) => "Success" + case regex(e) if(valueLength <= 512) => SILENCE_IS_GOLDEN case regex(e) if(valueLength > 512) => ErrorMessages.InvalidValueLength case _ => ErrorMessages.InvalidValueCharacters } } /** only A-Z ,a-z ,0-9 ,-,_,.and max length <= 512 */ - def assertMediumString(value:String): String ={ + def checkMediumString(value:String): String ={ val valueLength = value.length val regex = """^([A-Za-z0-9\-._@]+)$""".r value match { - case regex(e) if(valueLength <= 512) => "Success" + case regex(e) if(valueLength <= 512) => SILENCE_IS_GOLDEN case regex(e) if(valueLength > 512) => ErrorMessages.InvalidValueLength case _ => ErrorMessages.InvalidValueCharacters } diff --git a/src/main/scala/code/model/dataAccess/OBPUser.scala b/src/main/scala/code/model/dataAccess/OBPUser.scala index b3bb9100c..4028c0b21 100644 --- a/src/main/scala/code/model/dataAccess/OBPUser.scala +++ b/src/main/scala/code/model/dataAccess/OBPUser.scala @@ -364,7 +364,7 @@ import net.liftweb.util.Helpers._ case Full(user) => if ( user.validated_? && - // User is not locked AND the password is good + // User is NOT locked AND the password is good ! LoginAttempt.userIsLocked(username) && user.getProvider() == Props.get("hostname","") && user.testPassword(Full(password))) @@ -373,21 +373,19 @@ import net.liftweb.util.Helpers._ LoginAttempt.resetBadLoginAttempts(username) Full(user.user) // Return the user. } - //recording the login faild times when password is wrong + // User is locked OR password is bad else if ( user.validated_? && - // User is locked OR password is bad LoginAttempt.userIsLocked(username) || ! user.testPassword(Full(password)) ) { LoginAttempt.incrementBadLoginAttempts(username) Empty } + // User is locked else if (!LoginAttempt.userIsLocked(username) ) { info(ErrorMessages.UsernameHasBeenLocked) - S.error(S.?(ErrorMessages.UsernameHasBeenLocked)) - //Full(usernameLockedStateCode) Empty } else { @@ -512,6 +510,9 @@ import net.liftweb.util.Helpers._ case Full(user) if !user.validated_? => S.error(S.?("account.validation.error")) + + // TODO Check the User Lock situation for non mapped users + case _ => if (connector == "kafka" || connector == "obpjvm") { // If not found locally, try to authenticate user via Kafka, if enabled in props @@ -591,7 +592,7 @@ import net.liftweb.util.Helpers._ val theUser: TheUserType = mutateUserOnSignup(createNewUserInstance()) val theName = signUpPath.mkString("") - //save the intented login redirect here, as it gets wiped (along with the session) on login + //save the intended login redirect here, as it gets wiped (along with the session) on login val loginRedirectSave = loginRedirect.is def testSignup() { diff --git a/src/main/scala/code/util/Helper.scala b/src/main/scala/code/util/Helper.scala index 733029ca0..c65e8e1f2 100644 --- a/src/main/scala/code/util/Helper.scala +++ b/src/main/scala/code/util/Helper.scala @@ -11,6 +11,16 @@ import net.liftweb.json.Printer._ object Helper{ + /** + * + * + */ + + // If we need to return a string and all good, return an empty string + // rule of silence http://www.linfo.org/rule_of_silence.html + val SILENCE_IS_GOLDEN = "" + + /** * A css selector that will (unless you have a template containing an element * name i_am_an_id_that_should_never_exist) have no effect. Useful when you have