Refactor the admin architecture and configuration management #670
Conversation
kaiyou
added
the
status/wip
|
Just a suggestion. At this moment we are using We could use the setup utility to generate a password, store it encrypted in This way, there can be no mismatch in |
|
Sorry it took me so long to finish this up, I was struggling with sqlalchemy contexts when mixed with flask login. This is now ready for merge I believe. |
kaiyou
removed
the
status/wip
Fixes the following error: ``` admin_1 | [2018-11-09 09:44:10,533] ERROR in app: Exception on /internal/auth/email [GET] admin_1 | Traceback (most recent call last): admin_1 | File "/usr/lib/python3.6/site-packages/flask/app.py", line 2292, in wsgi_app admin_1 | response = self.full_dispatch_request() admin_1 | File "/usr/lib/python3.6/site-packages/flask/app.py", line 1815, in full_dispatch_request admin_1 | rv = self.handle_user_exception(e) admin_1 | File "/usr/lib/python3.6/site-packages/flask/app.py", line 1718, in handle_user_exception admin_1 | reraise(exc_type, exc_value, tb) admin_1 | File "/usr/lib/python3.6/site-packages/flask/_compat.py", line 35, in reraise admin_1 | raise value admin_1 | File "/usr/lib/python3.6/site-packages/flask/app.py", line 1813, in full_dispatch_request admin_1 | rv = self.dispatch_request() admin_1 | File "/usr/lib/python3.6/site-packages/flask/app.py", line 1799, in dispatch_request admin_1 | return self.view_functions[rule.endpoint](**req.view_args) admin_1 | File "/usr/lib/python3.6/site-packages/flask_limiter/extension.py", line 544, in __inner admin_1 | return obj(*a, **k) admin_1 | File "/app/mailu/internal/views/auth.py", line 18, in nginx_authentication admin_1 | headers = nginx.handle_authentication(flask.request.headers) admin_1 | File "/app/mailu/internal/nginx.py", line 48, in handle_authentication admin_1 | if user.check_password(password): admin_1 | File "/app/mailu/models.py", line 333, in check_password admin_1 | context = User.pw_context admin_1 | AttributeError: type object 'User' has no attribute 'pw_context' ```
There was a problem hiding this comment.
During our development of Postgresql, we found that login to the admin interface does not always work. We've downgraded to this branch and managed to reproduce the behavior. On Master everything seems fine.
Issue
Sometimes, when trying to log-in, the log-in page gets displayed again. No error is reported in the UI or container logs, pages seem to get served as intented (from the app point of view). When intentionally entering a wrong password, we do get an "wrong password" error served in UI.
Steps to reproduce
- Build and run this branch
- Log in to admin (this should work)
- Log out and try to log in again
You might need to repeat step 3 a couple of times to expose this. It seems when waiting a longer period between log-out and log-in, this does not expose. Try to keep the time as short as possible.
My gut tells me something (cookie, session) remains sticky on log-out and therefore the log-in page gets server over and over.
This happens on this branch and our usrpro/feat-psql-support, using both the SQlite and Postgresql database flavor. It might be very interesting to know, that looking at the postresql logs, no log-in queries are executed in cases where it didn't work. The only query that does get executed, is the one that lists the sign-up domains.
|
This seems to be chrom(ium) specific... When sending the login from chromium no redirect: From firefox: To be continued... |
|
After doing some I will drop my blocking review and go back to approving it. |
|
Some more checking on my side and I think we can safely merge this. |
Until now, the application was entirely generated by the init script, and configuration was only provided as environment variables.
This PR suggests generating the application in a function (namely
create_app) and writing everything else as Flask extensions. This clarifies a lot of code inside the admin container, and makes some room for further improvement.Among those improvements, the configuration is replaced in this version with a simple class that mimics the behavior of previous Flask configuration updated from env variables. It should evolve to the point where configuration can be modified from the Web interface, at least for items that do not impact the other containers.
Currently this builds and run but I still have an issue with the way Flask login interacts with Flask SQLAlchemy, as it does not seem to be using the application context properly, and thus fails when requesting SQLite which is not thread-safe.