Merge pull request #38 from onaio/dserkowski-OLMIS-6504

OLIMS-6504: Added oauth-init endpoint for auto-approve

.gitignore

@@ -103,5 +103,8 @@ venv.bak/
 # mypy
 .mypy_cache/
 
+# others
+tests/.superset/
+
 # vscode
 .vscode/

README.md

@@ -116,6 +116,13 @@ Ketchup's CustomSecurityManager class includes a custom `oauth_user_info` method
 - `openlmis`
 - `OpenSRP`
 
+### Custom oAuth init endpoint
+
+One of the ideas of integration Superset with an external service (eg. OpenLMIS) is to allow running the application and sign-in directly from the external application's API. To simplify the whole process, Ketchup provides the endpoint `/oauth-init/<provider>`. It is a backend functionality that essentially does what the endpoint `/login/<provider>` does, however it doesn't redirect a client. Instead of it, it returns information in the JSON form which contains the following fields:
+
+- `isAuthorized` - the flag which provides information about user's authorization in the app
+- `state` - oAuth state which is required during the oAuth sign-in process. It is provided only if `isAuthorized` is false
+
 #### PATCHUP_EMAIL_BASE
 
 In cases where an oAuth provider does not provide an email address for its users, Superset's oAuth process might fail.  To remedy this, Superset-patchup you can set the `PATCHUP_EMAIL_BASE` variable in `superset_config.py`.

requirements/dev.in

@@ -7,6 +7,7 @@ pylint-flask
 tox
 flake8
 pep8
+pydocstyle
 coverage
 yapf
 isort
@@ -20,3 +21,4 @@ urllib3>=1.24.2
 sqlalchemy>=1.3.0
 jinja2>=2.10.1
 parso>=0.5.0
+pyjwt

requirements/dev.txt

@@ -2,14 +2,13 @@
 # This file is autogenerated by pip-compile
 # To update, run:
 #
-#    pip-compile --output-file requirements/dev.txt requirements/dev.in
+#    pip-compile requirements/dev.in
 #
 
 alembic==1.0.10           # via flask-migrate
 amqp==2.4.2               # via kombu
 apispec[yaml]==1.3.3      # via flask-appbuilder
 appdirs==1.4.3            # via black
-appnope==0.1.0            # via ipython
 asn1crypto==0.24.0        # via cryptography
 astroid==2.2.5            # via pylint, pylint-flask
 atomicwrites==1.3.0       # via pytest
@@ -99,11 +98,12 @@ ptyprocess==0.6.0         # via pexpect
 py==1.8.0                 # via pytest, tox
 pycodestyle==2.5.0        # via flake8
 pycparser==2.19           # via cffi
+pydocstyle==4.0.1
 pydruid==0.5.2            # via superset
 pyflakes==2.1.1           # via flake8
 pygments==2.3.1           # via ipython
 pyhive==0.6.1             # via superset
-pyjwt==1.7.1              # via flask-appbuilder, flask-jwt-extended
+pyjwt==1.7.1
 pylint-flask==0.6
 pylint-plugin-utils==0.5  # via pylint-flask
 pylint==2.3.1
@@ -121,6 +121,7 @@ s3transfer==0.1.13        # via boto3
 sasl==0.2.1               # via thrift-sasl
 simplejson==3.16.0        # via superset
 six==1.12.0               # via astroid, bleach, cryptography, flask-jwt-extended, isodate, jsonlines, jsonschema, linear-tsv, pathlib2, pip-tools, polyline, prison, prompt-toolkit, pydruid, pyrsistent, pytest, python-dateutil, sasl, sqlalchemy-utils, tableschema, tabulator, thrift, tox, traitlets
+snowballstemmer==1.9.0    # via pydocstyle
 sqlalchemy-utils==0.33.11  # via superset
 sqlalchemy==1.3.3
 sqlparse==0.3.0           # via superset
@@ -147,3 +148,6 @@ wtforms==2.2.1            # via flask-wtf
 xlrd==1.2.0               # via tabulator
 yapf==0.27.0
 zipp==0.4.0               # via importlib-metadata
+
+# The following packages are considered to be unsafe in a requirements file:
+# setuptools==41.1.0        # via ipdb, ipython, jsonschema, markdown, pytest, tox

superset_patchup/__init__.py

@@ -1,5 +1,5 @@
 """
 Main init file for superset_patchup
 """
-VERSION = (0, 1, 4)
+VERSION = (0, 1, 5)
 __version__ = ".".join(str(v) for v in VERSION)

superset_patchup/oauth.py

@@ -2,7 +2,8 @@
 import logging
 import re
 
-from flask import abort, flash, g, redirect, request, session, url_for
+from flask import abort, flash, g, redirect, request, session, url_for, \
+    jsonify, make_response
 
 from flask_appbuilder._compat import as_unicode
 from flask_appbuilder.security.sqla import models as ab_models
@@ -49,11 +50,7 @@ def login(self, provider=None, register=None):
                 appbuilder=self.appbuilder,
             )
         logging.debug(f"Going to call authorize for: {provider}")
-        state = jwt.encode(
-            request.args.to_dict(flat=False),
-            self.appbuilder.app.config["SECRET_KEY"],
-            algorithm="HS256",
-        )
+        state = self.generate_state()
         try:
             if register:
                 logging.debug("Login to Register")
@@ -80,6 +77,39 @@ def login(self, provider=None, register=None):
             flash(as_unicode(self.invalid_login_message), "warning")
             return redirect(redirect_url)
 
+    @expose("/oauth-init/<provider>")
+    def login_init(self, provider=None):
+        """
+        Checks authorization and if a user is not authorized,
+        inits the sign-in process and returns a state
+        """
+        logging.debug("Provider: %s", provider)
+
+        if g.user is not None and g.user.is_authenticated:
+            logging.debug("Provider %s is already authorized by %s",
+                          provider, g.user)
+            return make_response(jsonify(
+                isAuthorized=True
+            ))
+
+        redirect_url = request.args.get("redirect_url")
+        if not redirect_url or not is_safe_url(redirect_url):
+            logging.debug("The arg redirect_url not found or not safe")
+            return abort(400)
+
+        logging.debug("Initialization of authorization process for: %s",
+                      provider)
+
+        # libraries assume that
+        # 'redirect_url' should be available in the session
+        session['%s_oauthredir' % provider] = redirect_url
+
+        state = self.generate_state()
+        return make_response(jsonify(
+            isAuthorized=False,
+            state=state
+        ))
+
     @expose("/oauth-authorized/<provider>")
     # pylint: disable=too-many-branches
     # pylint: disable=logging-fstring-interpolation
@@ -141,6 +171,16 @@ def oauth_authorized(self, provider):
 
         return redirect(self.appbuilder.get_url_for_index)
 
+    def generate_state(self):
+        """
+        Generates a state which is required during the OAuth sign-in process
+        """
+        return jwt.encode(
+            request.args.to_dict(flat=False),
+            self.appbuilder.app.config["SECRET_KEY"],
+            algorithm="HS256",
+        )
+
 
 class CustomSecurityManager(SupersetSecurityManager):
     """Custom Security Manager Class"""

tests/oauth/test_oauth.py

@@ -1,7 +1,7 @@
 """
 This module tests oauth
 """
-from unittest.mock import MagicMock, patch, call
+from unittest.mock import MagicMock, patch, call, PropertyMock
 
 from superset import app
 
@@ -340,3 +340,86 @@ def test_is_valid_provider_is_called_for_opendata(self, function_mock):
         assert call("opensrp", "OpenSRP") in function_mock.call_args_list
         csm.oauth_user_info(provider="OPENLMIS")
         assert call("OPENLMIS", "openlmis") in function_mock.call_args_list
+
+    @patch("superset_patchup.oauth.jwt")
+    @patch("superset_patchup.oauth.jsonify")
+    @patch("superset_patchup.oauth.make_response")
+    @patch("superset_patchup.oauth.g")
+    @patch("superset_patchup.oauth.is_safe_url")
+    @patch("superset_patchup.oauth.request.args.get")
+    @patch("superset_patchup.oauth.request")
+    def test_init_login_result_for_not_authorized_user(
+            self,
+            mock_request,
+            mock_redirect_arg,
+            mock_safe_url,
+            mock_g,
+            mock_make_response,
+            mock_jsonify,
+            mock_jwt
+    ):
+        """
+        Test that checks if the correct response for a not authorized user is returned.
+        """
+        oauth_view = AuthOAuthView()
+        oauth_view.appbuilder = MagicMock()
+
+        provider = "OPENLMIS"
+        redirect_url = "/superset/dashboard/3"
+        state = '12345'
+
+        with patch('superset_patchup.oauth.g.user.is_authenticated', False):
+            with patch("superset_patchup.oauth.session", dict()) as session:
+                mock_redirect_arg.return_value = redirect_url
+                mock_jwt.encode.return_value = state
+
+                oauth_view.login_init(provider=provider)
+
+                mock_make_response.assert_called()
+                assert call(isAuthorized=False, state=state) in mock_jsonify.call_args_list
+                assert session.get('%s_oauthredir' % provider) == redirect_url
+
+    @patch("superset_patchup.oauth.jsonify")
+    @patch("superset_patchup.oauth.make_response")
+    @patch("superset_patchup.oauth.g")
+    def test_init_login_result_for_already_authorized_user(
+            self,
+            mock_g,
+            mock_make_response,
+            mock_jsonify
+    ):
+        """
+        Test that checks if the correct response for an already authorized user is returned.
+        """
+        oauth_view = AuthOAuthView()
+        oauth_view.appbuilder = MagicMock()
+        provider = "OPENLMIS"
+
+        with patch('superset_patchup.oauth.g.user.is_authenticated', True):
+            with patch("superset_patchup.oauth.session", dict()) as session:
+
+                oauth_view.login_init(provider=provider)
+
+                mock_make_response.assert_called()
+                assert call(isAuthorized=True) in mock_jsonify.call_args_list
+                assert (('%s_oauthredir' % provider) in session) is False
+
+    @patch("superset_patchup.oauth.request")
+    def test_generate_state_result(
+            self,
+            mock_request,
+    ):
+        """
+        Test that checks if a valid state is returned.
+        """
+        oauth_view = AuthOAuthView()
+        oauth_view.appbuilder = MagicMock()
+        app_config = dict(SECRET_KEY='secret_key')
+        request_args = dict(dummy_parameter='dummy_parameter_value')
+
+        type(oauth_view.appbuilder.app).config = PropertyMock(return_value=app_config)
+        mock_request.args.to_dict.return_value = request_args
+
+        state = oauth_view.generate_state()
+
+        assert len(state) > 0