Project

General

Profile

Development #11324

Enhance registration frontend management.

Added by Mikaël Ates over 3 years ago. Updated almost 3 years ago.

Status:
Fermé
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
13 Jun 2016
Due date:
% Done:

100%

Patch proposed:
Yes
Planning:
No

Description

The proposition breaks the compatibility with plugins but as far as I know, registration frontend management is for now only used with the FC plugin.

  • Grab frontend from the plugin frontend class and not the plugin itself.
  • Provide the templates with information on the frontend and not only the rendered html.
  • Update registration_form template

The blocks building is largely inspired form the login frontend management.

Give more info to the template to freely define title is better than defining titles with a fixed level directly in the plugin.

0001-Enhance-get-backends-helper-function-and-registratio.patch View (10.9 KB) Mikaël Ates, 15 Jun 2016 09:34 AM


Related issues

Related to Plugin FS FranceConnect - Development #11351: Déplacer le frontend registration du plugin vers la classe Frontend Fermé 14 Jun 2016

Associated revisions

Revision c237f561 (diff)
Added by Mikaël Ates over 3 years ago

Enhance get backends helper function and registration frontend management (fixes #11324).

Revision 12aaadd2 (diff)
Added by Frédéric Péters over 3 years ago

update Frontend class to match recent authentic changes

commit c237f561bd866bb261ea2f082c91a4b607a8284a
Author: Mikaël Ates <>
Date: Mon Jun 13 16:50:45 2016 +0200

Enhance get backends helper function and registration frontend management
(fixes #11324).

Revision c99973df (diff)
Added by Benjamin Dauvergne about 3 years ago

make SAMLFrontend.id a simple string (fixes #13339)

It's broken since c237f561bd866bb261ea2f082c91a4b607a8284a and fix for #11324.

History

#1 Updated by Mikaël Ates over 3 years ago

  • Related to Development #11351: Déplacer le frontend registration du plugin vers la classe Frontend added

#2 Updated by Mikaël Ates over 3 years ago

Seul le thème montpellier utilise un thème différent pour registration_form.html et utilise les frontaux d'enregistrements des plugins. Le patch serait le suivant :

diff --git a/templates/registration/registration_form.html b/templates/registration/registration_form.html
index 33dccf3..06b6868 100644
--- a/templates/registration/registration_form.html
+++ b/templates/registration/registration_form.html
@@ -11,13 +11,13 @@
 </div>

 <div class="right">
-{% include 'authentic2/form.html' with form=form %}
-{% for registration_frontend in registration_frontends %}
+  {% include 'authentic2/form.html' with form=form %}
+  {% if frontends.fc %}
     <div class="textbreak"><span>OU</span></div>
-    {{ registration_frontend|safe }}
-{% endfor %}
+    <h2>{{ frontends.fc.name }}</h2>
+    {{ frontends.fc.content|safe }}
+  {% endif %}
 </div>

 <br class="clear"/>
-
 {% endblock %}
(END)

#3 Updated by Frédéric Péters over 3 years ago

The proposition breaks the compatibility with plugins but as far as I know, registration frontend management is for now only used with the FC plugin.

It's also used for the belgian eid authentication (https://git.entrouvert.org/authentic2-auth-fedict.git/).

#4 Updated by Frédéric Péters over 3 years ago

{% for id, block in frontends.items %}
    <br/>
    <h2>{{ block.name }}</h2>
    {{ block.content|safe }}
{% endfor %}

I wouldn't add <br> to the markup, but I would enclose title/content in a <div class="whatever">.

             if hasattr(frontend.name, '__call__'):

Use callable(). ?

            blocks.append({
                    'id': frontend.id(),
                    'name': frontend.name,
                    'content': response.content,
                    'frontend': frontend,
            })

Reading get_backends it allow .id not to be a callable, this is lost here. Maybe get_backends() could be changed to add a new attribute with the appropriate id instead of relying on callers to duplicate the work?

            response = frontend.registration(self.request, context_instance=request_context)
            if not response or response.status_code != 200:

I understand the convenience of shortcut functions (like django.shortcuts.render) but this feels wrong to rely on HttpResponse here.

#5 Updated by Mikaël Ates over 3 years ago

  • File 0001-Enhance-get-backends-helper-function-and-registratio.patch added
The patch is now mainly about enhancing get_backends helper function.
  • name and id attributes or methods of backends are now string attributes.
  • only enabled backends are returned. If no enabled function is defined, it is enabled by default.
  • finnaly, get_backends orders by priority.

The helper function get_backend_method is added to build a dic around the frontend method called.

Note that it is accepted that some login method return a http response with a status code different from 200, 302 for the LoginPasswordBackend for instance. If a frontend returns such a response, that http response is returned by the login view (it was the case before this patch).

<br> removed.

callable() used instead of testing call attribute.

#6 Updated by Mikaël Ates over 3 years ago

  • File deleted (0001-Enhance-registration-frontend-management.patch)

#7 Updated by Mikaël Ates over 3 years ago

Concerning the belgian eid authentication plugin there is no concern since no registration_frontend method is defined on the plugin.

#8 Updated by Mikaël Ates over 3 years ago

  • File 0001-Enhance-get-backends-helper-function-and-registratio.patch added

#9 Updated by Mikaël Ates over 3 years ago

  • File deleted (0001-Enhance-get-backends-helper-function-and-registratio.patch)

#10 Updated by Mikaël Ates over 3 years ago

  • File 0001-Enhance-get-backends-helper-function-and-registratio.patch added

#11 Updated by Mikaël Ates over 3 years ago

  • File deleted (0001-Enhance-get-backends-helper-function-and-registratio.patch)

#13 Updated by Mikaël Ates over 3 years ago

  • File deleted (0001-Enhance-get-backends-helper-function-and-registratio.patch)

#14 Updated by Benjamin Dauvergne over 3 years ago

Ack.

#15 Updated by Mikaël Ates over 3 years ago

  • Status changed from Nouveau to Résolu (à déployer)
  • % Done changed from 0 to 100

#16 Updated by Frédéric Péters over 3 years ago

And... it broke the authentic2-auth-fedict.

#17 Updated by Mikaël Ates over 3 years ago

  • Status changed from Résolu (à déployer) to En cours

#18 Updated by Frédéric Péters over 3 years ago

  • Status changed from En cours to Résolu (à déployer)

Nope, it's fine, I updated the plugin to fit the change.

#19 Updated by Mikaël Ates over 3 years ago

Defining the Frontend id with an attribute or a method should not be a problem. Could you explain a little bit more ?

#20 Updated by Mikaël Ates over 3 years ago

Hum, I see it.

#21 Updated by Mikaël Ates over 3 years ago

I extended the practice

if hasattr(frontend.name, '__call__'):
    frontend.name = frontend.name()

to the id attribute, what broke authentic2-auth-fedict since id was defined as a method and used as it in the plugin, but redefined as not being callable by get_backends().
name and id should stay callable if they are defined callable in the plugin to avoid issues like this.
So It was given a partial answer to

Maybe get_backends() could be changed to add a new attribute with the appropriate id instead of relying on callers to duplicate the work?

So we could go back and let callers check those attributes. Or we could move plugins towards id and name attributes not being callable and modify them as it was done for authentic2-auth-fedict. Finally, we could go towards id and name being only callable.

#22 Updated by Benjamin Dauvergne almost 3 years ago

  • Status changed from Résolu (à déployer) to Fermé

Also available in: Atom PDF