From 33d4c0b3d5332d46fefcb2fe91df9e641f207c60 Mon Sep 17 00:00:00 2001
From: Francis Charette Migneault <francis.charette.migneault@gmail.com>
Date: Wed, 29 Jul 2020 16:24:34 -0400
Subject: [PATCH] user input validation (fix #84, relates #171) + adjust input
 types + current user view tests

---
 .../api/management/register/register_views.py |  1 +
 magpie/api/management/user/user_utils.py      |  3 ++
 magpie/ui/management/templates/add_user.mako  |  2 +-
 .../ui/management/templates/edit_service.mako |  2 +-
 tests/interfaces.py                           | 35 ++++++++++++++---
 tests/test_magpie_ui.py                       |  4 +-
 tests/test_register.py                        |  1 +
 tests/utils.py                                | 39 ++++++++++++-------
 8 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/magpie/api/management/register/register_views.py b/magpie/api/management/register/register_views.py
index 930bbe54a..021d44012 100644
--- a/magpie/api/management/register/register_views.py
+++ b/magpie/api/management/register/register_views.py
@@ -15,6 +15,7 @@
 from magpie.api.management.group import group_formats as gf
 from magpie.api.management.register import register_utils as ru
 from magpie.api.management.user import user_utils as uu
+from magpie import models
 
 if TYPE_CHECKING:
     from pyramid.httpexceptions import HTTPException
diff --git a/magpie/api/management/user/user_utils.py b/magpie/api/management/user/user_utils.py
index 77c610591..a53b03207 100644
--- a/magpie/api/management/user/user_utils.py
+++ b/magpie/api/management/user/user_utils.py
@@ -332,6 +332,7 @@ def get_user_service_resources_permissions_dict(user, service, request, inherit_
 
 def check_user_info(user_name, email, password, group_name):
     # type: (Str, Str, Str, Str) -> None
+    """Validates provided user information to ensure they are adequate for user creation."""
     ax.verify_param(user_name, not_none=True, not_empty=True, http_error=HTTPBadRequest,
                     param_name=u"user_name",
                     msg_on_fail=s.Users_CheckInfo_Name_BadRequestResponseSchema.description)
@@ -343,6 +344,8 @@ def check_user_info(user_name, email, password, group_name):
                     msg_on_fail=s.Users_CheckInfo_ReservedKeyword_BadRequestResponseSchema.description)
     ax.verify_param(email, not_none=True, not_empty=True, http_error=HTTPBadRequest,
                     param_name=u"email", msg_on_fail=s.Users_CheckInfo_Email_BadRequestResponseSchema.description)
+    ax.verify_param(email, regex_match=True, param_compare=r"", http_error=HTTPBadRequest,
+                    param_name=u"email", msg_on_fail=s.Users_CheckInfo_Email_BadRequestResponseSchema.description)
     ax.verify_param(password, not_none=True, not_empty=True, http_error=HTTPBadRequest,
                     param_name=u"password",
                     msg_on_fail=s.Users_CheckInfo_Password_BadRequestResponseSchema.description)
diff --git a/magpie/ui/management/templates/add_user.mako b/magpie/ui/management/templates/add_user.mako
index 92ac4c2f0..9fa6106b5 100644
--- a/magpie/ui/management/templates/add_user.mako
+++ b/magpie/ui/management/templates/add_user.mako
@@ -35,7 +35,7 @@
         <tr>
             <td>Email:</td>
             <td><div class="input-container"><label>
-                <input type="text" name="email" value="${form_user_email}" class="equal-width">
+                <input type="email" name="email" value="${form_user_email}" class="equal-width">
                 </label></div>
             </td>
             <td>
diff --git a/magpie/ui/management/templates/edit_service.mako b/magpie/ui/management/templates/edit_service.mako
index 013dd7ebb..29cfc7600 100644
--- a/magpie/ui/management/templates/edit_service.mako
+++ b/magpie/ui/management/templates/edit_service.mako
@@ -129,7 +129,7 @@
                         <span class="panel-entry">Protected URL: </span>
                         %if edit_mode == "edit_url":
                             <label>
-                            <input type="text" value="${service_url}" name="new_svc_url"
+                            <input type="url" value="${service_url}" name="new_svc_url"
                                    id="input_url" onkeyup="adjustWidth('input_url')">
                             <input class="button theme" type="submit" value="Save" name="save_url">
                             <input class="button cancel" type="submit" value="Cancel" name="no_edit">
diff --git a/tests/interfaces.py b/tests/interfaces.py
index 304e018ab..cedb0e59e 100644
--- a/tests/interfaces.py
+++ b/tests/interfaces.py
@@ -1,6 +1,6 @@
 import unittest
 import warnings
-from abc import ABCMeta
+from abc import ABCMeta, abstactmethod
 from copy import deepcopy
 from distutils.version import LooseVersion
 from typing import TYPE_CHECKING
@@ -25,7 +25,7 @@
     from magpie.typedefs import CookiesType, HeadersType, JSON, Optional, Str
 
 
-class Base_Magpie_TestCase(unittest.TestCase):
+class Base_Magpie_TestCase(six.with_metaclass(ABCMeta, unittest.TestCase)):
     """
     Base definition for all other Test Suite interfaces.
 
@@ -62,7 +62,8 @@ class as this method still gets called by some test runners although marked with
     __test__ = False    # won't run this as a test suite, only its derived classes that overrides to True
 
     @classmethod
-    def initClass(cls):  # noqa: N802
+    @abstactmethod
+    def initClass(cls):
         raise NotImplementedError
 
     @classmethod
@@ -2000,17 +2001,41 @@ class Interface_MagpieUI_UsersAuth(six.with_metaclass(ABCMeta, Base_Magpie_TestC
     Derived classes must implement :meth:`initClass` accordingly to generate the Magpie test application.
     """
 
+    def __init__(self, *args, **kwargs):
+        super(Interface_MagpieUI_UsersAuth, self).__init__(*args, **kwargs)
+        self.magpie_title = "Magpie User Management"
+
+    def setUp(self):
+        utils.TestSetup.create_TestUser(self)
+        test_credentials = utils.check_or_try_login_user(self, self.test_user_name, self.test_user_name)
+        self.test_headers, self.test_cookies = test_credentials
+        assert self.test_cookies, "Cannot test UI user pages without a logged user"
+        resp = utils.test_request(self, "GET", "/users/{}/groups".format(self.test_user_name))
+        body = utils.check_response_basic_info(resp)
+        utils.check_val_not_in(self.grp, body["group_names"],
+                               msg="Cannot test functionalities if test user has administrator access permissions.")
+
+    @classmethod
+    def check_requirements(cls):
+        headers, cookies = utils.check_or_try_login_user(cls, cls.usr, cls.pwd)
+        assert headers and cookies, cls.require             # nosec
+        assert cls.headers and cls.cookies, cls.require     # nosec
+
     @runner.MAGPIE_TEST_USERS
     @runner.MAGPIE_TEST_LOGGED
     def test_UserAccount_ViewDetails(self):
         """Logged user can view its own details on account page."""
-        raise NotImplementedError  # TODO
+        resp = utils.TestSetup.check_UpStatus(self, method="GET", path="/ui/users/current")
+        utils.check_val_is_in("Account User", resp.text)
+        utils.check_val_is_in(self.test_user_name, resp.text)
 
     @runner.MAGPIE_TEST_USERS
     @runner.MAGPIE_TEST_LOGGED
     def test_UserAccount_ViewDiscoverableGroupsMembership(self):
         """Logged user can view discoverable groups and which ones he has membership on."""
-        raise NotImplementedError  # TODO
+        resp = utils.TestSetup.check_UpStatus(self, method="GET", path="/ui/users/current")
+        utils.check_val_is_in("Account User", resp.text)
+        utils.check_val_is_in(self.test_user_name, resp.text)
 
     @runner.MAGPIE_TEST_USERS
     @runner.MAGPIE_TEST_LOGGED
diff --git a/tests/test_magpie_ui.py b/tests/test_magpie_ui.py
index b3e5f259c..4f2df16f7 100644
--- a/tests/test_magpie_ui.py
+++ b/tests/test_magpie_ui.py
@@ -67,8 +67,8 @@ def setUpClass(cls):
         cls.require = "cannot run tests without logged in user with '{}' permissions".format(cls.grp)
         cls.check_requirements()
         cls.version = utils.TestSetup.get_Version(cls)
-        cls.test_user = get_constant("MAGPIE_TEST_USER", default_value="unittest-user_users-auth-local")
-        cls.test_group = get_constant("MAGPIE_USERS_GROUP")
+        cls.test_user_name = get_constant("MAGPIE_TEST_USER", default_value="unittest-user_users-auth-local")
+        cls.test_group_name = get_constant("MAGPIE_USERS_GROUP")
 
 
 @runner.MAGPIE_TEST_UI
diff --git a/tests/test_register.py b/tests/test_register.py
index 36b2e896e..733384818 100644
--- a/tests/test_register.py
+++ b/tests/test_register.py
@@ -363,6 +363,7 @@ def test_get_all_config_from_dict(self):
 @runner.MAGPIE_TEST_LOCAL
 @runner.MAGPIE_TEST_REGISTER
 def test_register_make_config_registry():
+    # pylint: disable=W0212
     assert register._make_config_registry(None, "whatever") == {}
     assert register._make_config_registry([], "whatever") == {}
     assert register._make_config_registry([{}], "whatever") == {}
diff --git a/tests/utils.py b/tests/utils.py
index 9473a9d9a..e0b070c8b 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -28,9 +28,10 @@
 if TYPE_CHECKING:
     # pylint: disable=W0611,unused-import
     from tests.interfaces import Base_Magpie_TestCase  # noqa: F401
+    from typing import Any, Callable, Dict, Iterable, List, NoReturn, Optional, Type, Union  # noqa: F401
     from magpie.typedefs import (  # noqa: F401
-        Any, AnyMagpieTestType, AnyHeadersType, AnyResponseType, Callable, Dict, Iterable, HeadersType, JSON, List,
-        Optional, OptionalHeaderCookiesType, SettingsType, Str, TestAppOrUrlType, Type, Union
+        AnyMagpieTestType, AnyHeadersType, AnyResponseType, HeadersType, JSON,
+        OptionalHeaderCookiesType, SettingsType, Str, TestAppOrUrlType
     )
 
 OptionalStringType = six.string_types + tuple([type(None)])  # pylint: disable=C0103,invalid-name   # noqa: 802
@@ -303,20 +304,20 @@ def check_or_try_login_user(test_item,                      # type: AnyMagpieTes
                             expect_errors=False,            # type: bool
                             ):                              # type: (...) -> OptionalHeaderCookiesType
     """
-    Verifies that the required user is already logged in (or none is if username=None), or tries to login him otherwise.
-    Validates that the logged user (if any), matched the one specified with `username`.
+    Verifies that the required user is already logged in (or none is if ``username=None``), or attempts to log him in
+    otherwise. Validates that the logged user (if any), matched the one specified by :paramref:`username`.
 
     :param test_item: instance of the test application or remote server URL to call
     :param username: name of the user to login or None otherwise
     :param password: password to use for login if the user was not already logged in
-    :param provider: provider string to use for login (default: `MAGPIE_DEFAULT_PROVIDER`, ie: magpie's local signin)
+    :param provider: provider string to use for login (default: ``MAGPIE_DEFAULT_PROVIDER``, ie: magpie's local signin)
     :param headers: headers to include in the test request
     :param use_ui_form_submit: use Magpie UI login 'form' to obtain cookies
-        (required for local `WebTest.App` login, ignored by requests using URL)
+        (required for local :class:`WebTest.TestApp` login, ignored by requests using URL)
     :param version: server or local app version to evaluate responses with backward compatibility
     :param expect_errors: indicate if the login is expected to fail, used only if using UI form & `webtest.TestApp`
     :return: headers and cookies of the user session or (None, None)
-    :raise: Exception on any login failure as required by the caller's specifications (username/password)
+    :raise AssertionError: if login failed or logged user session does not meet specifications (username/password)
     """
     app_or_url = get_app_or_url(test_item)
     headers = headers or {}
@@ -359,7 +360,7 @@ def check_or_try_login_user(test_item,                      # type: AnyMagpieTes
         else:
             logged_user = body.get("user_name", "")
         if username != logged_user:
-            raise Exception("invalid user")
+            raise AssertionError("invalid user")
         if isinstance(app_or_url, TestApp):
             resp_cookies = app_or_url.cookies
         else:
@@ -523,14 +524,21 @@ def check_response_basic_info(response, expected_code=200, expected_type=CONTENT
     return json_body
 
 
-def check_ui_response_basic_info(response, expected_code=200, expected_type=CONTENT_TYPE_HTML):
+def check_ui_response_basic_info(response, expected_code=200, expected_type=CONTENT_TYPE_HTML,
+                                 expected_title="Magpie Administration"):
+    # type: (AnyResponseType, int, Str, Str) -> Optional[NoReturn]
+    """
+    Validates minimal expected element in a `Magpie` UI page.
+
+    :raises AssertionError: if any of the expected validation elements does not meet requirement.
+    """
     msg = None \
         if get_header("Content-Type", response.headers) != CONTENT_TYPE_JSON \
         else "Response body: {}".format(get_json_body(response))
     check_val_equal(response.status_code, expected_code, msg=msg)
     check_val_is_in("Content-Type", dict(response.headers))
     check_val_is_in(expected_type, get_response_content_types_list(response))
-    check_val_is_in("Magpie Administration", response.text, msg=null)   # don't output big html if failing
+    check_val_is_in(expected_title, response.text, msg=null)   # don't output big html if failing
 
 
 class NullType(six.with_metaclass(SingletonMeta)):
@@ -672,15 +680,18 @@ def get_Version(test_class):
 
     @staticmethod
     def check_UpStatus(test_class, method, path, timeout=20):
+        # type: (TestAppOrUrlType, Str, Str, int) -> AnyResponseType
         """
-        Verifies that the Magpie UI page at very least returned an Ok response with the displayed title. Validates that
-        at the bare minimum, no underlying internal error occurred from the API or UI calls.
+        Verifies that the Magpie UI page at very least returned an HTTP Ok response with the displayed title.
+        Validates that at the bare minimum, no underlying internal error occurred from the API or UI calls.
 
         :returns: response from the rendered page for further tests
         """
         app_or_url = get_app_or_url(test_class)
-        resp = test_request(app_or_url, method, path, cookies=test_class.cookies, timeout=timeout)
-        check_ui_response_basic_info(resp)
+        cookies = getattr(test_class, "test_cookies", getattr(test_class, "cookies"))
+        resp = test_request(app_or_url, method, path, cookies=cookies, timeout=timeout)
+        kw_args = {"expected_title": getattr(test_class, "magpie_title")} if hasattr(test_class, "magpie_title") else {}
+        check_ui_response_basic_info(resp, **kw_args)
         return resp
 
     @staticmethod