horizon: Trailing spaces removed on passwords
From the dashboard openstack is removing the trailing spaces from our user's passwords. We have a modified sql.py backend, that does an ldap bind to an active directory data store. And that works almost always. I say almost because for some users it doesn't work at all. We figure out (and a co-worker also confirmed this) that openstack is removing trailing (also leading?) spaces from the password entered in the dashboard. Also, inside the dashboard trailing spaces are not accepted even when they are equal byte by byte (including the space, I get an error). So this is going on.
Do anybody knows where is this removal performed? (python script location, line) So I can remove that since I have users (me included, I have the issue since the very beginning of this deployment) that cannot login. And they can use their Active Directrory passwords from other apps without problem. We are running 'stein' with the latest update for ubuntu 18.04-AMD64.
I have found a way to solve it and give access to users that have passwords with spaces at the beginning/end. The issue (not an issue per se, but it affects horizon [stein]) lies in django. Specifically on 'django/forms/fields.py' Horizon uses the fields and those by default remove spaces as stated, what I did is the following: On that file, the class CharField's constructor was removing leading/trailing spaces: Below is the diff between the original and the modified python script (one line modified, strip=False) --- fields.py.orig 2020-01-28 15:16:22.696047918 -0500 +++ fields.py 2020-01-28 15:16:45.520084974 -0500 @@ -220,7 +220,7 @@ class CharField(Field): - def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs): + def __init__(self, max_length=None, min_length=None, strip=False, empty_value='', *args, **kwargs): self.max_length = max_length self.min_length = min_length self.strip = strip Now passwords are not altered by the underlying framework. Not sure the effect of not removing trailing/leading spaces from the textfields will have on the Horizon operations, though. Maybe horizon should redefine that django class to avoid this behavior. I'm also open to other solutions from the community. Have a great evening, Thanks. Orestes On 1/28/20, Orestes Leal Rodríguez <olealrd1981@gmail.com> wrote:
From the dashboard openstack is removing the trailing spaces from our user's passwords. We have a modified sql.py backend, that does an ldap bind to an active directory data store. And that works almost always. I say almost because for some users it doesn't work at all. We figure out (and a co-worker also confirmed this) that openstack is removing trailing (also leading?) spaces from the password entered in the dashboard. Also, inside the dashboard trailing spaces are not accepted even when they are equal byte by byte (including the space, I get an error). So this is going on.
Do anybody knows where is this removal performed? (python script location, line) So I can remove that since I have users (me included, I have the issue since the very beginning of this deployment) that cannot login. And they can use their Active Directrory passwords from other apps without problem.
We are running 'stein' with the latest update for ubuntu 18.04-AMD64.
Stripping leading/trailing spaces from passwords is the correct behavior. Passwords should not contain leading/trailing spaces, and when they do it is usually because of a paste error. -----Original Message----- From: Orestes Leal Rodríguez <olealrd1981@gmail.com> Sent: Tuesday, January 28, 2020 1:24 PM To: openstack-discuss@lists.openstack.org Subject: Re: horizon: Trailing spaces removed on passwords I have found a way to solve it and give access to users that have passwords with spaces at the beginning/end. The issue (not an issue per se, but it affects horizon [stein]) lies in django. Specifically on 'django/forms/fields.py' Horizon uses the fields and those by default remove spaces as stated, what I did is the following: On that file, the class CharField's constructor was removing leading/trailing spaces: Below is the diff between the original and the modified python script (one line modified, strip=False) --- fields.py.orig 2020-01-28 15:16:22.696047918 -0500 +++ fields.py 2020-01-28 15:16:45.520084974 -0500 @@ -220,7 +220,7 @@ class CharField(Field): - def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs): + def __init__(self, max_length=None, min_length=None, strip=False, empty_value='', *args, **kwargs): self.max_length = max_length self.min_length = min_length self.strip = strip Now passwords are not altered by the underlying framework. Not sure the effect of not removing trailing/leading spaces from the textfields will have on the Horizon operations, though. Maybe horizon should redefine that django class to avoid this behavior. I'm also open to other solutions from the community. Have a great evening, Thanks. Orestes On 1/28/20, Orestes Leal Rodríguez <olealrd1981@gmail.com> wrote:
From the dashboard openstack is removing the trailing spaces from our user's passwords. We have a modified sql.py backend, that does an ldap bind to an active directory data store. And that works almost always. I say almost because for some users it doesn't work at all. We figure out (and a co-worker also confirmed this) that openstack is removing trailing (also leading?) spaces from the password entered in the dashboard. Also, inside the dashboard trailing spaces are not accepted even when they are equal byte by byte (including the space, I get an error). So this is going on.
Do anybody knows where is this removal performed? (python script location, line) So I can remove that since I have users (me included, I have the issue since the very beginning of this deployment) that cannot login. And they can use their Active Directrory passwords from other apps without problem.
We are running 'stein' with the latest update for ubuntu 18.04-AMD64.
Indeed, a well known web UX improving feature, very useful one. I hope nobody tries to remove it. This kind of feature must always be implemented in the client (browser). no server side API should ever try to “sanitize” a password string. On Tue, 28 Jan 2020 at 22:14, Albert Braden <Albert.Braden@synopsys.com> wrote:
Stripping leading/trailing spaces from passwords is the correct behavior. Passwords should not contain leading/trailing spaces, and when they do it is usually because of a paste error.
-----Original Message----- From: Orestes Leal Rodríguez <olealrd1981@gmail.com> Sent: Tuesday, January 28, 2020 1:24 PM To: openstack-discuss@lists.openstack.org Subject: Re: horizon: Trailing spaces removed on passwords
I have found a way to solve it and give access to users that have passwords with spaces at the beginning/end. The issue (not an issue per se, but it affects horizon [stein]) lies in django. Specifically on 'django/forms/fields.py'
Horizon uses the fields and those by default remove spaces as stated, what I did is the following:
On that file, the class CharField's constructor was removing leading/trailing spaces: Below is the diff between the original and the modified python script (one line modified, strip=False)
--- fields.py.orig 2020-01-28 15:16:22.696047918 -0500 +++ fields.py 2020-01-28 15:16:45.520084974 -0500 @@ -220,7 +220,7 @@
class CharField(Field): - def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs): + def __init__(self, max_length=None, min_length=None, strip=False, empty_value='', *args, **kwargs): self.max_length = max_length self.min_length = min_length self.strip = strip
Now passwords are not altered by the underlying framework. Not sure the effect of not removing trailing/leading spaces from the textfields will have on the Horizon operations, though. Maybe horizon should redefine that django class to avoid this behavior. I'm also open to other solutions from the community. Have a great evening,
Thanks. Orestes
On 1/28/20, Orestes Leal Rodríguez <olealrd1981@gmail.com> wrote:
From the dashboard openstack is removing the trailing spaces from our user's passwords. We have a modified sql.py backend, that does an ldap bind to an active directory data store. And that works almost always. I say almost because for some users it doesn't work at all. We figure out (and a co-worker also confirmed this) that openstack is removing trailing (also leading?) spaces from the password entered in the dashboard. Also, inside the dashboard trailing spaces are not accepted even when they are equal byte by byte (including the space, I get an error). So this is going on.
Do anybody knows where is this removal performed? (python script location, line) So I can remove that since I have users (me included, I have the issue since the very beginning of this deployment) that cannot login. And they can use their Active Directrory passwords from other apps without problem.
We are running 'stein' with the latest update for ubuntu 18.04-AMD64.
-- -- /sorin
Folks, I believe the password value should never ever be modified, that includes space stripping. Albert wrote:
Passwords should not contain leading/trailing spaces
Strong claim. I think it's clumsy if they do, but still a password is a password :-) Albert wrote:
it is usually because of a paste error
I agree here, I rarely see people willing to have trailing spaces in their passwords. UI/UX-wise people should be allowed to peek at their password as they are entering it (to validate its correctness). Also, it's the very reason why password change form has you to repeat the new password (and sometimes even blocks any copy-pasting which is actually bad UI/UX because it cripples password managers). Akihiro wrote:
Django AuthenticationForm does not strip an input password
Which is how it should be. Akihiro wrote:
Other usages of CharField may assume the default behavior.
Indeed, one should modify horizon, not django, here. Sorin wrote:
This kind of feature must always be implemented in the client (browser)
Well, it can (and is in this case) also be implemented on the server side (by horizon/django here). Sorin wrote:
no server side API should ever try to “sanitize” a password string.
Sanitization is always performed to avoid SQL injection and alike. -yoctozepto
I have two comments. The first one is whether we should strip leading/trailing spaces in password forms. As Albert commented, someone thinks they should be stripped. On the other hand, Django AuthenticationForm does not strip an input password [0]. I am not sure which is better. IIRC, horizon login form strips leading/trailing spaces in an input password since long ago but I see no discussion on this. The second one is about the original question. Django CharField strips leading/trailing spaces in an input string by default. strip=True is the default [1]. The horizon login form just uses CharField [2], so leading/trailing spaces in an input password are stripped. To avoid this, you need to add strip=False to CharField [2]. It is not a good idea to change the default value of CharField in Django. Other usages of CharField may assume the default behavior. If you would like horizon to change the current behavior, there are several password related fields and we need to change all of them consistently, so it is worth filing a bug. [0] https://github.com/django/django/blob/master/django/contrib/auth/forms.py#L1... [1] https://docs.djangoproject.com/en/3.0/ref/forms/fields/#charfield [2] https://opendev.org/openstack/horizon/src/branch/master/openstack_auth/forms... Thanks, Akihiro On Wed, Jan 29, 2020 at 6:27 AM Orestes Leal Rodríguez <olealrd1981@gmail.com> wrote:
I have found a way to solve it and give access to users that have passwords with spaces at the beginning/end. The issue (not an issue per se, but it affects horizon [stein]) lies in django. Specifically on 'django/forms/fields.py'
Horizon uses the fields and those by default remove spaces as stated, what I did is the following:
On that file, the class CharField's constructor was removing leading/trailing spaces: Below is the diff between the original and the modified python script (one line modified, strip=False)
--- fields.py.orig 2020-01-28 15:16:22.696047918 -0500 +++ fields.py 2020-01-28 15:16:45.520084974 -0500 @@ -220,7 +220,7 @@
class CharField(Field): - def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs): + def __init__(self, max_length=None, min_length=None, strip=False, empty_value='', *args, **kwargs): self.max_length = max_length self.min_length = min_length self.strip = strip
Now passwords are not altered by the underlying framework. Not sure the effect of not removing trailing/leading spaces from the textfields will have on the Horizon operations, though. Maybe horizon should redefine that django class to avoid this behavior. I'm also open to other solutions from the community. Have a great evening,
Thanks. Orestes
On 1/28/20, Orestes Leal Rodríguez <olealrd1981@gmail.com> wrote:
From the dashboard openstack is removing the trailing spaces from our user's passwords. We have a modified sql.py backend, that does an ldap bind to an active directory data store. And that works almost always. I say almost because for some users it doesn't work at all. We figure out (and a co-worker also confirmed this) that openstack is removing trailing (also leading?) spaces from the password entered in the dashboard. Also, inside the dashboard trailing spaces are not accepted even when they are equal byte by byte (including the space, I get an error). So this is going on.
Do anybody knows where is this removal performed? (python script location, line) So I can remove that since I have users (me included, I have the issue since the very beginning of this deployment) that cannot login. And they can use their Active Directrory passwords from other apps without problem.
We are running 'stein' with the latest update for ubuntu 18.04-AMD64.
participants (5)
-
Akihiro Motoki
-
Albert Braden
-
Orestes Leal Rodríguez
-
Radosław Piliszek
-
Sorin Sbarnea