nuancier needs to give an error message if the user is not in cla+1 group

[abadger]

Currently if the user is not in cla +1 nuancier redirects back to fas_openid which redirects immediately back to nuancier (as the user is already logged in) and so on in an infinite loop. I've changed the code to break that cycle and display an error message in the nuancier deployed in fedora infra. Instead of opening up a pull request I'm posting the patch below because the patch isn't very general. We probably need to change fas_login_required() to take a url to go to on error and a msg to display on error.

diff --git a/roles/nuancier/templates/__init__.py b/roles/nuancier/templates/__init__.py
index 2e9806b..3bd5942 100644
--- a/roles/nuancier/templates/__init__.py
+++ b/roles/nuancier/templates/__init__.py
@@ -88,11 +88,14 @@ def fas_login_required(function):
     """
     @wraps(function)
     def decorated_function(*args, **kwargs):
-        if flask.g.fas_user is None \
-                or not flask.g.fas_user.cla_done \
-                or len(flask.g.fas_user.groups) < 1:
+        if flask.g.fas_user is None:
             return flask.redirect(flask.url_for(
                 '.login', next=flask.request.url))
+        if not flask.g.fas_user.cla_done \
+                or len(flask.g.fas_user.groups) < 1:
+           flask.flash('You need to have signed the FPCA and be in one non-CLA'
+                    ' group to vote in this election', 'errors')
+            return flask.redirect(flask.url_for('index'))
         return function(*args, **kwargs)
     return decorated_function

@@ -103,15 +106,13 @@ def nuancier_admin_required(function):
     """
     @wraps(function)
     def decorated_function(*args, **kwargs):
-        if flask.g.fas_user is None or \
-                not flask.g.fas_user.cla_done or \
-                len(flask.g.fas_user.groups) < 1:
+        if flask.g.fas_user is None:
             return flask.redirect(flask.url_for('.login',
                                                 next=flask.request.url))
         elif not is_nuancier_admin():
             flask.flash('You are not an administrator of nuancier-lite',
                         'errors')
-            return flask.redirect(flask.url_for('msg'))
+            return flask.redirect(flask.url_for('index'))
         else:
             return function(*args, **kwargs)
     return decorated_function

[pypingou]

What do you think of the approach taken in fedocal? https://github.com/fedora-infra/fedocal/blob/master/fedocal/__init__.py#L64

Should we just re-use it?

[ralphbean]

Yeah, that looks good to me.

[abadger]

That doesn't work because fedocal is also broken. The problem is:

        if not valid:
            return flask.redirect(flask.url_for('auth_login',
                                                next=flask.request.url))

When that is used in conjunction with openid, the auth_login page redirects to fas_openid which sees that the user is already logged in and redirects back to the calling application's page which then performs the cla check again. The cla check still fails so it redirects to fas_openid again which redirects back to the app.

So we only want to redirect to auth_logn if the user is not logged in. If they're missing required groups (in this case, cla +1) then we need an error message (the flask.flash works for that) and then redirect to a page appropriate for whatever the user was trying to do. (note that I tried using flask.redirect('msg') at first but that ended up not working well with fas_openid. In some instances, hitting the back button on the 'msg' page took you back to fas_openid instead of back to the previous application page.