Signup error #109
Merged
Signup error #109
Conversation
This file contains 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
lambdaTotoro
commented
Apr 24, 2020
nativeauthenticator/handlers.py
Outdated
Comment on lines
71
to
74
| elif self.authenticator.open_signup: | ||
| alert = 'alert-success' | ||
| message = ('The signup was successful. You can now go to ' | ||
| 'home page and log in the system') |
There was a problem hiding this comment.
I moved this part under the "if taken" branch so that it would not report success on open signup, even when the name was, in fact, already taken.
lambdaTotoro
commented
Apr 24, 2020
| user = self.authenticator.create_user(**user_info) | ||
| name = self.authenticator.user_exists(user_info['username']) |
There was a problem hiding this comment.
Variable name was confusing, this is a boolean that indicates if the name is taken, not the name as a string.
lambdaTotoro
commented
Apr 24, 2020
| alert = 'alert-danger' | ||
| pw_len = self.authenticator.minimum_password_length | ||
| taken = self.authenticator.user_exists(username) |
There was a problem hiding this comment.
This is the line that caused the errors. "username" here is a boolean indicating wether the name exists in the system, but it is used like a string containing a username. Apart from this being nonsensical, during jupyterhub username normalisation, .lower() is called on this variable, which throws an error because that's not defined on booleans, leading to a 500 error.
leportella
approved these changes
May 23, 2020
| alert = 'alert-danger' | ||
| pw_len = self.authenticator.minimum_password_length | ||
| taken = self.authenticator.user_exists(username) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is basically a follow-up PR to #102, which introduced a bug that now leads to 500 errors (instead of sensible error messages) when signing up with a taken username. This PR removes this bug.
There are also a few minor improvements that I found along the way. These can be factored out if necessary.