Skip to content

Commit

Permalink
Strip spaces on registration and have reset password use email addres…
Browse files Browse the repository at this point in the history
…s instead of names (CTFd#1218)

* Usernames are now properly stripped before being used in registration checks
* Reset password function now uses email addresses instead of user names for tokens
* Prevent MLC users from resetting their password
  • Loading branch information
ColdHeat authored Jan 20, 2020
1 parent fe85fdf commit f660ed1
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 33 deletions.
53 changes: 36 additions & 17 deletions CTFd/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def confirm(data=None):
def reset_password(data=None):
if data is not None:
try:
name = unserialize(data, max_age=1800)
email_address = unserialize(data, max_age=1800)
except (BadTimeSignature, SignatureExpired):
return render_template(
"reset_password.html", errors=["Your link has expired"]
Expand All @@ -111,20 +111,35 @@ def reset_password(data=None):
if request.method == "GET":
return render_template("reset_password.html", mode="set")
if request.method == "POST":
user = Users.query.filter_by(name=name).first_or_404()
user.password = request.form["password"].strip()
password = request.form.get("password", "").strip()
user = Users.query.filter_by(email=email_address).first_or_404()
if user.oauth_id:
return render_template(
"reset_password.html",
errors=[
"Your account was registered via an authentication provider and does not have an associated password. Please login via your authentication provider."
],
)

pass_short = len(password) == 0
if pass_short:
return render_template(
"reset_password.html", errors=["Please pick a longer password"]
)

user.password = password
db.session.commit()
log(
"logins",
format="[{date}] {ip} - successful password reset for {name}",
name=name,
name=user.name,
)
db.session.close()
return redirect(url_for("auth.login"))

if request.method == "POST":
email_address = request.form["email"].strip()
team = Users.query.filter_by(email=email_address).first()
user = Users.query.filter_by(email=email_address).first()

get_errors()

Expand All @@ -134,15 +149,23 @@ def reset_password(data=None):
errors=["Email could not be sent due to server misconfiguration"],
)

if not team:
if not user:
return render_template(
"reset_password.html",
errors=[
"If that account exists you will receive an email, please check your inbox"
],
)

email.forgot_password(email_address, team.name)
if user.oauth_id:
return render_template(
"reset_password.html",
errors=[
"The email address associated with this account was registered via an authentication provider and does not have an associated password. Please login via your authentication provider."
],
)

email.forgot_password(email_address)

return render_template(
"reset_password.html",
Expand All @@ -159,9 +182,9 @@ def reset_password(data=None):
def register():
errors = get_errors()
if request.method == "POST":
name = request.form["name"]
email_address = request.form["email"]
password = request.form["password"]
name = request.form.get("name", "").strip()
email_address = request.form.get("email", "").strip().lower()
password = request.form.get("password", "").strip()

name_len = len(name) == 0
names = Users.query.add_columns("name", "id").filter_by(name=name).first()
Expand All @@ -170,9 +193,9 @@ def register():
.filter_by(email=email_address)
.first()
)
pass_short = len(password.strip()) == 0
pass_short = len(password) == 0
pass_long = len(password) > 128
valid_email = validators.validate_email(request.form["email"])
valid_email = validators.validate_email(email_address)
team_name_email_check = validators.validate_email(name)

if not valid_email:
Expand Down Expand Up @@ -206,11 +229,7 @@ def register():
)
else:
with app.app_context():
user = Users(
name=name.strip(),
email=email_address.lower(),
password=password.strip(),
)
user = Users(name=name, email=email_address, password=password)
db.session.add(user)
db.session.commit()
db.session.flush()
Expand Down
1 change: 1 addition & 0 deletions CTFd/schemas/teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def validate_name(self, data):
name = data.get("name")
if name is None:
return
name = name.strip()

existing_team = Teams.query.filter_by(name=name).first()
current_team = get_current_team()
Expand Down
2 changes: 2 additions & 0 deletions CTFd/schemas/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def validate_name(self, data):
name = data.get("name")
if name is None:
return
name = name.strip()

existing_user = Users.query.filter_by(name=name).first()
current_user = get_current_user()
Expand Down Expand Up @@ -95,6 +96,7 @@ def validate_email(self, data):
email = data.get("email")
if email is None:
return
email = email.strip()

existing_user = Users.query.filter_by(email=email).first()
current_user = get_current_user()
Expand Down
2 changes: 1 addition & 1 deletion CTFd/teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def new():

return render_template("teams/new_team.html", infos=infos, errors=errors)
elif request.method == "POST":
teamname = request.form.get("name")
teamname = request.form.get("name", "").strip()
passphrase = request.form.get("password", "").strip()
errors = get_errors()

Expand Down
15 changes: 8 additions & 7 deletions CTFd/utils/email/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ def sendmail(addr, text, subject="Message from {ctf_name}"):
return False, "No mail settings configured"


def forgot_password(email, team_name):
token = serialize(team_name)
text = """Did you initiate a password reset? Click the following link to reset your password:
def forgot_password(email):
token = serialize(email)
text = """Did you initiate a password reset? If you didn't initiate this request you can ignore this email.
Click the following link to reset your password:
{0}/{1}
""".format(
url_for("auth.reset_password", _external=True), token
)

return sendmail(email, text)
subject = "Password Reset Request from {ctf_name}"
return sendmail(addr=email, text=text, subject=subject)


def verify_email_address(addr):
Expand All @@ -36,7 +36,8 @@ def verify_email_address(addr):
url=url_for("auth.confirm", _external=True),
token=token,
)
return sendmail(addr, text)
subject = "Confirm your account for {ctf_name}"
return sendmail(addr=addr, text=text, subject=subject)


def user_created_notification(addr, name, password):
Expand Down
23 changes: 18 additions & 5 deletions tests/users/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ def test_register_duplicate_username():
password="password",
raise_for_error=False,
)
register_user(
app,
name="admin ",
email="[email protected]",
password="password",
raise_for_error=False,
)
user_count = Users.query.count()
assert user_count == 2 # There's the admin user and the first created user
destroy_ctfd(app)
Expand Down Expand Up @@ -353,11 +360,15 @@ def test_user_can_reset_password(mock_smtp):

# Build the email
msg = (
"""Did you initiate a password reset? Click the following link to reset """
"""your password:\n\nhttp://localhost/reset_password/InVzZXIxIg.TxD0vg.-gvVg-KVy0RWkiclAE6JViv1I0M\n\n"""
"Did you initiate a password reset? If you didn't initiate this request you can ignore this email."
"\n\nClick the following link to reset your password:\n"
"http://localhost/reset_password/InVzZXJAdXNlci5jb20i.TxD0vg.28dY_Gzqb1TH9nrcE_H7W8YFM-U\n"
)
ctf_name = get_config("ctf_name")
email_msg = MIMEText(msg)
email_msg["Subject"] = "Message from CTFd"
email_msg["Subject"] = "Password Reset Request from {ctf_name}".format(
ctf_name=ctf_name
)
email_msg["From"] = from_addr
email_msg["To"] = to_addr

Expand All @@ -374,9 +385,11 @@ def test_user_can_reset_password(mock_smtp):
data = {"nonce": sess.get("nonce"), "password": "passwordtwo"}

# Do the password reset
client.get("/reset_password/InVzZXIxIg.TxD0vg.-gvVg-KVy0RWkiclAE6JViv1I0M")
client.get(
"/reset_password/InVzZXJAdXNlci5jb20i.TxD0vg.28dY_Gzqb1TH9nrcE_H7W8YFM-U"
)
client.post(
"/reset_password/InVzZXIxIg.TxD0vg.-gvVg-KVy0RWkiclAE6JViv1I0M",
"/reset_password/InVzZXJAdXNlci5jb20i.TxD0vg.28dY_Gzqb1TH9nrcE_H7W8YFM-U",
data=data,
)

Expand Down
8 changes: 5 additions & 3 deletions tests/utils/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ def test_sendmail_with_mailgun_from_db_config(fake_post_request):


@patch("smtplib.SMTP")
@freeze_time("2012-01-14 03:21:34")
def test_verify_email(mock_smtp):
"""Does verify_email send emails"""
app = create_ctfd()
Expand All @@ -171,7 +170,8 @@ def test_verify_email(mock_smtp):
from_addr = get_config("mailfrom_addr") or app.config.get("MAILFROM_ADDR")
to_addr = "[email protected]"

verify_email_address(to_addr)
with freeze_time("2012-01-14 03:21:34"):
verify_email_address(to_addr)

# This is currently not actually validated
msg = (
Expand All @@ -182,7 +182,9 @@ def test_verify_email(mock_smtp):

ctf_name = get_config("ctf_name")
email_msg = MIMEText(msg)
email_msg["Subject"] = "Message from {0}".format(ctf_name)
email_msg["Subject"] = "Confirm your account for {ctf_name}".format(
ctf_name=ctf_name
)
email_msg["From"] = from_addr
email_msg["To"] = to_addr

Expand Down

0 comments on commit f660ed1

Please sign in to comment.