Skip to content

Commit

Permalink
Merge pull request #674 from SUNET/lundberg_signup_fix
Browse files Browse the repository at this point in the history
Better password handling in signup
  • Loading branch information
helylle authored Aug 14, 2024
2 parents 702bd50 + 18bcc1b commit 193999b
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/eduid/webapp/jsconfig/tests/data/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ eduid:
login_service_url: 'https://idp.example.com/services/idp'
lookup_mobile_proofing_service_url: 'https://dashboard.example.com/services/mobile-proofing'
orcid_service_url: 'https://dashboard.example.com/services/orcid'
password_entropy: 12
password_length: 10
password_entropy: 25
password_length: 12
personal_data_service_url: 'https://dashboard.example.com/services/pdata'
phone_service_url: 'https://dashboard.example.com/services/phone'
reset_password_link: 'https://example.com/reset-password'
Expand Down
4 changes: 2 additions & 2 deletions src/eduid/webapp/jsconfig/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ def update_config(self, config: dict[str, Any]) -> dict[str, Any]:
"login_service_url": "https://idp.example.com/services/idp",
"lookup_mobile_proofing_service_url": "https://dashboard.example.com/services/mobile-proofing",
"orcid_service_url": "https://dashboard.example.com/services/orcid",
"password_entropy": 12,
"password_length": 10,
"password_entropy": 25,
"password_length": 12,
"personal_data_service_url": "https://dashboard.example.com/services/pdata",
"phone_service_url": "https://dashboard.example.com/services/phone",
"reset_password_link": "https://example.com/reset-password",
Expand Down
4 changes: 2 additions & 2 deletions src/eduid/webapp/signup/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class Captcha(EduidSchema):

class Credentials(EduidSchema):
completed = fields.Boolean(required=True)
generated_password = fields.String(required=False)
custom_password = fields.String(required=False)
generated_password = fields.String(required=True, default=None)
custom_password = fields.Boolean(required=True, default=False)
# TODO: implement webauthn signup

already_signed_up = fields.Boolean(required=True)
Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/signup/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class SignupConfig(
signup_url: str
dashboard_url: str

password_length: int = 10
password_length: int = 12
throttle_resend: timedelta = Field(default=timedelta(minutes=5))
email_verification_code_length: int = 6
email_verification_max_bad_attempts: int = 3
Expand Down
22 changes: 11 additions & 11 deletions src/eduid/webapp/signup/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,8 @@ def _create_user(
"use_suggested_password": True,
"use_webauthn": False,
}
if custom_password is not None:
_data["custom_password"] = custom_password
if data is not None:
_data.update(data)

Expand All @@ -563,10 +565,8 @@ def _create_user(
assert self.get_response_payload(response)["state"]["email"]["completed"] is True
assert self.get_response_payload(response)["state"]["credentials"]["completed"] is True
if custom_password:
assert (
self.get_response_payload(response)["state"]["credentials"]["custom_password"]
== custom_password
)
assert self.get_response_payload(response)["state"]["credentials"]["custom_password"] is True
assert self.get_response_payload(response)["state"]["credentials"]["generated_password"] is None
else:
assert (
self.get_response_payload(response)["state"]["credentials"]["generated_password"]
Expand Down Expand Up @@ -848,7 +848,7 @@ def test_get_state_initial(self):
assert state == {
"already_signed_up": False,
"captcha": {"completed": False},
"credentials": {"completed": False, "generated_password": None},
"credentials": {"completed": False, "custom_password": False, "generated_password": None},
"email": {"address": None, "bad_attempts": 0, "bad_attempts_max": 3, "completed": False, "sent_at": None},
"invite": {"completed": False, "finish_url": None, "initiated_signup": False},
"name": {"given_name": None, "surname": None},
Expand All @@ -863,7 +863,7 @@ def test_get_state_initial_logged_in(self):
assert state == {
"already_signed_up": True,
"captcha": {"completed": False},
"credentials": {"completed": False, "generated_password": None},
"credentials": {"completed": False, "custom_password": False, "generated_password": None},
"email": {"address": None, "bad_attempts": 0, "bad_attempts_max": 3, "completed": False, "sent_at": None},
"invite": {"completed": False, "finish_url": None, "initiated_signup": False},
"name": {"given_name": None, "surname": None},
Expand Down Expand Up @@ -1249,10 +1249,9 @@ def test_create_user_with_custom_password(self):
self._prepare_for_create_user(given_name=given_name, surname=surname, email=email)
data = {
"use_suggested_password": False,
"custom_password": "9MbKxTHhCDK3Y9hhn6",
"use_webauthn": False,
}
response = self._create_user(data=data, expect_success=True)
response = self._create_user(data=data, custom_password="9MbKxTHhCDK3Y9hhn6", expect_success=True)
assert response.reached_state == SignupState.S6_CREATE_USER

with self.session_cookie_anon(self.browser) as client:
Expand All @@ -1272,10 +1271,11 @@ def test_create_user_with_weak_custom_password(self):
self._prepare_for_create_user(given_name=given_name, surname=surname, email=email)
data = {
"use_suggested_password": True,
"custom_password": "abc123",
"use_webauthn": False,
}
response = self._create_user(data=data, expect_success=False, expected_message=SignupMsg.weak_custom_password)
response = self._create_user(
data=data, custom_password="abc123", expect_success=False, expected_message=SignupMsg.weak_custom_password
)
assert response.reached_state == SignupState.S6_CREATE_USER

def test_create_user_out_of_sync(self):
Expand Down Expand Up @@ -1382,7 +1382,7 @@ def test_get_state_after_accept_invite(self):
assert normalised_data(state, exclude_keys=["expires_time_left", "throttle_time_left", "sent_at"]) == {
"already_signed_up": False,
"captcha": {"completed": False},
"credentials": {"completed": False, "generated_password": None},
"credentials": {"completed": False, "custom_password": False, "generated_password": None},
"email": {
"address": "[email protected]",
"bad_attempts": 0,
Expand Down
8 changes: 5 additions & 3 deletions src/eduid/webapp/signup/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,14 @@ def create_user(use_suggested_password: bool, use_webauthn: bool, custom_passwor
session.signup.user_created = True
session.signup.credentials.completed = True
session.common.eppn = signup_user.eppn
# create payload before clearing passwords
# create payload before clearing generated password
state = session.signup.to_dict()
state["credentials"]["custom_password"] = custom_password
if custom_password is not None:
state["credentials"]["custom_password"] = True
state["credentials"]["generated_password"] = None
# clear passwords from session and namespace
session.signup.credentials.generated_password = None
del custom_password
session.signup.credentials.generated_password = None
# clear signup session if the user is done
if not session.signup.invite.initiated_signup:
del session.signup
Expand Down

0 comments on commit 193999b

Please sign in to comment.