Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

PasswordField fails to change the current password and it throws validation errors when no password was changed #1276

Merged
merged 1 commit into from

2 participants

@rusho
Collaborator

Torstens message:

when I want to change the current password, I see that the apply() method is called and I see the "Password changed" message, but the password is still the old one.

Then, when I try to change the user data (that do not contain the password), I get the error message with that key: "password.must.be.set"
That's not a problem of apply() I think. Rather I would guess that the password field does not validate if it had not been filled with a new password.
And since changing the user data validates the user object and thus also the password field, that problem appears…


setFromAny() is completely wrong, I forgot to rewrite it when doing the change.

group discussion: https://groups.google.com/forum/?fromgroups#!topic/liftweb/OErzpfpyBjk

some of these issues probably cause this problem as well, though set_! shoud stay as is, as far as I'm aware of, so proposed solution should not work: #1256

@rusho rusho was assigned
@rusho
Collaborator

"when I want to change the current password, I see that the apply() method is called and I see the "Password changed" message, but the password is still the old one."


this is invalid, it turned out to be a MongoDB issue, where in case of no _id, a new one has been assigned, thus record failed to update since there was no matching _id

@dpp dpp merged commit 58705b4 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 13, 2012
  1. @rusho

    Refactored PasswordField. Fixes issues with validations, setFromAny a…

    rusho authored
    …nd some other setting methods.
This page is out of date. Refresh to see the latest.
View
83 persistence/record/src/main/scala/net/liftweb/record/field/PasswordField.scala
@@ -37,8 +37,8 @@ object PasswordField {
}
trait PasswordTypedField extends TypedField[String] {
- private var invalidMsg : String = ""
- private[record] var validatedValue: Box[String] = valueBox
+ private var invalidPw = false
+ private var invalidMsg = ""
def match_?(toTest: String): Boolean =
valueBox.filter(_.length > 0)
@@ -46,32 +46,45 @@ trait PasswordTypedField extends TypedField[String] {
.openOr(false)
override def set_!(in: Box[String]): Box[String] = {
- // can't be hashed here, because this get's called when setting value from database
+ // can't be hashed here, because this get's called when setting value from database (Squeryl)
in
}
def setPlain(in: String): String = setBoxPlain(Full(in)) openOr defaultValue
def setBoxPlain(in: Box[String]): Box[String] = {
- validatedValue = in
- val hashed = in.map(s => PasswordField.hashpw(s) openOr s)
- setBox(hashed)
+ if(!validatePassword(in)) {
+ val hashed = in.map(s => PasswordField.hashpw(s) openOr s)
+ setBox(hashed)
+ }
+ else setBox(defaultValueBox)
}
+ /**
+ * If passed value is an Array[String] or a List[String] containing 2 items with equal value, it it hashes this value and sets it as new password.
+ * If passed value is a String or a Full[String] that starts with "$2a$", it assumes that it's a hashed version, thus sets it as it is, without hashing.
+ * In any other case, it fails the validation with "Passwords do not match" error
+ */
def setFromAny(in: Any): Box[String] = {
in match {
- case (a: Array[String]) if (a.length == 2 && a(0) == a(1)) => setBox(Full(a(0)))
- case (h1: String) :: (h2: String) :: Nil if h1 == h2 => setBox(Full(h1))
- case _ => genericSetFromAny(in)
+ case (a: Array[String]) if (a.length == 2 && a(0) == a(1)) => setBoxPlain(Full(a(0)))
+ case (h1: String) :: (h2: String) :: Nil if h1 == h2 => setBoxPlain(Full(h1))
+ case (hash: String) if(hash.startsWith("$2a$")) => setBox(Full(hash))
+ case Full(hash: String) if(hash.startsWith("$2a$")) => setBox(Full(hash))
+ case _ => {invalidPw = true; invalidMsg = S.??("passwords.do.not.match"); Failure(invalidMsg)}
}
}
def setFromString(s: String): Box[String] = s match {
- case "" if optional_? => setBox(Empty)
- case _ => setBox(Full(s))
+ case "" if optional_? => setBoxPlain(Empty)
+ case _ => setBoxPlain(Full(s))
}
- override def validate: List[FieldError] = runValidation(validatedValue)
+ override def validate: List[FieldError] = {
+ if (!invalidPw && valueBox != defaultValueBox) Nil
+ else if (invalidPw) List(FieldError(this, Text(invalidMsg)))
+ else List(FieldError(this, Text(notOptionalErrorMessage)))
+ }
override def notOptionalErrorMessage = S.??("password.must.be.set")
@@ -87,26 +100,29 @@ trait PasswordTypedField extends TypedField[String] {
case _ => Full(elem)
}
- protected def validatePassword(pwdValue: ValueType): List[FieldError] =
- toBoxMyType(pwdValue) match {
- case Empty|Full(""|null) if !optional_? => Text(notOptionalErrorMessage)
- case Full(s) if s == "*" || s == PasswordField.blankPw || s.length < PasswordField.minPasswordLength =>
- Text(S.??("password.too.short"))
- case _ => Nil
+ protected def validatePassword(pwdValue: Box[String]): Boolean = {
+ pwdValue match {
+ case Empty|Full(""|null) if !optional_? => { invalidPw = true ; invalidMsg = notOptionalErrorMessage }
+ case Full(s) if s == "" || s == PasswordField.blankPw || s.length < PasswordField.minPasswordLength =>
+ { invalidPw = true ; invalidMsg = S.??("password.too.short") }
+ case _ => { invalidPw = false; invalidMsg = "" }
}
-
- override def validations = validatePassword _ :: Nil
+ invalidPw
+ }
def defaultValue = ""
def asJs = valueBox.map(Str) openOr JsNull
def asJValue: JValue = valueBox.map(v => JString(v)) openOr (JNothing: JValue)
+
def setFromJValue(jvalue: JValue): Box[MyType] = jvalue match {
- case JNothing|JNull if optional_? => setBox(Empty)
+ case JNothing|JNull if optional_? => setBoxPlain(Empty)
case JString(s) => setFromString(s)
- case other => setBox(FieldHelpers.expectedA("JString", other))
+ case other => setBoxPlain(FieldHelpers.expectedA("JString", other))
}
+
+
}
class PasswordField[OwnerType <: Record[OwnerType]](rec: OwnerType)
@@ -120,11 +136,13 @@ class PasswordField[OwnerType <: Record[OwnerType]](rec: OwnerType)
def owner = rec
override def apply(in: Box[String]): OwnerType =
- {
- validatedValue = in
- val hashed = in.map(s => PasswordField.hashpw(s) openOr s)
- super.apply(hashed)
- }
+ if(owner.meta.mutable_?) {
+ this.setBoxPlain(in)
+ owner
+ } else {
+ owner.meta.createWithMutableField(owner, this, in)
+ }
+
}
class OptionalPasswordField[OwnerType <: Record[OwnerType]](rec: OwnerType)
@@ -138,10 +156,11 @@ class OptionalPasswordField[OwnerType <: Record[OwnerType]](rec: OwnerType)
def owner = rec
override def apply(in: Box[String]): OwnerType =
- {
- validatedValue = in
- val hashed = in.map(s => PasswordField.hashpw(s) openOr s)
- super.apply(hashed)
- }
+ if(owner.meta.mutable_?) {
+ this.setBoxPlain(in)
+ owner
+ } else {
+ owner.meta.createWithMutableField(owner, this, in)
+ }
}
View
8 persistence/record/src/test/scala/net/liftweb/record/FieldSpec.scala
@@ -408,11 +408,13 @@ object FieldSpec extends Specification("Record Field Specification") {
)
}
- "validate the unencrypted value" in {
+ "correctly validate the unencrypted value" in {
val rec = PasswordTestRecord.createRecord.password("testvalue")
-
+ rec.validate must_== Nil
+
+ rec.password("1234")
rec.validate must_== (
- FieldError(rec.password, Text("no way!")) ::
+ FieldError(rec.password, Text(S.??("password.too.short"))) ::
Nil
)
}
Something went wrong with that request. Please try again.