Skip to content

Verify email on registration #2960

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.

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

Merged
merged 4 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 15 additions & 5 deletions tests/unit/accounts/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,26 @@ def test_check_password_updates(self, user_service):

def test_create_user(self, user_service):
user = UserFactory.build()
email = "[email protected]"
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 = "[email protected]"
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()
Expand All @@ -176,15 +184,17 @@ 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
assert user_service.check_password(user.id, "test_password")

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")
Expand Down
22 changes: 18 additions & 4 deletions tests/unit/accounts/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={},
Expand All @@ -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: "/")
Expand All @@ -330,10 +334,20 @@ def test_register_redirect(self, pyramid_request):
"email": "[email protected]",
"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, '[email protected]', primary=True),
]
assert send_email.calls == [pretend.call(pyramid_request, email)]


class TestRequestPasswordReset:
Expand Down
21 changes: 7 additions & 14 deletions tests/unit/manage/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ def test_save_profile_validation_fails(self, monkeypatch):

def test_add_email(self, monkeypatch, pyramid_config):
email_address = "[email protected]"
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),
Expand All @@ -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)

Expand All @@ -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(
Expand All @@ -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):
Expand Down
14 changes: 10 additions & 4 deletions warehouse/accounts/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -53,21 +53,27 @@ 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.

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
Expand Down
22 changes: 15 additions & 7 deletions warehouse/accounts/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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():
Expand Down
13 changes: 9 additions & 4 deletions warehouse/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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}

Expand Down
9 changes: 2 additions & 7 deletions warehouse/manage/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down