Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple errors while using OpenID #1374

Open
trendspotter opened this issue Sep 12, 2017 · 6 comments
Open

Multiple errors while using OpenID #1374

trendspotter opened this issue Sep 12, 2017 · 6 comments

Comments

@trendspotter
Copy link
Contributor

There are several problems while enabling OpenID login method.

Simply enabling settings.auth.openid displays following error on main page http://127.0.0.1/eden/default/index. Strangely enough, the same error is not showing on http://127.0.0.1/eden/default/user/login.

Traceback (most recent call last):
  File "/srv/sahana/gluon/restricted.py", line 227, in restricted
    exec ccode in environment
  File "/srv/sahana/applications/eden/controllers/default.py", line 1547, in <module>
  File "/srv/sahana/gluon/globals.py", line 417, in <lambda>
    self._caller = lambda f: f()
  File "/srv/sahana/applications/eden/controllers/default.py", line 298, in index
    login_form = auth.login(inline=True)
  File "applications/eden/modules/s3/s3aaa.py", line 736, in login
    cas_user = cas.get_user()
  File "/srv/sahana/gluon/contrib/login_methods/extended_login_form.py", line 54, in get_user
    return self.alt_login_form.get_user()
  File "/srv/sahana/gluon/contrib/login_methods/openid_auth.py", line 178, in get_user
    if args[0] == 'logout':
IndexError: list index out of range

This may actually be an error in Web2py framework. Hotfixing the file gluon/contrib/login_methods/openid_auth.py as follows seems to circumvent the problem.

--- a/gluon/contrib/login_methods/openid_auth.py
+++ b/gluon/contrib/login_methods/openid_auth.py
@@ -175,7 +175,7 @@ class OpenIDAuth(object):
         request = current.request
         args = request.args
 
-        if args[0] == 'logout':
+        if args and args[0] == 'logout':
             return True  # Let logout_url got called
 
         if current.session.w2popenid:

Once the file is adjusted and Eden restarted, a new error occurs. However this error occurs only if the user does not have a registered cookie set.

Traceback (most recent call last):
  File "/srv/sahana/gluon/restricted.py", line 227, in restricted
    exec ccode in environment
  File "/srv/sahana/applications/eden/views/default/index.html", line 614, in <module>
AttributeError: 'DIV' object has no attribute 'errors'

This suggests some type confusion in variable login_form in views/default/index.html or its controller controllers/default.py.

@nursix
Copy link
Member

nursix commented Sep 12, 2017

Actually, it's inconsistent of openid_auth.py to return a DIV rather than a FORM when requesting a login_form() - this can't be expected.

And I'm not sure whether catching this in the default controller before checking for form.errors would actually help - but it may be a workaround for /this/ problem.

However, I'm not aware of an active use-case (which template uses openID?).

@trendspotter
Copy link
Contributor Author

No template yet. We're evaluating Sahana Eden for possible deployments on our use cases. OpenID is one of the features which would be nice to have, as it seemed that it's already implemented.

@nursix
Copy link
Member

nursix commented Sep 13, 2017

Well, as it seems, the framework for that exists - but the use-case isn't currently part of the default template (maybe it never was).

That's actually the case with many features - they may be (or have been) part of specific templates, and wouldn't work in others. This one seems useful for default, though, and should thus be fixed for it.

However, you may eventually want to develop a specific template for your case - and then only fix the problem there. If/when you do, then maybe remember this report of yours, and port your fix to default ;)

@nursix
Copy link
Member

nursix commented Sep 13, 2017

I should put this right:

Many of the config switches (particularly of framework-level features), require the respective template to support them - but most templates only expose/support a subset of the functionality. In most templates, therefore, switches can have adverse side-effects or no effect at all.

Surely "default" is meant to be the "kitchen sink", and expose sort-of everything - but then again it doesn't because some of the features actually conflict with each other (so it would only support the most common case), and other features are untested and may indeed not work (and are therefore not be exposed by default). For exactly such reasons, a number of modules is disabled by default.

For your evaluation that means that everything you get in /any/ template without modifying its config switches is what is supposed to work in this template - and everything else /could/ work, but requires testing and maybe some hands-on to make it work.

Also take into account that the default template is currently one of the least maintained ones - simply because nobody is actually using it in a deployment - so that's a natural effect of us focusing on specific cases much more than the "kitchen sink" (in fact, it may be that nobody has been working on the default template for the past 2-3 years).

The best strategy for an evaluation for deployment may be to discuss your case on the mailing list. That way you can get a much better idea of how much effort it would be to support your requirements.

@hallamoore
Copy link
Contributor

@nursix, given your explanation above, should this issue be closed? Or is there still a bug to fix in the default template?

Currently, enabling settings.auth.openid with the default template shows a banner with "Library support not available for OpenID", which is much more user friendly than the previously reported error. Though I guess it still doesn't really give any direction on how to get support for OpenID.

@nursix
Copy link
Member

nursix commented Dec 10, 2019

I think the point is that an implementation of OpenID support had once been started, but never really completed let alone maintained - so you could call it an "experimental feature".

The direction here would be to make it ready for production, so that it can indeed simply be switched on and off for a template.

Note though that the requirement has not come up again since this issue was first reported, so there was and is very little incentive for anyone to work on it. Given the questionable relevance of an issue report that has not been followed up and not seen any responses for two years, I would thus indeed be inclined to close it without further action - but then again, maybe you want to fix it simply as an exercise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants