Skip to content

Commit f32cdc3

Browse files
jwag956slint
andcommitted
Fix - don't allow re-use of confirmation token.
Added option to not automatically log in user when they confirm their registration. The default still it to log in the user. Backport of pallets-eco/flask-security#137. Co-authored-by: Alex Ioannidis <a.ioannidis@cern.ch>
1 parent a1140d8 commit f32cdc3

File tree

4 files changed

+118
-20
lines changed

4 files changed

+118
-20
lines changed

flask_security/core.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
"CONFIRM_EMAIL_WITHIN": "5 days",
9898
"RESET_PASSWORD_WITHIN": "5 days",
9999
"LOGIN_WITHOUT_CONFIRMATION": False,
100+
"AUTO_LOGIN_AFTER_CONFIRM": True,
100101
"EMAIL_SENDER": LocalProxy(
101102
lambda: current_app.config.get("MAIL_DEFAULT_SENDER", "no-reply@localhost")
102103
),

flask_security/views.py

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -141,39 +141,50 @@ def confirm_email(token):
141141
expired, invalid, user = confirm_email_token_status(token)
142142

143143
if not user or invalid:
144-
invalid = True
145144
do_flash(*get_message("INVALID_CONFIRMATION_TOKEN"))
145+
return redirect(
146+
get_url(_security.confirm_error_view) or url_for("send_confirmation")
147+
)
146148

147-
already_confirmed = user is not None and user.confirmed_at is not None
148-
149-
if expired and not already_confirmed:
150-
send_confirmation_instructions(user)
151-
do_flash(
152-
*get_message(
153-
"CONFIRMATION_EXPIRED",
154-
email=user.email,
155-
within=_security.confirm_email_within,
149+
already_confirmed = user.confirmed_at is not None
150+
151+
if expired or already_confirmed:
152+
if already_confirmed:
153+
do_flash(*get_message("ALREADY_CONFIRMED"))
154+
else:
155+
send_confirmation_instructions(user)
156+
do_flash(
157+
*get_message(
158+
"CONFIRMATION_EXPIRED",
159+
email=user.email,
160+
within=_security.confirm_email_within,
161+
)
156162
)
157-
)
158-
if invalid or (expired and not already_confirmed):
159163
return redirect(
160164
get_url(_security.confirm_error_view) or url_for("send_confirmation")
161165
)
162166

163167
if user != current_user:
164168
logout_user()
165-
login_user(user)
169+
if config_value("AUTO_LOGIN_AFTER_CONFIRM"):
170+
# N.B. this is a (small) security risk if email went to wrong place.
171+
# and you have the LOGIN_WITHOUT_CONFIRMATION flag since in that case
172+
# you can be logged in and doing stuff - but another person could
173+
# get the email.
174+
login_user(user)
166175

167-
if confirm_user(user):
168-
after_this_request(_commit)
169-
msg = "EMAIL_CONFIRMED"
170-
else:
171-
msg = "ALREADY_CONFIRMED"
176+
confirm_user(user)
177+
after_this_request(_commit)
172178

173-
do_flash(*get_message(msg))
179+
do_flash(*get_message("EMAIL_CONFIRMED"))
174180

175181
return redirect(
176-
get_url(_security.post_confirm_view) or get_url(_security.post_login_view)
182+
get_url(_security.post_confirm_view)
183+
or get_url(
184+
_security.post_login_view
185+
if config_value("AUTO_LOGIN_AFTER_CONFIRM")
186+
else _security.login_url
187+
)
177188
)
178189

179190

tests/conftest.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ def post_logout():
111111
def post_register():
112112
return render_template("index.html", content="Post Register")
113113

114+
@app.route("/post_confirm")
115+
def post_confirm():
116+
return render_template("index.html", content="Post Confirm")
117+
114118
@app.route("/admin")
115119
@roles_required("admin")
116120
def admin():

tests/test_confirmable.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,88 @@ def test_login_when_unconfirmed(client, get_message):
156156
assert b"mary@lp.com" in response.data
157157

158158

159+
@pytest.mark.registerable()
160+
def test_confirmation_token_reuse_does_not_auto_login(client, get_message):
161+
"""A reused confirmation link must not silently log the user in."""
162+
email = "reuse@lp.com"
163+
164+
with capture_registrations() as registrations:
165+
data = dict(email=email, password="password", next="")
166+
client.post("/register", data=data)
167+
168+
token = registrations[0]["confirm_token"]
169+
170+
# First confirm: legitimate, user gets logged in (default AUTO_LOGIN_AFTER_CONFIRM=True).
171+
response = client.get("/confirm/" + token, follow_redirects=True)
172+
assert get_message("EMAIL_CONFIRMED") in response.data
173+
assert b"Hello " + email.encode() in response.data
174+
175+
logout(client)
176+
177+
# Re-using the same link from anonymous context must NOT log us back in.
178+
response = client.get("/confirm/" + token, follow_redirects=True)
179+
assert get_message("ALREADY_CONFIRMED") in response.data
180+
assert b"Hello " + email.encode() not in response.data
181+
182+
183+
@pytest.mark.registerable()
184+
@pytest.mark.settings(login_without_confirmation=True, auto_login_after_confirm=False)
185+
def test_confirmation_different_user_when_logged_in_no_auto(client, get_message):
186+
"""AUTO_LOGIN_AFTER_CONFIRM=False must not log the second user in."""
187+
e1 = "dude@lp.com"
188+
e2 = "lady@lp.com"
189+
190+
with capture_registrations() as registrations:
191+
for e in e1, e2:
192+
data = dict(email=e, password="password", next="")
193+
client.post("/register", data=data)
194+
logout(client)
195+
196+
token1 = registrations[0]["confirm_token"]
197+
token2 = registrations[1]["confirm_token"]
198+
199+
client.get("/confirm/" + token1, follow_redirects=True)
200+
logout(client)
201+
authenticate(client, email=e1)
202+
203+
response = client.get("/confirm/" + token2, follow_redirects=True)
204+
assert get_message("EMAIL_CONFIRMED") in response.data
205+
# second user must not have been logged in
206+
assert b"Hello " + e2.encode() not in response.data
207+
208+
209+
@pytest.mark.registerable()
210+
@pytest.mark.settings(auto_login_after_confirm=False)
211+
def test_confirm_redirect(client, get_message):
212+
"""With AUTO_LOGIN_AFTER_CONFIRM=False, redirect target is the login URL."""
213+
with capture_registrations() as registrations:
214+
data = dict(email="jane@lp.com", password="password", next="")
215+
client.post("/register", data=data, follow_redirects=True)
216+
217+
token = registrations[0]["confirm_token"]
218+
219+
response = client.get("/confirm/" + token)
220+
assert "location" in response.headers
221+
assert "/login" in response.location
222+
223+
response = client.get(response.location)
224+
assert get_message("EMAIL_CONFIRMED") in response.data
225+
226+
227+
@pytest.mark.registerable()
228+
@pytest.mark.settings(post_confirm_view="/post_confirm")
229+
def test_confirm_redirect_to_post_confirm(client, get_message):
230+
"""post_confirm_view takes precedence over the login redirect."""
231+
with capture_registrations() as registrations:
232+
data = dict(email="john@lp.com", password="password", next="")
233+
client.post("/register", data=data, follow_redirects=True)
234+
235+
token = registrations[0]["confirm_token"]
236+
237+
response = client.get("/confirm/" + token, follow_redirects=True)
238+
assert b"Post Confirm" in response.data
239+
240+
159241
@pytest.mark.registerable()
160242
@pytest.mark.settings(login_without_confirmation=True)
161243
def test_confirmation_different_user_when_logged_in(client, get_message):

0 commit comments

Comments
 (0)