Support for LDAP server authentication

[alecdwm]

Hello!
I have here a proposal PR for enabling LDAP authentication via the vesse/passport-ldapauth library.

This PR is not quite ready to be merged. The LDAP authentication process is functional, but I want to open a channel for discussion regarding the following items before I continue to finish it up:

1. Optional tlsOptions for LDAP auth
The vesse/passport-ldapauth lib allows us to connect to TLS-enabled LDAP servers by specifying a tlsOptions object in our config.
Possible parameters are the same as those offered by the Node.js tls module.
Some of these parameters take values that are not specifiable on the command line, such as socket <net.Socket>, secureContext <object> and session <Buffer>.

I propose to make only the ca <string> TLS option available as an environment variable, which will take the root certificate used to generate the LDAP server's TLS cert as a string in PEM format.

2. Generating access/refresh tokens
HackMD uses token based authentication. I didn't look very deep, but it seems that so far we've been able to consume the tokens generated by OAuth, and haven't needed to manage token generation/expiry ourselves.

Unfortunately though, LDAP authentication involves sending a username|password combination and receiving either a success message with user data, or an error message.
If one wants to use token based auth, it's left to the client.

In this commit, some very simple tokens are generated in the following format:
debug-[access/refresh]-token|LDAP-[ldap-uidNumber]|[secretkey]|[timestamp]
e.g. debug-access-token|LDAP-1892|pommes|1481665157094

This is not a secure solution as the tokens are neither hashed nor encrypted, and there is (as far as I am aware) no mechanism to expire them.

I am very open to hear if anyone has a preferred structure for the tokens, how they should be secured, how they should be expired, and of course whether I'm missing something and we don't need to create tokens at all! 😁

3. Other notes

• authentication failures are not yet gracefully handled by the UI (I intend to fix this shortly)
  • instead the error message is shown on a blank page (/auth/ldap)
• no email address is associated with an LDAP user's account
  (my test server doesn't store LDAP users' emails, I'm not sure if that's standard or not)
• no picture/profile URL is associated with an LDAP user's account (same as emails)
• 'LDAP Sign in' needs to be translated to each locale

Relevant Issues
#93
#272

[jackycute]

Hi @alecdwm
Big thanks to propose this PR!
I want to give you some suggestions about some problems you've faced.

1. It's enough to save only the ca part, and leave the tlsOption to config or environment variables.
2. Actually we are not token based auth, since the email function works the accessToken and refreshToken are not necessary fields.

• We've added some flash message to the index on implementing the email sigin feature, you could use that too https://github.com/hackmdio/hackmd/blob/master/app.js#L420
• I also not know if we should store email on LDAP auth or not, but we will need it for further usage (might ask user to provide later).
• It's ok not having email associate with account, we could use the capital of username as their avatar.

[alecdwm]

Thanks for the feedback! 😁

Open points:

• We no longer need to translate 'LDAP Sign in', the separate modal this text was on has been merged into the main signin modal.
• Email addresses are still an issue, but it seems that active directory can keep a record of email addresses.
  I will look further into this, but alternatively we can try your idea to ask for an email after logging in, or even fall back to this if the LDAP server doesn't have an email on file.
• Profile pictures are also still an issue

[almereyda]

To the latest point: if you have an email address available, there are still gravatar or libravatar.

[neopostmodern]

If my understanding of LDAP is correct, it's always bound to an identity provider, right?
I'd therefore like to suggest the following things:

• pass a name for that identity provider (in config) and display Sign in via YOUR PROVIDER HERE (LDAP) instead of Sign in via LDAP because the average user won't get what you mean otherwise, plus you get a notion of who you're authenticating against
• allow configuring multiple LDAP providers (definitely requires above)

These don't need to be included in a first version. All in all, 🎉 for LDAP-support!

[alecdwm]

Profile pictures are good to go!
If the LDAP server specifies an email, we use Gravatar. Otherwise, we generate a letter avatar:

The LDAP username is used to seed a RNG, which then selects a background colour from of a list of possible colours that I stole from here (alphabetcolors).

Next the first letter is stripped off, uppercased and used along with the selected colour to format an SVG.

Finally, the SVG (stored in memory as a UTF8-encoded string) is encoded into base64 with the prefix data:image/svg+xml;base64,.

Support for embedding the generated image into an img tag looks comprehensive, for all intents and purposes. For example:

<img class="ui-avatar" src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+PHN2ZyB4bWxuczpyZGY9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkvMDIvMjItcmRmLXN5bnRheC1ucyMiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgaGVpZ2h0PSI5NiIgd2lkdGg9Ijk2IiB2ZXJzaW9uPSIxLjEiIHZpZXdCb3g9IjAgMCA5NiA5NiI+PGc+PHJlY3Qgd2lkdGg9Ijk2IiBoZWlnaHQ9Ijk2IiBmaWxsPSIjNDA3ODg3IiAvPjx0ZXh0IGZvbnQtc2l6ZT0iNjRweCIgZm9udC1mYW1pbHk9InNhbnMtc2VyaWYiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGZpbGw9IiNmZmZmZmYiPjx0c3BhbiB4PSI0OCIgeT0iNzIiIHN0cm9rZS13aWR0aD0iLjI2NDU4cHgiIGZpbGw9IiNmZmZmZmYiPkE8L3RzcGFuPjwvdGV4dD48L2c+PC9zdmc+">

[jackycute]

@alecdwm Very clever move!
Can you explain about why you choose seedrandom instead of built-in Math.random?

And any else need to concern before merge this PR?
Also need people who have LDAP environment to help us conduct some tests.

[alecdwm]

@jackycute Cheers! As far as I can tell, Math.random cannot be seeded, so if I were to use it the background colour would randomly change for all avatars.

By seeding seedrandom with the username for the avatar that's about to be generated, each user will have a random background colour, but it will always be the same colour for a single user.

Should be ready to merge!
Just @neopostmodern's suggestion left, and any feedback from testing of-course 😁

[jackycute]

@alecdwm Thanks!
We already used randomColor on generating color for guest users and I found that could be seeded.
Would you take a look at https://github.com/davidmerfield/randomColor and see if that works better?
Less dependency is better, you know.

[alecdwm]

@jackycute Oh neat! I should have looked closer and seen that >.<
Agreed, I'll swap us over to randomColor 😁

[jackycute]

Small tip: pass luminosity: 'dark' to randomColor or the white text might hard to read.

[jackycute]

Please remove the redundant comma after the last option.

[jackycute]

OK, looks all good.
Time for testing!

@almereyda @neopostmodern Could you please help?
We need one more person review and test then I will merge this.

[neopostmodern]

I can confirm that authentication against our institutional LDAP worked as of 3491f97. I can look through the code in detail if you want me to but from what I've skimmed through it seemed pretty sound.

We should discuss if the configuration value for the name of the LDAP provider is a show-stopper or if we rather move ahead to get this into master and I'll file a subsequent PR for the features mentioned above?

[jackycute]

I think we could continue commit here until we support multiple providers.
But just out of curiosity, are there many people need multiple LDAP providers?

And for multiple LDAP providers, you'll need to pass an array to ldap config.
I just confirmed that passport-ldapauth support multiple LDAP server configurations.
Please look into their tests and PRs by search. @alecdwm

Thanks guys! 👍

[alecdwm]

@jackycute I see two possible approaches to multiple-ldap-provider support:

1. The approach I believe @neopostmodern envisions:
   Each provider has their own log-in prompt and an associated name for the provider.
   For this, it shouldn't matter whether or not passport-ldapauth supports multiple server configurations;
   we would only need to supply the config for the server which the user entered their credentials.

2. All providers share a single log-in prompt (perhaps titled LDAP: (provider one), (provider two)).
   For this we would need to rely upon passport-ldapauth to support checking multiple providers with one set of credentials.
   I can't find this functionality in their project, but an alternative could be to iterate over the configured LDAP providers ourselves and only fail the auth request once it has been rejected by all possible providers.
   I'm not so sure though that I like the security implications of sending valid user credentials for one provider out to other providers.

I imagine approach 2 would be simpler to implement (with less changes), but security favours approach 1.
I'm not sure which is the superior choice in UI/UX.

Either way, the hackmd host will need to provide the LDAP configuration via an array. This is not a problem for the config file, but how should we approach the environment variables?

[jackycute]

@alecdwm Let me show you the way: vesse/passport-ldapauth#24

[jackycute]

I think the support of multiple LDAP providers might need more discussion.
So maybe we should open another PR/issue for that (yeah I changed my mind). @neopostmodern?

[neopostmodern]

Agree, I think we should postpone that into another PR.

[jackycute]

OK, so we merge this first.
Big thanks to @alecdwm @neopostmodern