From 5c0db639ebbe51b1185d4e8be5d66ed4d3e0b6fb Mon Sep 17 00:00:00 2001
From: Karsten Hopp <karsten@redhat.com>
Date: Aug 27 2018 14:25:57 +0000
Subject: [PATCH 1/3] add configure option to restrict email to certain domains


fixes #3550
add a config option ALLOWED_EMAIL_DOMAINS which can be used to limit
allowed email addresses to a list of domains.

Signed-off-by: Karsten Hopp <karsten@redhat.com>

---

diff --git a/files/pagure.cfg.sample b/files/pagure.cfg.sample
index 2898966..8bfee4d 100644
--- a/files/pagure.cfg.sample
+++ b/files/pagure.cfg.sample
@@ -49,6 +49,12 @@ FROM_EMAIL = 'pagure@localhost.localdomain'
 DOMAIN_EMAIL_NOTIFICATIONS = 'localhost.localdomain'
 SALT_EMAIL = '<secret key to be changed>'
 
+### Restrict outgoing emails to these domains:
+## If set, adding emailaccounts that don't end with these domainnames
+## will not be permitted. Mails to already existing emailaccounts
+## that are not covered by this list will not get sent.
+# ALLOWED_EMAIL_DOMAINS = [ 'localhost.localdomain', 'example.com' ]
+
 ### The URL at which the project is available.
 APP_URL = 'http://localhost.localdomain/'
 ### The URL at which the documentation of projects will be available
diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index c6c9b24..0debe4a 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -3541,7 +3541,10 @@ def set_up_user(
         emails = set()
     emails.add(default_email)
     for email in emails:
-        add_email_to_user(session, user, email)
+        try:
+            add_email_to_user(session, user, email)
+        except pagure.exceptions.PagureException as err:
+            _log.exception(err)
 
     if ssh_key and not user.public_ssh_key:
         update_user_ssh(session, user, ssh_key, keydir)
@@ -3549,8 +3552,26 @@ def set_up_user(
     return user
 
 
+def allowed_emailaddress(email):
+    ''' check if email domains are restricted and if a given email address
+    is allowed. '''
+    allowed_email_domains = pagure_config.get('ALLOWED_EMAIL_DOMAINS', None)
+    if allowed_email_domains:
+        for domain in allowed_email_domains:
+            if email.endswith(domain):
+                return
+        raise pagure.exceptions.PagureException(
+            'The email address ' + email + ' ' +
+            'is not in the list of allowed email domains:\n' +
+            "\n".join(allowed_email_domains))
+
+
 def add_email_to_user(session, user, user_email):
     """ Add the provided email to the specified user. """
+    try:
+        allowed_emailaddress(user_email)
+    except pagure.exceptions.PagureException:
+        raise
     emails = [email.email for email in user.emails]
     if user_email not in emails:
         useremail = model.UserEmail(user_id=user.id, email=user_email)
@@ -3741,8 +3762,12 @@ def update_blocked_issue(session, repo, issue, blocks, username, ticketfolder):
 
 
 def add_user_pending_email(session, userobj, email):
-    """ Add the provided user to the specified user.
+    """ Add the provided email to the specified user.
     """
+    try:
+        allowed_emailaddress(email)
+    except pagure.exceptions.PagureException:
+        raise
     other_user = search_user(session, email=email)
     if other_user and other_user != userobj:
         raise pagure.exceptions.PagureException(
diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py
index cec176f..7504203 100644
--- a/pagure/lib/notify.py
+++ b/pagure/lib/notify.py
@@ -323,6 +323,10 @@ def send_email(
 
     smtp = None
     for mailto in to_mail.split(","):
+        try:
+            pagure.lib.allowed_emailaddress(mailto)
+        except pagure.exceptions.PagureException:
+            continue
         msg = MIMEText(text.encode("utf-8"), "plain", "utf-8")
         msg["Subject"] = Header("[%s] %s" % (subject_tag, subject), "utf-8")
         msg["From"] = from_email
diff --git a/pagure/ui/app.py b/pagure/ui/app.py
index 8daf0b0..c4f3425 100644
--- a/pagure/ui/app.py
+++ b/pagure/ui/app.py
@@ -1331,6 +1331,9 @@ def confirm_email(token):
             flask.g.session.delete(email)
             flask.g.session.commit()
             flask.flash("Email validated")
+        except pagure.exceptions.PagureException as err:
+            flask.flash(str(err), "error")
+            _log.exception(err)
         except SQLAlchemyError as err:  # pragma: no cover
             flask.g.session.rollback()
             flask.flash(
diff --git a/pagure/ui/login.py b/pagure/ui/login.py
index e015ccc..283de31 100644
--- a/pagure/ui/login.py
+++ b/pagure/ui/login.py
@@ -61,17 +61,19 @@ def new_user():
         flask.g.session.add(user)
         flask.g.session.flush()
 
-        pagure.lib.add_email_to_user(
-            flask.g.session, user, form.email_address.data
-        )
-
         try:
+            pagure.lib.add_email_to_user(
+                flask.g.session, user, form.email_address.data
+            )
             flask.g.session.commit()
             send_confirmation_email(user)
             flask.flash(
                 "User created, please check your email to activate the "
                 "account"
             )
+        except pagure.exceptions.PagureException as err:
+            flask.flash(str(err), "error")
+            _log.exception(err)
         except SQLAlchemyError:  # pragma: no cover
             flask.g.session.rollback()
             flask.flash("Could not create user.")
diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 1b4af4b..450ba31 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2120,6 +2120,8 @@ class PagureLibtests(tests.Modeltests):
         statuses = pagure.lib.get_issue_statuses(self.session)
         self.assertEqual(sorted(statuses), ['Closed', 'Open'])
 
+    @patch.dict(pagure.config.config, {
+        'ALLOWED_EMAIL_DOMAINS': ["fp.o"]}, clear=True)
     def test_set_up_user(self):
         """ Test the set_up_user of pagure.lib. """
 
@@ -2185,6 +2187,22 @@ class PagureLibtests(tests.Modeltests):
         self.assertEqual(
             sorted(['skvidal@fp.o', 'svidal@fp.o']),
             sorted([email.email for email in items[2].emails]))
+        # add again with forbidden email domain
+        pagure.lib.set_up_user(
+            session=self.session,
+            username='skvidal',
+            fullname='Seth',
+            default_email='svidal@example.o',
+            keydir=pagure.config.config.get('GITOLITE_KEYDIR', None),
+        )
+        self.session.commit()
+        # Email not added
+        items = pagure.lib.search_user(self.session)
+        self.assertEqual(3, len(items))
+        self.assertEqual('skvidal', items[2].user)
+        self.assertEqual(
+            sorted(['skvidal@fp.o', 'svidal@fp.o']),
+            sorted([email.email for email in items[2].emails]))
 
     def test_update_user_ssh(self):
         """ Test the update_user_ssh of pagure.lib. """
@@ -5495,6 +5513,8 @@ foo bar
         )
         self.assertEqual(msg, 'Successfully edited your settings')
 
+    @patch.dict(pagure.config.config, {
+        'ALLOWED_EMAIL_DOMAINS': ["pingoured.fr"]}, clear=True)
     def test_add_email_to_user_with_logs(self):
         """ Test the add_email_to_user function of pagure.lib when there
         are log entries associated to the email added.
@@ -5523,6 +5543,18 @@ foo bar
         # Check emails after
         self.assertEqual(len(user.emails), 3)
 
+        # add another email address that is not in an allowed domain
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_email_to_user,
+            session=self.session,
+            user=user,
+            user_email='foo@bar.com'
+        )
+        self.session.commit()
+        # Check emails after
+        self.assertEqual(len(user.emails), 3)
+
     @patch('pagure.lib.is_valid_ssh_key', MagicMock(return_value='foo bar'))
     def test_update_user_ssh_valid_key(self):
         """ Test the update_user_ssh function of pagure.lib. """
@@ -5557,6 +5589,8 @@ foo bar
         )
 
     @patch('pagure.lib.notify.notify_new_email', MagicMock(return_value=True))
+    @patch.dict(pagure.config.config, {
+        'ALLOWED_EMAIL_DOMAINS': ["pingoured.fr"]}, clear=True)
     def test_add_user_pending_email(self):
         """ Test the add_user_pending_email function of pagure.lib. """
         user = pagure.lib.search_user(self.session, username='pingou')
@@ -5573,6 +5607,18 @@ foo bar
 
         self.assertEqual(len(user.emails), 2)
         self.assertEqual(len(user.emails_pending), 1)
+        # add another email address that is not allowed
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_pending_email,
+            session=self.session,
+            userobj=user,
+            email='foo@bar.com'
+        )
+        self.session.commit()
+
+        self.assertEqual(len(user.emails), 2)
+        self.assertEqual(len(user.emails_pending), 1)
 
     def test_resend_pending_email_someone_else_email(self):
         """ Test the resend_pending_email function of pagure.lib. """

From d944cf70cdda9906b8c8112cb345808ebac2825b Mon Sep 17 00:00:00 2001
From: Karsten Hopp <karsten@redhat.com>
Date: Aug 27 2018 14:25:57 +0000
Subject: [PATCH 2/3] test with 2 allowed email domain names


Signed-off-by: Karsten Hopp <karsten@redhat.com>

---

diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 450ba31..624ed62 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2196,7 +2196,7 @@ class PagureLibtests(tests.Modeltests):
             keydir=pagure.config.config.get('GITOLITE_KEYDIR', None),
         )
         self.session.commit()
-        # Email not added
+        # Email should not be added
         items = pagure.lib.search_user(self.session)
         self.assertEqual(3, len(items))
         self.assertEqual('skvidal', items[2].user)
@@ -2204,6 +2204,26 @@ class PagureLibtests(tests.Modeltests):
             sorted(['skvidal@fp.o', 'svidal@fp.o']),
             sorted([email.email for email in items[2].emails]))
 
+        # redo tests with 2 allowed domains
+        pagure.config.config['ALLOWED_EMAIL_DOMAINS'] = ["fp.o", "example.com"]
+        # add again with forbidden email domain
+        pagure.lib.set_up_user(
+            session=self.session,
+            username='skvidal',
+            fullname='Seth',
+            default_email='svidal@example.o',
+            keydir=pagure.config.config.get('GITOLITE_KEYDIR', None),
+        )
+        self.session.commit()
+        # Email should not be added
+        items = pagure.lib.search_user(self.session)
+        self.assertEqual(3, len(items))
+        self.assertEqual('skvidal', items[2].user)
+        self.assertEqual(
+            sorted(['skvidal@fp.o', 'svidal@fp.o']),
+            sorted([email.email for email in items[2].emails]))
+
+
     def test_update_user_ssh(self):
         """ Test the update_user_ssh of pagure.lib. """
 
@@ -5555,6 +5575,28 @@ foo bar
         # Check emails after
         self.assertEqual(len(user.emails), 3)
 
+        # redo tests with 2 allowed domains
+        pagure.config.config['ALLOWED_EMAIL_DOMAINS'] = ["pingoured.fr", "example.com"]
+        # Add the new_email to the user
+        pagure.lib.add_email_to_user(
+            self.session, user, 'another_email@example.com'
+        )
+        self.session.commit()
+        # Check emails after
+        self.assertEqual(len(user.emails), 4)
+
+        # add another email address that is not in an allowed domain
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_email_to_user,
+            session=self.session,
+            user=user,
+            user_email='another_foo@bar.com'
+        )
+        self.session.commit()
+        # Check emails after
+        self.assertEqual(len(user.emails), 4)
+
     @patch('pagure.lib.is_valid_ssh_key', MagicMock(return_value='foo bar'))
     def test_update_user_ssh_valid_key(self):
         """ Test the update_user_ssh function of pagure.lib. """

From 66e2584d9dc624b56c478ece4a57b4d316c81036 Mon Sep 17 00:00:00 2001
From: Karsten Hopp <karsten@redhat.com>
Date: Aug 27 2018 14:48:04 +0000
Subject: [PATCH 3/3] use @patch to correctly reset config options after the tests


Signed-off-by: Karsten Hopp <karsten@redhat.com>

---

diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 624ed62..312dd9f 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2204,8 +2204,45 @@ class PagureLibtests(tests.Modeltests):
             sorted(['skvidal@fp.o', 'svidal@fp.o']),
             sorted([email.email for email in items[2].emails]))
 
-        # redo tests with 2 allowed domains
-        pagure.config.config['ALLOWED_EMAIL_DOMAINS'] = ["fp.o", "example.com"]
+    @patch.dict(pagure.config.config, {
+        'ALLOWED_EMAIL_DOMAINS': ["fp.o", "example.c"]}, clear=True)
+    def test_set_up_user_multiple_emaildomains(self):
+        """ Test the set_up_user of pagure.lib when there are
+        multimple domains configured in ALLOWED_EMAIL_DOMAINS"""
+
+        items = pagure.lib.search_user(self.session)
+        self.assertEqual(2, len(items))
+        self.assertEqual(2, items[0].id)
+        self.assertEqual('foo', items[0].user)
+        self.assertEqual(1, items[1].id)
+        self.assertEqual('pingou', items[1].user)
+
+        pagure.lib.set_up_user(
+            session=self.session,
+            username='skvidal',
+            fullname='Seth',
+            default_email='skvidal@fp.o',
+            keydir=pagure.config.config.get('GITOLITE_KEYDIR', None),
+            ssh_key='foo key',
+        )
+        self.session.commit()
+
+        # Add the user a second time with a different email
+        pagure.lib.set_up_user(
+            session=self.session,
+            username='skvidal',
+            fullname='Seth',
+            default_email='skvidal@example.c',
+            keydir=pagure.config.config.get('GITOLITE_KEYDIR', None),
+        )
+        self.session.commit()
+        # Email added
+        items = pagure.lib.search_user(self.session)
+        self.assertEqual(3, len(items))
+        self.assertEqual('skvidal', items[2].user)
+        self.assertEqual(
+            sorted(['skvidal@fp.o', 'skvidal@example.c']),
+            sorted([email.email for email in items[2].emails]))
         # add again with forbidden email domain
         pagure.lib.set_up_user(
             session=self.session,
@@ -2220,10 +2257,9 @@ class PagureLibtests(tests.Modeltests):
         self.assertEqual(3, len(items))
         self.assertEqual('skvidal', items[2].user)
         self.assertEqual(
-            sorted(['skvidal@fp.o', 'svidal@fp.o']),
+            sorted(['skvidal@fp.o', 'skvidal@example.c']),
             sorted([email.email for email in items[2].emails]))
 
-
     def test_update_user_ssh(self):
         """ Test the update_user_ssh of pagure.lib. """
 

