From 7322f929849da575fee95d31a92d37163c240f60 Mon Sep 17 00:00:00 2001
From: Mark <mark@openappstack.net>
Date: Fri, 25 Oct 2019 14:53:46 +0200
Subject: [PATCH] Refactor is safe url and add helper function unittest

---
 .gitlab-ci.yml                               |  7 +++++++
 login_provider/app.py                        |  9 +--------
 login_provider/helper.py                     | 11 +++++++++++
 login_provider/test/__init__.py              |  0
 login_provider/test/test_helper_functions.py | 17 +++++++++++++++++
 5 files changed, 36 insertions(+), 8 deletions(-)
 create mode 100644 login_provider/helper.py
 create mode 100644 login_provider/test/__init__.py
 create mode 100644 login_provider/test/test_helper_functions.py

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f07a6c6..1f1ea87 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -55,6 +55,13 @@ login-provider-test:
       - login_provider/**/*
       - .gitlab-ci.yml
 
+unittest-login:
+  stage: application-test
+  image: ${CI_REGISTRY_IMAGE}/login_provider_ci_test:${CI_COMMIT_REF_NAME}
+  script:
+    - cd login_provider
+    - python3 -m unittest discover
+
 behave-login:
   stage: application-test
   variables:
diff --git a/login_provider/app.py b/login_provider/app.py
index ce92fa6..0f2c2a8 100644
--- a/login_provider/app.py
+++ b/login_provider/app.py
@@ -4,6 +4,7 @@ from hydra_client import HydraAdmin
 from flask_login import login_user, logout_user, LoginManager, login_required, current_user
 from db import User
 from forms import LoginForm, LogoutForm
+from helper import is_safe_url
 
 HYDRA_ADMIN_URL = environ['HYDRA_ADMIN_URL']
 hydra = HydraAdmin(HYDRA_ADMIN_URL)
@@ -47,14 +48,6 @@ def login():
     login_form.next_url.data = request.args.get('next')
     return render_template('login.html', login_form=login_form)
 
-def is_safe_url(url):
-    print(url)
-    safe = True if url == "" else False
-    safe = True if url == "/" or safe else False
-    safe = True if url[:18] == "/?login_challenge=" \
-                   and url[18:].isalnum() or safe else False
-    return safe
-
 @app.route('/logout', methods=['POST'])
 def logout():
     logout_form = LogoutForm()
diff --git a/login_provider/helper.py b/login_provider/helper.py
new file mode 100644
index 0000000..491bc56
--- /dev/null
+++ b/login_provider/helper.py
@@ -0,0 +1,11 @@
+import re
+
+def is_safe_url(url):
+    safe_urls = [
+        "^[/]*$",                             # Home page
+        "^/\?login_challenge=[a-z|A-Z|0-9]+$" # Login challenge with alphanumeric code
+        ]
+    for safe_url in safe_urls:
+        if re.fullmatch(safe_url, url) is not None:
+            return True
+    return False
diff --git a/login_provider/test/__init__.py b/login_provider/test/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/login_provider/test/test_helper_functions.py b/login_provider/test/test_helper_functions.py
new file mode 100644
index 0000000..34acc3d
--- /dev/null
+++ b/login_provider/test/test_helper_functions.py
@@ -0,0 +1,17 @@
+import unittest
+from helper import is_safe_url
+
+class UnitTests(unittest.TestCase):
+
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    def test_safe_urls(self):
+        self.assertTrue(is_safe_url("/"))
+        self.assertTrue(is_safe_url("/?login_challenge=9a8s9da8s9dhahsda"))
+        self.assertFalse(is_safe_url("/malicious"))
+        self.assertFalse(is_safe_url("/?login_challenge=Not_alpha_numeric"))
+
-- 
GitLab