Django 1.10.x breaks WEBSSO login form on firefox

[Colleen Murphy]

When using horizon's WebSSO feature and selecting an external IdP for the "Authenticate Using" field, clicking the Connect button produces a popup box in the upper left corner saying "Please fill out this field" instead of forwarding to the keystone IdP. This only happens on Firefox (tried with 52.2.0), it works fine in Chrome. The problem goes away when Django is downgraded from 1.10.x to 1.9.13. The problem also goes away when the "required" attribute of these input fields are deleted using the Inspector tool in firefox:

<input autofocus="autofocus" class="form-control" id="id_username" name="username" required="" type="text">
<input class="form-control" id="id_password" name="password" required="" type="password">

Steps to reproduce:

In local_settings.py, set:

WEBSSO_ENABLED = True
WEBSSO_CHOICES = (("mapped", _("Mapped")),)

Open the dashboard in Firefox and click "Connect".

Expected result:

Horizon redirects to /identity/v3/auth/OS-FEDERATION/websso/mapped and produces a 401 because keystone isn't set up for federation.

Actual result:

Horizon produces a small box in the upper left that says "Please fill out this field" with a little arrow that seems like it's intending to point to an empty field but is pointing to nothing in particular.

[Colleen Murphy (krinkle)]

Update - the old behavior that I'm used to, where selecting a websso option from the "Authenticate using" dropdown made the username and password fields disappear (since they're not applicable any more), is gone since https://review.openstack.org/#/c/477447/ was introduced. So now those fields stick around. With Django 1.10.x and 1.11.x they must be filled in, even with dummy values, in order to get past the login screen to the keystone redirect (for both chrome and firefox). On Django 1.9.13 it will let me click through without filling in the fields.

[Jouke (jouker)]

@krinkle The fields not disappearing has been fixed now (https://bugs.launchpad.net/horizon/+bug/1708671).

You mentioned you did get redirected on Chrome, but this isn't the case for me (Chrome 59). Could you check if this still works for you with the latest version on git? Firefox seems to silently fail for me.

Specifically, I get the following errors in console on Chrome:
An invalid form control with name='username' is not focusable.
An invalid form control with name='password' is not focusable.

[Colleen Murphy (krinkle)]

Jouke: With the latest version I now see the behavior you described in Chrome. On firefox (52.2.1) it still gives me a little indicator box pointing at nothing and no messages in the console.

[Jouke (jouker)]

I must add that, while quite rough around the edges, the browser blocking the request due to required fields being empty is correct in my opinion. A field being invisible does not simply mean it is no longer required.

As far as I can see, there are three possible fixes for this problem:
1) Wrap the user/pass/domain/region in an ng-if block
2) Kick out html's required and use ng-required
3) Toggle the required attribute when showing and hiding the element

While not the most beautiful of the three, I have implemented #3 as a two-liner and confirmed it works on Chrome, FF, IE and Edge (although I don't know if it was ever an issue on IE and Edge).
If joining as a developer is not too much work I'll create a review tomorrow, otherwise I might just add it here.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/491743

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/492602

[OpenStack Infra (hudson-openstack)]

Change abandoned by Jouke (<email address hidden>) on branch: master
Review: https://review.openstack.org/492602

[Rob Cresswell (robcresswell-deactivatedaccount)]

I think this is the same as https://bugs.launchpad.net/horizon/+bug/1708601; could you check with that patch and see if it fixes your issues?

[Jouke (jouker)]

> I think this is the same as https://bugs.launchpad.net/horizon/+bug/1708601; could you check with that patch and see if it fixes your issues?

Hi Rob, that patch works in the same way as my fix you rejected earlier, but on a higher level. Colleen reported that did not work everywhere.

> Fixes the problem for me on firefox, though the issue you pointed out for chrome is still there.

[Colleen Murphy (krinkle)]

The proposed fix for #1708601 fixes neither problem for me, not the one I originally reported with Firefox nor the new one that arose later with Chrome.

[Jouke (jouker)]

Is it safe to assume that, given the other bug's importance, this bug could also be marked critical?
In my opinion it would make sense, given that it is currently impossible to login if 'credentials' is not used at all (so only using federation) without hacking in the html, and requires a very unnatural action if credentials is available, i.e. entering fake credentials and only then switching to federation.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/516439

[OpenStack Infra (hudson-openstack)]

Change abandoned by Colleen Murphy (<email address hidden>) on branch: master
Review: https://review.openstack.org/516439
Reason: Abandoning in favor of https://review.openstack.org/#/c/517382/

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/519357

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/519357
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=63d71468824031ae40bbf34bb379b42a98e2ca94
Submitter: Zuul
Branch: master

commit 63d71468824031ae40bbf34bb379b42a98e2ca94
Author: Colleen Murphy <email address hidden>
Date: Thu Nov 2 18:19:09 2017 +0100

    Don't add required attribute to html form fields

    (cherry picked from commit 1fa9ae26cc6006f8ee94fabddb7fea171adee55d
     in django_openstack_auth)

    In Django 1.10 a new Form property was introduced, defaulting to True,
    which enabled HTML form validation for fields marked "required" in
    Django. This changed old behavior, which was that required fields were
    only validated server-side. This patch restores old behavior by setting
    use_required_attribute to False for the inherited AuthenticationForm.

    This problem arose because when WebSSO is enabled and a
    non-keystone-credentials authentication method is selected from the
    dropdown list, the now-hidden username and password fields are still
    marked "required" and still validated client-side, even though they are
    invisible to the user and cannot be filled in. It would be nice to fix
    the javascript to properly turn the "required" attribute on or off
    depending on what authentication method is selected and whether the
    "required" fields are even visible, but for now this just restores the
    behavior we had before Djanto 1.10.

    Change-Id: I3e798a2288d9c33396b40a86b07ea8c163d3b525
    Closes-bug: #1703109

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 13.0.0.0b2 development milestone.

[Akihiro Motoki (amotoki)]

Cherry-pick from django-openstack-auth accidentally closed this bug. We still need a fix on horizon side (non openstack_auth).

[Akihiro Motoki (amotoki)]

We need double-check whether we need more work or a patch mentioned in #15 fixed the bug completely.

[Akihiro Motoki (amotoki)]

After the patch linked in #15 is applied, 'required' attribute does not exist in the password field. I think the bug is fixed by the patch above.

[Akihiro Motoki (amotoki)]

I added django_openstack_auth to an affected project to handle a backport to pike.
(In the master branch, it is marked as "Fix Released" because the fix was merged before the project retirement.)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/534409
Committed: https://git.openstack.org/cgit/openstack/django_openstack_auth/commit/?id=04491deed11022c29e24a02e69d750e981a7ac7a
Submitter: Zuul
Branch: stable/pike

commit 04491deed11022c29e24a02e69d750e981a7ac7a
Author: Colleen Murphy <email address hidden>
Date: Thu Nov 2 18:19:09 2017 +0100

    Don't add required attribute to html form fields

    In Django 1.10 a new Form property was introduced, defaulting to True,
    which enabled HTML form validation for fields marked "required" in
    Django. This changed old behavior, which was that required fields were
    only validated server-side. This patch restores old behavior by setting
    use_required_attribute to False for the inherited AuthenticationForm.

    This problem arose because when WebSSO is enabled and a
    non-keystone-credentials authentication method is selected from the
    dropdown list, the now-hidden username and password fields are still
    marked "required" and still validated client-side, even though they are
    invisible to the user and cannot be filled in. It would be nice to fix
    the javascript to properly turn the "required" attribute on or off
    depending on what authentication method is selected and whether the
    "required" fields are even visible, but for now this just restores the
    behavior we had before Djanto 1.10.

    Change-Id: I3e798a2288d9c33396b40a86b07ea8c163d3b525
    Closes-bug: #1703109
    (cherry picked from commit 1fa9ae26cc6006f8ee94fabddb7fea171adee55d)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/django_openstack_auth 3.6.0 release.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Ivan Kolodyazhny (<email address hidden>) on branch: master
Review: https://review.openstack.org/491743
Reason: This review is > 4 months without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.