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.

Sign up for GitHub

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

Signup error #109

Merged
merged 5 commits into from
May 23, 2020
Merged

Signup error #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1c9d0f7
fix signup handling
lambdaTotoro Apr 22, 2020
1e9971b
tabs/spaces
lambdaTotoro Apr 22, 2020
0fc21ad
typo
lambdaTotoro Apr 22, 2020
99bce95
fix line length and overindentation
lambdaTotoro Apr 24, 2020
bd61472
fix multiple spaces before operator
lambdaTotoro Apr 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 29 additions & 26 deletions nativeauthenticator/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,36 @@ async def get(self):
)
self.finish(html)

def get_result_message(self, user, username):
def get_result_message(self, user, taken):
alert = 'alert-info'
message = 'Your information have been sent to the admin'
message = 'Your information has been sent to the admin'

if self.authenticator.open_signup:
alert = 'alert-success'
message = ('The signup was successful. You can now go to '
'home page and log in the system')
if not user:
# Always error if username is taken.
if taken:
alert = 'alert-danger'
pw_len = self.authenticator.minimum_password_length
taken = self.authenticator.user_exists(username)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a great catch


if pw_len:
message = ("Something went wrong. Be sure your password has "
"at least {} characters, doesn't have spaces or "
"commas and is not too common.").format(pw_len)

elif taken:
message = ("Something went wrong. It appears that this "
"username is already in use. Please try again "
"with a different username.")

else:
message = ("Something went wrong. Be sure your password "
" doesn't have spaces or commas and is not too "
"common.")
message = ("Something went wrong. It appears that this "
"username is already in use. Please try again "
"with a different username.")
else:
# Error if user creation was not successful.
if not user:
alert = 'alert-danger'
pw_len = self.authenticator.minimum_password_length
if pw_len:
message = ("Something went wrong. Be sure your "
"password has at least {} characters, doesn't "
"have spaces or commas and is not too "
"common.").format(pw_len)
else:
message = ("Something went wrong. Be sure your password "
"doesn't have spaces or commas and is not too "
"common.")

# If user creation went through & open-signup is enabled, success.
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')

return alert, message

Expand All @@ -83,10 +86,10 @@ async def post(self):
'email': self.get_body_argument('email', '', strip=False),
'has_2fa': bool(self.get_body_argument('2fa', '', strip=False)),
}
taken = self.authenticator.user_exists(user_info['username'])
user = self.authenticator.create_user(**user_info)
name = self.authenticator.user_exists(user_info['username'])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name was confusing, this is a boolean that indicates if the name is taken, not the name as a string.


alert, message = self.get_result_message(user, name)
alert, message = self.get_result_message(user, taken)

otp_secret, user_2fa = '', ''
if user:
Expand Down
2 changes: 1 addition & 1 deletion nativeauthenticator/templates/native-login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
<input type="submit" id="login_submit" class='btn btn-jupyter' value='Sign In' tabindex="3" />
{% if enable_signup %}
<div style="padding-top: 25px;">
<p>Don't have an user? <a href="{{ base_url }}signup">Signup!</a></p>
<p>Don't have an account? <a href="{{ base_url }}signup">Signup!</a></p>
</div>
{% endif %}
</div>
Expand Down
4 changes: 2 additions & 2 deletions nativeauthenticator/templates/signup.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
{% endif %}
<input type="submit" id="signup_submit" class='btn btn-jupyter' value='Create User' tabindex="3" />
<div style="padding-top: 25px;">
Already have an user? <a href="{{ base_url }}login">Login!</a>
Already have an account? <a href="{{ base_url }}login">Login!</a>
</div>
{% if alert %}
<div class="alert {{alert}}" style="margin-top: 15px;" role="alert">
Expand All @@ -61,4 +61,4 @@
</form>
</div>
{% endblock login %}
{% endblock %}
{% endblock %}