This patch adds a try-catch around the rolesClaim extraction to prevent
full crashes of HedgeDoc when a user profile is read, that doesn't
contain any such claim, which can happen with some IdPs, like Keycloak,
that omit the attribute when it's empty.
As a result an authorized user would crash the entire server, which is
definitely unintended behaviour. The simply try-catch should resolve the
issue and make sure that roles is always defined even if the
`extractProfileAttribute` call fails.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
Since https://github.com/vesse/node-ldapauth-fork/commit
/741a648df98d789856b3301d65103b74872fdeea, ldapauth-fork calls `push` on
the attributes array.
Since we deep-freeze our config object in https://github
.com/hedgedoc/hedgedoc/blob/master/lib/config/index.js#L200, this
causes a crash.
This commit fixes the crash by creating a mutable clone of the LDAP
config and passing that to the LDAP strategy.
Fixes https://github.com/hedgedoc/hedgedoc/issues/2561
Signed-off-by: David Mehren <git@herrmehren.de>
We use the attribute `emails` (plural) for email addresses with other auth providers like LDAP or SAML. In case of OAuth2 we used the attribute `email` (singular) which resulted in problems.
Furthermore the OAuth2 strategy fell into the default fallback of the provider switch statement. This statement did not check email addresses but did generate the letter-avatar instantly.
Signed-off-by: Erik Michelson <github@erik.michelson.eu>
If the optional config options `config.oauth2.userProfileIdAttr` or `config.oauth2.rolesClaim` were not set, `String.split` was called on `undefined`, triggering a crash.
This commit adds handling of these cases and improves error logging in `checkAuthorization`.
Fixes#608
Signed-off-by: David Mehren <git@herrmehren.de>
The OAuth2 specification RECOMMENDS setting the state to protect against
CSRF attacks. Some OAuth2 providers (e.g. ORY Hydra) refuse to
authenticate without the state set.
This is a cherry-pick of 852868419d.
Signed-off-by: haslersn <sebastian.hasler@gmx.net>
This is a backport of #278 with the default value of `scope` changed to
`undefined`. This is thus a fully backward-compatible change.
Signed-off-by: Victor Berger <victor.berger@m4x.org>
Because of circular import problems, this commit also moves the error messages from response.js to errors.js
Signed-off-by: David Mehren <dmehren1@gmail.com>
We have various places with overly simple if statements that could be
handled by our logging library. Also a lot of those logs are not marked
as debug logs but as info logs, which can cause confusion during
debugging.
This patch removed unneeded if clauses around debug logging statements,
reworks debug log messages towards ECMA templates and add some new
logging statements which might be helpful in order to debug things like
image uploads.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
Since we are about to release it's time to finally fix our linting. This
patch basically runs eslint --fix and does some further manual fixes.
Also it sets up eslint to fail on every warning on order to make
warnings visable in the CI process.
There should no functional change be introduced.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
disableRequestedAuthnContext: true|false
By default only Password authmethod is accepted, this option allows any other method.
Issue and option described here:
https://github.com/bergie/passport-saml/issues/226
Signed-off-by: Emmanuel Ormancey <emmanuel.ormancey@cern.ch>
Since Google+ is shutting down soon, we need to get the profile data
from another URL. Since the library already supports it, all we need to
do is adding a single line of code.
Details:
https://github.com/hackmdio/codimd/issues/1160
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
This fixes part of #1056: an error while obtaining the profile
would have `502`-crashed the server.
Signed-off-by: Claudius Coenen <opensource@amenthes.de>
With OpenID every OpenID capable provider can provide authentication for
users of a CodiMD instance. This means we have federated
authentication.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
This refactors the configs a bit to now use camel case everywhere.
This change should help to clean up the config interface and make it
better understandable.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
This check is needed at there are tons of LDAP implementations out there
and none has at least one guaranteed unique field. As we currently check
three fields and added an option to select one yourself, it's still not
said that any of these fields is set. This will now create an error
and fail the authentication instead of letting people may get access to
other people's notes which are stored under a this way deterministic
wrong userid named `LDAP-undefined`.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
Since we added user management it's possible to get non-existent users
which can cause a crash of the Backend server.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
This determines which ldap field is used as the username on
HackMD. By default, the "id" is used as username, too. The id
is taken from the fields `uidNumber`, `uid` or
`sAMAccountName`. To give the user more flexibility, they can
now choose the field used for the username instead.
