Skip to content
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.

Sign up for GitHub

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

ENT-669: Activation email received when skip_email_verification flag is 'True' #16321

Merged
merged 1 commit into from
Oct 30, 2017
Merged

ENT-669: Activation email received when skip_email_verification flag is 'True' #16321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
122 changes: 121 additions & 1 deletion common/djangoapps/student/tests/test_create_account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,48 @@
)
from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
from student.models import UserAttribute
from student.views import REGISTRATION_AFFILIATE_ID, REGISTRATION_UTM_CREATED_AT, REGISTRATION_UTM_PARAMETERS
from student.views import REGISTRATION_AFFILIATE_ID, REGISTRATION_UTM_CREATED_AT, REGISTRATION_UTM_PARAMETERS, \
skip_activation_email
from student.tests.factories import UserFactory
from third_party_auth.tests import factories as third_party_auth_factory

TEST_CS_URL = 'https://comments.service.test:123/'

TEST_USERNAME = 'test_user'
TEST_EMAIL = 'test@test.com'


def get_mock_pipeline_data(username=TEST_USERNAME, email=TEST_EMAIL):
"""
Return mock pipeline data.
"""
return {
'backend': 'tpa-saml',
'kwargs': {
'username': username,
'auth_entry': 'register',
'request': {
'SAMLResponse': [],
'RelayState': [
'testshib-openedx'
]
},
'is_new': True,
'new_association': True,
'user': None,
'social': None,
'details': {
'username': username,
'fullname': 'Test Test',
'last_name': 'Test',
'first_name': 'Test',
'email': email,
},
'response': {},
'uid': 'testshib-openedx:{}'.format(username)
}
}


@ddt.ddt
@override_settings(
Expand Down Expand Up @@ -423,6 +461,88 @@ def test_created_on_site_user_attribute_set(self):
profile = self.create_account_and_fetch_profile(host=self.site.domain)
self.assertEqual(UserAttribute.get_user_attribute(profile.user, 'created_on_site'), self.site.domain)

@ddt.data(
(
False, False, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False,
},
False # Do not skip activation email for normal scenario.
),
(
False, False, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': True, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False,
},
True # Skip activation email when `SKIP_EMAIL_VALIDATION` FEATURE flag is active.
),
(
False, False, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': True,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False,
},
True # Skip activation email when `AUTOMATIC_AUTH_FOR_TESTING` FEATURE flag is active.
),
(
True, False, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': True,
},
True # Skip activation email for external auth scenario.
),
(
False, False, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': True,
},
False # Do not skip activation email when `BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH` feature flag is set
# but it is not external auth scenario.
),
(
False, True, get_mock_pipeline_data(),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False,
},
True # Skip activation email if `skip_email_verification` is set for third party authentication.
),
(
False, False, get_mock_pipeline_data(email='invalid@yopmail.com'),
{
'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False,
'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False,
},
False # Send activation email when `skip_email_verification` is not set.
)
)
@ddt.unpack
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
def test_should_skip_activation_email(
self, do_external_auth, skip_email_verification, running_pipeline, feature_overrides, expected,
):
"""
Test `skip_activation_email` works as expected.
"""
third_party_provider = third_party_auth_factory.SAMLProviderConfigFactory(
skip_email_verification=skip_email_verification,
)
user = UserFactory(username=TEST_USERNAME, email=TEST_EMAIL)

with override_settings(FEATURES=dict(settings.FEATURES, **feature_overrides)):
result = skip_activation_email(
user=user,
do_external_auth=do_external_auth,
running_pipeline=running_pipeline,
third_party_provider=third_party_provider
)

assert result == expected


@ddt.ddt
class TestCreateAccountValidation(TestCase):
Expand Down
79 changes: 56 additions & 23 deletions common/djangoapps/student/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import dogstats_wrapper as dog_stats_api
import openedx.core.djangoapps.external_auth.views
import third_party_auth
from third_party_auth.saml import SAP_SUCCESSFACTORS_SAML_KEY
import track.views
from bulk_email.models import BulkEmailFlag, Optout # pylint: disable=import-error
from certificates.api import get_certificate_url, has_html_certificates_enabled # pylint: disable=import-error
Expand Down Expand Up @@ -2038,32 +2039,16 @@ def create_account_with_params(request, params):

create_comments_service_user(user)

# Don't send email if we are:
#
# 1. Doing load testing.
# 2. Random user generation for other forms of testing.
# 3. External auth bypassing activation.
# 4. Have the platform configured to not require e-mail activation.
# 5. Registering a new user using a trusted third party provider (with skip_email_verification=True)
#
# Note that this feature is only tested as a flag set one way or
# the other for *new* systems. we need to be careful about
# changing settings on a running system to make sure no users are
# left in an inconsistent state (or doing a migration if they are).
send_email = (
not settings.FEATURES.get('SKIP_EMAIL_VALIDATION', None) and
not settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING') and
not (do_external_auth and settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH')) and
not (
third_party_provider and third_party_provider.skip_email_verification and
user.email == running_pipeline['kwargs'].get('details', {}).get('email')
)
# Check if we system is configured to skip activation email for the current user.
skip_email = skip_activation_email(
user, do_external_auth, running_pipeline, third_party_provider,
)
if send_email:
compose_and_send_activation_email(user, profile, registration)
else:

if skip_email:
registration.activate()
_enroll_user_in_pending_courses(user) # Enroll student in any pending courses
else:
compose_and_send_activation_email(user, profile, registration)

# Immediately after a user creates an account, we log them in. They are only
# logged in until they close the browser. They can't log in again until they click
Expand Down Expand Up @@ -2099,6 +2084,54 @@ def create_account_with_params(request, params):
return new_user


def skip_activation_email(user, do_external_auth, running_pipeline, third_party_provider):
"""
Return `True` if activation email should be skipped.

Skip email if we are:
1. Doing load testing.
2. Random user generation for other forms of testing.
3. External auth bypassing activation.
4. Have the platform configured to not require e-mail activation.
5. Registering a new user using a trusted third party provider (with skip_email_verification=True)

Note that this feature is only tested as a flag set one way or
the other for *new* systems. we need to be careful about
changing settings on a running system to make sure no users are
left in an inconsistent state (or doing a migration if they are).

Arguments:
user (User): Django User object for the current user.
do_external_auth (bool): True if external authentication is in progress.
running_pipeline (dict): Dictionary containing user and pipeline data for third party authentication.
third_party_provider (ProviderConfig): An instance of third party provider configuration.

Returns:
(bool): `True` if account activation email should be skipped, `False` if account activation email should be
sent.
"""
sso_pipeline_email = running_pipeline and running_pipeline['kwargs'].get('details', {}).get('email')

# Email is valid if the SAML assertion email matches the user account email or
# no email was provided in the SAML assertion. Some IdP's use a callback
# to retrieve additional user account information (including email) after the
# initial account creation.
valid_email = (
sso_pipeline_email == user.email or (
sso_pipeline_email is None and
third_party_provider and
getattr(third_party_provider, "identity_provider_type", None) == SAP_SUCCESSFACTORS_SAML_KEY
)
)

return (
settings.FEATURES.get('SKIP_EMAIL_VALIDATION', None) or
settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING') or
(settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH') and do_external_auth) or
(third_party_provider and third_party_provider.skip_email_verification and valid_email)
)


def _enroll_user_in_pending_courses(student):
"""
Enroll student in any pending courses he/she may have.
Expand Down