Add check for undefined UUID#770
Merged
SISheogorath merged 1 commit intohackmdio:masterfrom Mar 18, 2018
SISheogorath:fix/ldapUUID
Merged
Add check for undefined UUID#770SISheogorath merged 1 commit intohackmdio:masterfrom SISheogorath:fix/ldapUUID
SISheogorath merged 1 commit intohackmdio:masterfrom
SISheogorath:fix/ldapUUID
Conversation
fooker
reviewed
Mar 17, 2018
| username = user[config.ldap.usernameField] | ||
| } | ||
|
|
||
| if (typeof uuid === 'undefined') { |
Contributor
There was a problem hiding this comment.
Maybe this should be moved up before the username code. If the username field is not used, this will say something like Could not determine UUID for LDAP user "undefined".
Contributor
|
Looks got to me. |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.Fixes #764
@fooker would you like to review and test this?