tiagocoutinho / multivisor

Centralized supervisor WebUI and CLI
GNU General Public License v3.0
189 stars 35 forks source link

Add login capabilities #8

Closed tiagocoutinho closed 5 years ago

tiagocoutinho commented 5 years ago

Add capability to optionally demand login access. Ideally with LDAP connection option.

guy881 commented 5 years ago

Hello @tiagocoutinho! Amazing project, thank you! I would like to help with this one.

tiagocoutinho commented 5 years ago

Great @guy881! Welcome to the project. Do you need any help? Feel free to make any suggestions. The only feature I would really like is to have optional LDAP connection

guy881 commented 5 years ago

Thank you @tiagocoutinho! Actually at first I thought about it, as just another web application authentication, but this one is somehow different, as there is no database. :) I guess we can store username and password in multivisor configuration file, similarly to supervisor. In supervisor.conf file password can be either a plain string or a SHA-1 hash. However SHA-1 is no longer secure, so maybe we should use SHA-256 or something else, what do you think?

If it comes to LDAP connection, to be honest, I have never implemented such a thing, so I might need a little help. How should it work? Some kind of checkbox next to the login / password inputs indicating that credentials should be checked by local LDAP server running on the host?

tiagocoutinho commented 5 years ago

Good suggestion.

LDAP might be more complicated. Since you don't need it yourself, I propose you implement the simple supervisor like login capability.

We can make another issue in the future to add LDAP.

kontact-chan commented 5 years ago

To start with simple multivisor configuration file based user/password is good enough. Same way supervisor does. Keeping the dashboard password protected is a necessary feature, since the dashboard has stop/restart capability.

guy881 commented 5 years ago

@tiagocoutinho, let me ask for your advice. I implemented quite simple authentication based on flask sessions, but now I wonder whether it was a good idea. Right now I am using local storage to keep isAuthenticated flag, I use it to disable all the pages except login in client app and to show "logout" option in menu. Because I am using local storage, this flag can easily be modified in the browser. I mean there won't be a big tragedy if someone alters it, because API will still throw 401 before successful authentication, so there won't be any data available in dashboard. But maybe using a JWT token auth would be a better idea? There is a package called Flask-JWT for that. That token would be sent with every API request and we could logout user if backend returns 401. But I guess this would involve replacing browser fetchAPI which is currently used with axios to add credentials to every request. Right now it's done by browser cookies mechanism.

guy881 commented 5 years ago

@tiagocoutinho, I've made a pull-request, I am asking you for a review. :) In the end, I haven't used local storage for isAuthenthicated flag. Instead I am getting this data from backend on a new endpoint /api/auth and I am still using flask session.

anthosz commented 5 years ago

Hi,

Can you support also CAS or github auth by example?

Best regards,

kontact-chan commented 5 years ago

fantastic @guy881 . Get the login merged. I would be first person to test this in our production environment.

tiagocoutinho commented 5 years ago

Issue solved by PR #28. I will close this

anthosz commented 5 years ago

Dear @tiagocoutinho ,

When it will be merged into master (next release)?

Best regards,