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

Integrate ep_hash_auth #3463

Closed
muxator opened this issue Aug 18, 2018 · 11 comments
Closed

Integrate ep_hash_auth #3463

muxator opened this issue Aug 18, 2018 · 11 comments

Comments

@muxator
Copy link
Contributor

muxator commented Aug 18, 2018

The plan for integrating ep_hash_auth into Etherpad was discussed as part of #1650 (see also #3442, #3444, turnkeylinux/tracker#1132).

In ether/ep_hash_auth#4 its author agreed to donate its code for allowing the integration. This ticket tracks this work.

@ukcb
Copy link

ukcb commented Aug 19, 2018

ep_hash_auth needs bcrypt. bcrypt needs a g++ compiler for installation. Will ep_hash_auth also work correctly without bcrypt? You do not really want to have a compiler on the server.

For me, I still see the problem that ep_hash_auth does not work with ep_mypads.

https://framagit.org/framasoft/Etherpad/ep_mypads/issues/88

@muxator
Copy link
Contributor Author

muxator commented Aug 19, 2018

Hi @ukcb, thanks for your remarks.

Let me answer separately on your points.

  1. Necessity of g++ for installing bcrypt.

    Preliminary note: glad you brought this up. I see this as a packaging problem, and a real one: installing Etherpad on a production machine is not different than having a development copy on one's desktop: git clone, run a script that downloads dependencies from the Internet and creates a symlink to ../src -> ep_etherpad-lite in node_modules, and you are all set. This is very convenient, but maybe for "serious" server installs there should be a better way.

    Going back to your question: bcrypt offers binary releases on a best effort basis. The install procedure tries to downlad this pre-built library for supported architectures:

    > node-pre-gyp install --fallback-to-build
    [bcrypt] Success: "/tmp/prova-bcrypt/node_modules/bcrypt/lib/binding/bcrypt_lib.node" is installed via remote

    According to the installation instructions the architectures for which a pre-built library are currently offered are Windows, Linux and MacOS. If the binary lib cannot be downloaded, or is incompatible, it resorts to building from source. This is the part where you need a compiler, and the one for which I feel we could benefit from a packaging phase.

    Thinking of what could be done now, maybe we could think about making the bcrypt dependency optional, and handle incompatibilities programmatically (e.g.: fail giving instructions if settings.json requires hashes and bcrypt was not installed, etc).

    While far from optimal, maybe this could be a possible way forward?

  2. ep_hash_auth incompatibility with ep_mypads in their current state.

    I was not thinking about making password encryption mandatory (that would be a sudden compatibility break).
    The plan is to faciliate as much as possible the use of hashed passwords for the users that want it. This would involve:

    • getting rid of the necessity of installing a plugin
    • better documentation
    • maybe a native nodejs script for generating hashes (without resorting to python).

    In this initial scenario, a user wanting to use ep_mypads would still be forced to use clear text passwords. This does not exclude that a subsequent development work could be made to make ep_mypads and password hashing compatible, possibly creating new integration points in Etherpad. I do not plan to directly look at it now, which means that observations and proposals would be more than welcome.

Thanks again for your observations, and please tell me if you think this approach could make sense.

@ukcb
Copy link

ukcb commented Oct 5, 2018

Etherpad could optionally create an extra table for administrators in the database. My_Pads could then optionally use this for his administration. You both need to discuss this. It would be good if there was exactly one interface for it.

@JedMeister
Copy link

@muxator - IMO the long game should be to support the whole user generation/management process (including setting and hashing passwords) a part of the admin UI.

Obviously that will require a fair bit of work and is likely not on your immediate roadmap, so is not a legitimate short term goal IMO. However, it's well worth deciding on the long term goal before any of this is implemented IMO. Then you can plan for the long term goals from the get go (even if little of it is actually implemented). I.e. start with the ends in mind. That won't completely eliminate the need to rewrite sections of code later, but it will reduce it, and hopefully make it an easier process.

@ukcb - I really like your idea of using the db for user account storage (as opposed to the settings file). Personally, I think it's a good idea for all users (not just admin users). There is precedent for that sort of setup in lots of other web apps.

Having said that, I understand that Etherpad may be run without a proper DB backend (that's right isn't it?). If you were to go that way, you may need to make a requirement for a SQL db (although SQLite might be a good option for those who don't want to run a "proper" DB?).

Just adding my 2c to the mix - actually it's probably cheaper than that because I don't need to implement any of this... 😄

@ukcb
Copy link

ukcb commented Oct 8, 2018

@JedMeister Etherpad always needs a database. SQLite is also SQL. I myself use postgrespool. It would be conceivable to say look into the database if you can find the admin account or look in settings.json. Plugins then have to deal with it, and adjust if necessary.

@JedMeister
Copy link

@ukcb - Ok thanks for the clarification. I was under the impression that if you didn't use "a proper DB" (e.g. MySQL etc), that Etherpad just used some sort of (non standard) plain text data storage mechanism (i.e. not SQL at all). I admit that I'm not very familiar with the Etherpad codebase and have only run it with a MySQL/MariaDB backend.

I agree that SQLite is a SQL DB. I used the quotes around proper (i.e. "proper" database) because it's generally not considered adequate for production usage of scalable web apps because of it's inherent design limitations. Having said that, I'm sure for many low-to-medium traffic pads, it'd be fine.

@muxator muxator modified the milestones: 1.7.5, 1.8 Feb 5, 2019
@muxator muxator modified the milestones: 1.8.0, 1.8.1 Dec 7, 2019
@muxator
Copy link
Contributor Author

muxator commented Dec 17, 2019

Related: #3681

@muxator
Copy link
Contributor Author

muxator commented Mar 23, 2020

@JohnMcLear, could you allocate some of your bandwidth on this?

@JohnMcLear JohnMcLear self-assigned this Mar 23, 2020
@JohnMcLear
Copy link
Member

JohnMcLear commented Mar 23, 2020

Okay my thoughts on the conversation. Etherpad shouldn't bring in any further authentication features for user control / management. So with that in mind I respectfully disagree with @JedMeister

But keep reading..

Our skill / experience and passion is in building the editor and integration with various plugins / tools etc. Our skill is not providing a security authentication mechanism or alternative to active directory. Various other services do this (Think Auth0) and they do it very well. We should only ever consume those services for any Secure deployed at scale Etherpad instance. Passport is a great example of a project that should/could be integrated into Etherpad (through a plugin) and would consume hundreds(or is it more now?) authentication service providers, we could and would not want to get involved in trying to duplicate that effort.

So do I think hash of admin password is important? Yes. But any dev/admin putting Etherpad in production where they will be integrating into a larger environment is going to be managing multiple instances with something like docker and be familiar with Python tools. For me, as an example, I close off the /admin front end because I do all my instance management from the CLI and using the scripts provided in bin/

So I think the scope of this should be very close to what @LaKing did, in fact in my opinion I think the plugin approach is completely fine as it provides a great example for others to develop similar auth mechanisms (think certificates as per the comments).

That said, the turnkeylinux project are very against passwords in plain text (and rightly so) so this does leave us with some important considerations.. On one hand, we need to be able to empower plugin developers and provide a flexible framework. On the other hand our service consumers want us to provide security out of the box.

I think I have a compromise:

Important point: Currently by default ALL admin users are disabled so an admin has to comment out the admin section to enable /admin..

Why don't we just have a huge disclaimer for the password section saying, "doing it this way is okay if you are just quickly throwing up a site for your local LAN hack event but if you want a server hitting the Internet you should use ep_hash_auth"...

I don't think that's beyond the effort of someone wanting to enable a few plugins.. Hell shit, they might even enable a few plugins then just comment back out the auth section and never care about it again.. Bringing in loads of complexity for someone a site admin does once in a blue moon, for me, is just burdening a project and reducing it's ability to deliver it's core objectives to it's users.

@JohnMcLear
Copy link
Member

@muxator please share thoughts.

@JohnMcLear
Copy link
Member

Closing as the big warning is in and it appears ep_hash_auth has more users.

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

4 participants