diff --git a/tests/unit/accounts/test_services.py b/tests/unit/accounts/test_services.py index 59f94ba2eb26..ac907bba8ca0 100644 --- a/tests/unit/accounts/test_services.py +++ b/tests/unit/accounts/test_services.py @@ -140,18 +140,26 @@ def test_check_password_updates(self, user_service): def test_create_user(self, user_service): user = UserFactory.build() - email = "foo@example.com" new_user = user_service.create_user( username=user.username, name=user.name, password=user.password, - email=email ) user_service.db.flush() user_from_db = user_service.get_user(new_user.id) + assert user_from_db.username == user.username assert user_from_db.name == user.name - assert user_from_db.email == email + + def test_add_email(self, user_service): + user = UserFactory.create() + email = "foo@example.com" + new_email = user_service.add_email(user.id, email) + + assert new_email.email == email + assert new_email.user == user + assert not new_email.primary + assert not new_email.verified def test_update_user(self, user_service): user = UserFactory.create() @@ -176,7 +184,8 @@ def test_find_by_email_not_found(self, user_service): def test_create_login_success(self, user_service): user = user_service.create_user( - "test_user", "test_name", "test_password", "test_email") + "test_user", "test_name", "test_password", + ) assert user.id is not None # now make sure that we can log in as that user @@ -184,7 +193,8 @@ def test_create_login_success(self, user_service): def test_create_login_error(self, user_service): user = user_service.create_user( - "test_user", "test_name", "test_password", "test_email") + "test_user", "test_name", "test_password", + ) assert user.id is not None assert not user_service.check_password(user.id, "bad_password") diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index 9ff87e076cf0..cb91a896f633 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -306,8 +306,13 @@ def test_redirect_authenticated_user(self): assert isinstance(result, HTTPSeeOther) assert result.headers["Location"] == "/" - def test_register_redirect(self, pyramid_request): + def test_register_redirect(self, pyramid_request, monkeypatch): pyramid_request.method = "POST" + + user = pretend.stub(id=pretend.stub()) + email = pretend.stub() + create_user = pretend.call_recorder(lambda *args, **kwargs: user) + add_email = pretend.call_recorder(lambda *args, **kwargs: email) pyramid_request.find_service = pretend.call_recorder( lambda *args, **kwargs: pretend.stub( csp_policy={}, @@ -316,10 +321,9 @@ def test_register_redirect(self, pyramid_request): verify_response=pretend.call_recorder(lambda _: None), find_userid=pretend.call_recorder(lambda _: None), find_userid_by_email=pretend.call_recorder(lambda _: None), - create_user=pretend.call_recorder( - lambda *args, **kwargs: pretend.stub(id=1), - ), update_user=lambda *args, **kwargs: None, + create_user=create_user, + add_email=add_email, ) ) pyramid_request.route_path = pretend.call_recorder(lambda name: "/") @@ -330,10 +334,20 @@ def test_register_redirect(self, pyramid_request): "email": "foo@bar.com", "full_name": "full_name", }) + send_email = pretend.call_recorder(lambda *a: None) + monkeypatch.setattr(views, 'send_email_verification_email', send_email) result = views.register(pyramid_request) + assert isinstance(result, HTTPSeeOther) assert result.headers["Location"] == "/" + assert create_user.calls == [ + pretend.call('username_value', 'full_name', 'MyStr0ng!shP455w0rd'), + ] + assert add_email.calls == [ + pretend.call(user.id, 'foo@bar.com', primary=True), + ] + assert send_email.calls == [pretend.call(pyramid_request, email)] class TestRequestPasswordReset: diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index c1bacbd66c7d..b579be1fd30f 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -138,7 +138,10 @@ def test_save_profile_validation_fails(self, monkeypatch): def test_add_email(self, monkeypatch, pyramid_config): email_address = "test@example.com" - user_service = pretend.stub() + email = pretend.stub(id=pretend.stub(), email=email_address) + user_service = pretend.stub( + add_email=pretend.call_recorder(lambda *a, **kw: email) + ) request = pretend.stub( POST={'email': email_address}, db=pretend.stub(flush=lambda: None), @@ -158,10 +161,6 @@ def test_add_email(self, monkeypatch, pyramid_config): ) ) - email_obj = pretend.stub(id=pretend.stub(), email=email_address) - email_cls = pretend.call_recorder(lambda **kw: email_obj) - monkeypatch.setattr(views, 'Email', email_cls) - send_email = pretend.call_recorder(lambda *a: None) monkeypatch.setattr(views, 'send_email_verification_email', send_email) @@ -171,14 +170,8 @@ def test_add_email(self, monkeypatch, pyramid_config): view = views.ManageProfileViews(request) assert view.add_email() == view.default_response - assert request.user.emails == [email_obj] - assert email_cls.calls == [ - pretend.call( - email=email_address, - user_id=request.user.id, - primary=False, - verified=False, - ), + assert user_service.add_email.calls == [ + pretend.call(request.user.id, email_address) ] assert request.session.flash.calls == [ pretend.call( @@ -188,7 +181,7 @@ def test_add_email(self, monkeypatch, pyramid_config): ), ] assert send_email.calls == [ - pretend.call(request, email_obj), + pretend.call(request, email), ] def test_add_email_validation_fails(self, monkeypatch): diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index 80c85ff02109..c8a7d8d05cf0 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -35,7 +35,7 @@ class TokenMissing(Exception): class IUserService(Interface): - def get_user(userid): + def get_user(user_id): """ Return the user object that represents the given userid, or None if there is no user for that ID. @@ -53,14 +53,15 @@ def find_userid(username): is no user with the given username. """ - def check_password(userid, password): + def check_password(user_id, password): """ Returns a boolean representing whether the given password is valid for the given userid. """ - def create_user(username, name, password, email, - is_active=False, is_staff=False, is_superuser=False): + def create_user( + username, name, password, + is_active=False, is_staff=False, is_superuser=False): """ Accepts a user object, and attempts to create a user with those attributes. @@ -68,6 +69,11 @@ def create_user(username, name, password, email, A UserAlreadyExists Exception is raised if the user already exists. """ + def add_email(user_id, email_address, primary=False, verified=False): + """ + Adds an email for the provided user_id + """ + def update_user(user_id, **changes): """ Updates the user object diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 02c76eaef871..7fd8f796472d 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -148,8 +148,9 @@ def check_password(self, userid, password): return False - def create_user(self, username, name, password, email, - is_active=False, is_staff=False, is_superuser=False): + def create_user( + self, username, name, password, + is_active=False, is_staff=False, is_superuser=False): user = User(username=username, name=name, @@ -158,13 +159,20 @@ def create_user(self, username, name, password, email, is_staff=is_staff, is_superuser=is_superuser) self.db.add(user) - email_object = Email(email=email, user=user, - primary=True, verified=False) - self.db.add(email_object) - # flush the db now so user.id is available - self.db.flush() + self.db.flush() # flush the db now so user.id is available + return user + def add_email(self, user_id, email_address, primary=False, verified=False): + user = self.get_user(user_id) + email= Email( + email=email_address, user=user, primary=primary, verified=verified, + ) + self.db.add(email) + self.db.flush() # flush the db now so email.id is available + + return email + def update_user(self, user_id, **changes): user = self.get_user(user_id) for attr, value in changes.items(): diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py index 0c0286723b14..6305ae57eed2 100644 --- a/warehouse/accounts/views.py +++ b/warehouse/accounts/views.py @@ -32,7 +32,9 @@ ) from warehouse.accounts.models import Email from warehouse.cache.origin import origin_cache -from warehouse.email import send_password_reset_email +from warehouse.email import ( + send_password_reset_email, send_email_verification_email, +) from warehouse.packaging.models import Project, Release from warehouse.utils.http import is_safe_url @@ -231,13 +233,16 @@ def register(request, _form_class=RegistrationForm): if request.method == "POST" and form.validate(): user = user_service.create_user( - form.username.data, form.full_name.data, form.password.data, - form.email.data + form.username.data, form.full_name.data, form.password.data ) + email = user_service.add_email(user.id, form.email.data, primary=True) + + send_email_verification_email(request, email) return HTTPSeeOther( request.route_path("index"), - headers=dict(_login_user(request, user.id))) + headers=dict(_login_user(request, user.id)) + ) return {"form": form} diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 7bc82e6e91fc..4e3a52ad739d 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -78,14 +78,9 @@ def add_email(self): form = AddEmailForm(self.request.POST, user_service=self.user_service) if form.validate(): - email = Email( - email=form.email.data, - user_id=self.request.user.id, - primary=False, - verified=False, + email = self.user_service.add_email( + self.request.user.id, form.email.data, ) - self.request.user.emails.append(email) - self.request.db.flush() # To get the new ID send_email_verification_email(self.request, email)