From 2543ec96270286623cda0d284e9b63a3ad36d779 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:19:32 -0400 Subject: [PATCH 01/11] Set up GitHub actions (initial attempt) --- .github/workflows/run-tests.yml | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/workflows/run-tests.yml diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml new file mode 100644 index 0000000..419e905 --- /dev/null +++ b/.github/workflows/run-tests.yml @@ -0,0 +1,43 @@ +name: Run tests + +on: + push: + branches: [master, develop] + pull_request: + branches: [master, develop] + +jobs: + build: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [3.7, 3.8, 3.9, 3.10] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r test_requirements.txt + - name: Create settings and log files + run: | + cp settings.py.template settings.py + mkdir logs; touch logs/faculty-tools.log + - name: Lint with flake8 + run: flake8 + - name: Check formatting + uses: psf/black@stable + - name: Check import sorting + uses: jamescurtin/isort-action@master + - name: Lint markdown files + uses: bewuethr/mdl-action@v1 + - name: Run tests + run: coverage run -m unittest discover + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + fail_ci_if_error: true From 457d0dd9797efe5d1573367e77017a581639ac69 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:23:26 -0400 Subject: [PATCH 02/11] Remove travis.yml. Make python versions strings --- .github/workflows/run-tests.yml | 2 +- .travis.yml | 23 ----------------------- 2 files changed, 1 insertion(+), 24 deletions(-) delete mode 100644 .travis.yml diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 419e905..45dc429 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.7, 3.8, 3.9, 3.10] + python-version: ['3.7', '3.8', '3.9', '3.10'] steps: - uses: actions/checkout@v2 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 802f960..0000000 --- a/.travis.yml +++ /dev/null @@ -1,23 +0,0 @@ -language: python -matrix: - include: - - python: 3.7 - - python: 3.8 -install: -- pip install -r test_requirements.txt -- pip install coveralls -- gem install mdl -before_script: -- cp settings.py.template settings.py; cp whitelist.json.template whitelist.json -- mkdir logs; touch logs/faculty-tools.log -script: -- flake8 -- black --check . -- mdl . -- coverage run -m unittest discover -after_success: -- coveralls -notifications: - slack: - rooms: - secure: F3YANiuNHZjtbgiJC8M1JOKBKhct2EyDEkCitW4FMwrauV0G5XcWKqtqLyRzSchI1LUALn+Dnj032ElGEx7D7cJrXlqC700UjQ4wXv938GQObVnRfTVCVrsktpr5gf077dIXvwcYJYt9ZSu4uIyk+HqyNnHLX4qIL0zhFR8HytoOeXVdik35SQoLJLvorgf4EGfqU8Yo25LUJArp2AB7RceAiMg3QXmi+nDHumFFczURexaYXIDBrRYTyZgfpYP245HOUmEf/LD6G53e6FU+8hiISYr7nE0hTkNkj3U4WNga25//9VIdpjWW8VWd+G7vf8CuhzHYuWEtredoVqNnJDwKE/MLhixleA+1lAEUypAEp+0k3+zMfTk748gdl1buJ/kINjoNqjhLv6MtDH/YTw/eQKEXA+V+odoudDiHUCztHQbCaIXYmIDFnebzO9u/Gz7QJ7PfpBlmUASQru5qTFPL0tmHP/w6/zrog0n07+uwl4qo2d6qxslgtmw4+K0VxGXl1Z8STArEgD6a8KoTZ1N8XDFF0E7KXE3kGYEtLHNoV7Z9OaohB3AFwJaKPTytYyIQ+OPvzYmpyUwRGIWBfRIP7t9qemyOExbYoinw0rF7jCMm7T3gLxkwxHNOCt+Gc1kwfLvuDy8QhQR2iHspMM8NeuDQKVzK4L3FeLx7bTo= From fee63da9effec977dad061bca75c63c87cc56fee Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:32:26 -0400 Subject: [PATCH 03/11] Add isort. Add pyproject.toml to configure black and isort. run isort. --- lti.py | 18 +++++++++--------- pyproject.toml | 6 ++++++ test_requirements.txt | 1 + tests.py | 12 ++++++------ utils.py | 5 ++--- 5 files changed, 24 insertions(+), 18 deletions(-) create mode 100644 pyproject.toml diff --git a/lti.py b/lti.py index 3b98987..8914491 100644 --- a/lti.py +++ b/lti.py @@ -1,28 +1,28 @@ -from logging import Formatter, INFO -from logging.handlers import RotatingFileHandler import json import os import time +from logging import INFO, Formatter +from logging.handlers import RotatingFileHandler +import jinja2 +import requests +import settings from canvasapi.exceptions import CanvasException from flask import ( Flask, + Response, + redirect, render_template, - session, request, - redirect, - url_for, - Response, send_from_directory, + session, + url_for, ) from flask_sqlalchemy import SQLAlchemy -import jinja2 from pylti.flask import lti -import requests from requests.exceptions import HTTPError from utils import filter_tool_list, slugify -import settings app = Flask(__name__) app.config.from_object(settings.configClass) diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..389d764 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,6 @@ +[tool.black] +line-length = 88 +target_version = ['py37', 'py38', 'py39'] + +[tool.isort] +profile = "black" diff --git a/test_requirements.txt b/test_requirements.txt index 7330fb4..b89f9d0 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -5,6 +5,7 @@ blinker coverage flake8 Flask-Testing>=0.8.0 +isort mock oauthlib requests-mock diff --git a/tests.py b/tests.py index c7c09e4..fcfbe69 100644 --- a/tests.py +++ b/tests.py @@ -1,20 +1,20 @@ -from json.decoder import JSONDecodeError import logging +import time import unittest +from json.decoder import JSONDecodeError from urllib.parse import urlencode import canvasapi -import oauthlib.oauth1 import flask -from flask import url_for import flask_testing +import oauthlib.oauth1 import requests_mock +import settings +from flask import url_for +from mock import mock_open, patch from pylti.common import LTI_SESSION_KEY -import time -from mock import patch, mock_open import lti -import settings import utils diff --git a/utils.py b/utils.py index aca6e28..792bbbf 100644 --- a/utils.py +++ b/utils.py @@ -1,10 +1,9 @@ -from collections import defaultdict import json import re - -from canvasapi import Canvas +from collections import defaultdict import settings +from canvasapi import Canvas def get_tool_info(whitelist, tool_name): From dfd5673f2db333d5f522846593bf6898744994b5 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:34:21 -0400 Subject: [PATCH 04/11] Ignore src dir for black and isort --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 389d764..7707c56 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,8 @@ [tool.black] line-length = 88 target_version = ['py37', 'py38', 'py39'] +exclude = "src" [tool.isort] profile = "black" +skip = "src" From f15de8e14d86c06213d75364ffa5b308ed3db9c1 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:36:29 -0400 Subject: [PATCH 05/11] Update default settings.py template to be black-compliant --- settings.py.template | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/settings.py.template b/settings.py.template index ce34aac..e0221f8 100644 --- a/settings.py.template +++ b/settings.py.template @@ -17,11 +17,7 @@ SHARED_SECRET = "secret" # Configuration for pylti library. Uses the above key and secret PYLTI_CONFIG = { - "consumers": { - CONSUMER_KEY: { - "secret": SHARED_SECRET - } - }, + "consumers": {CONSUMER_KEY: {"secret": SHARED_SECRET}}, # Custom configurable roles "roles": { "staff": [ From bd0eb49fa3da361373c71f9e88b55593bdb2d9bb Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:42:42 -0400 Subject: [PATCH 06/11] fix settings import location --- lti.py | 2 +- tests.py | 2 +- utils.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lti.py b/lti.py index 8914491..920db0e 100644 --- a/lti.py +++ b/lti.py @@ -6,7 +6,6 @@ import jinja2 import requests -import settings from canvasapi.exceptions import CanvasException from flask import ( Flask, @@ -22,6 +21,7 @@ from pylti.flask import lti from requests.exceptions import HTTPError +import settings from utils import filter_tool_list, slugify app = Flask(__name__) diff --git a/tests.py b/tests.py index fcfbe69..17c1088 100644 --- a/tests.py +++ b/tests.py @@ -9,12 +9,12 @@ import flask_testing import oauthlib.oauth1 import requests_mock -import settings from flask import url_for from mock import mock_open, patch from pylti.common import LTI_SESSION_KEY import lti +import settings import utils diff --git a/utils.py b/utils.py index 792bbbf..b7dcc33 100644 --- a/utils.py +++ b/utils.py @@ -2,9 +2,10 @@ import re from collections import defaultdict -import settings from canvasapi import Canvas +import settings + def get_tool_info(whitelist, tool_name): """ From a9f24942665e0e07f58559ef99bcf29804568683 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:46:52 -0400 Subject: [PATCH 07/11] Remove 'editable' flag on pylti --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b4f768f..a4badfc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,6 @@ canvasapi==0.15.0 Flask==1.1.1 Flask-SQLAlchemy==2.4.1 mysqlclient --e git+https://github.com/ucfcdl/pylti.git@roles#egg=PyLTI +git+https://github.com/ucfcdl/pylti.git@roles#egg=PyLTI requests==2.22.0 Werkzeug>=1.0.1 # Chrome 80 SameSite fix From eb37ebe3da986fb5b803ae393c7a7e01040392ed Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 12:50:47 -0400 Subject: [PATCH 08/11] Upgrade requirements --- requirements.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index a4badfc..9902bfe 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ -canvasapi==0.15.0 -Flask==1.1.1 -Flask-SQLAlchemy==2.4.1 +canvasapi==3.0.0 +Flask==2.2.2 +Flask-SQLAlchemy==3.0.2 mysqlclient git+https://github.com/ucfcdl/pylti.git@roles#egg=PyLTI requests==2.22.0 From 272b607d92719ae4354be9310b23689b972e0fea Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 13:16:35 -0400 Subject: [PATCH 09/11] Create whitelist in actions. Update setup shortcut to use FLASK_DEBUG and remove old venv pattern --- .github/workflows/run-tests.yml | 3 ++- setup.sh | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 45dc429..b2587d2 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -23,9 +23,10 @@ jobs: run: | python -m pip install --upgrade pip pip install -r test_requirements.txt - - name: Create settings and log files + - name: Create settings, whitelist, and log files run: | cp settings.py.template settings.py + cp whitelist.json.template whitelist.json mkdir logs; touch logs/faculty-tools.log - name: Lint with flake8 run: flake8 diff --git a/setup.sh b/setup.sh index b586eda..786baf0 100644 --- a/setup.sh +++ b/setup.sh @@ -1,3 +1,2 @@ -source env/bin/activate export FLASK_APP=lti.py -export FLASK_ENV=development +export FLASK_DEBUG=1 From 24e99e446dcb03b0a6b6670c64afad75f7464f1b Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 15:17:37 -0400 Subject: [PATCH 10/11] Patch assert_redirects to handle relative urls --- tests.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/tests.py b/tests.py index 17c1088..b70ed38 100644 --- a/tests.py +++ b/tests.py @@ -86,6 +86,18 @@ def generate_launch_request( new_url = signed_url[len(base_url) :] return new_url + def assert_redirects_new(self, response, location, message=None): + valid_status_codes = (301, 302, 303, 305, 307) + valid_status_code_str = ", ".join(str(code) for code in valid_status_codes) + not_redirect = "HTTP Status %s expected but got %d" % ( + valid_status_code_str, + response.status_code, + ) + self.assertTrue( + response.status_code in valid_status_codes, message or not_redirect + ) + self.assertEqual(response.location, location, message) + def test_select_theme_dirs(self, m): theme_dirs = lti.select_theme_dirs() @@ -257,7 +269,7 @@ def test_index_api_key_expired(self, m): redirect_url = ( "{}login/oauth2/auth?client_id={}&response_type=code&redirect_uri={}" ) - self.assert_redirects( + self.assert_redirects_new( response, redirect_url.format( settings.BASE_URL, settings.oauth2_id, settings.oauth2_uri @@ -297,7 +309,7 @@ def test_index_api_key_404(self, m): redirect_url = ( "{}login/oauth2/auth?client_id={}&response_type=code&redirect_uri={}" ) - self.assert_redirects( + self.assert_redirects_new( response, redirect_url.format( settings.BASE_URL, settings.oauth2_id, settings.oauth2_uri @@ -592,7 +604,7 @@ def test_oauth_login_new_user(self, m): ) ) - self.assert_redirects(response, url_for("index")) + self.assert_redirects_new(response, url_for("index")) # Check that user is created user = lti.Users.query.filter_by( @@ -701,7 +713,7 @@ def test_oauth_login_existing_user(self, m): ) ) - self.assert_redirects(response, url_for("index")) + self.assert_redirects_new(response, url_for("index")) self.assertGreater(user.expires_in, old_expire) def test_oauth_login_existing_user_db_error(self, m): @@ -905,7 +917,7 @@ def test_auth_no_user(self, m): redirect_url = ( "{}login/oauth2/auth?client_id={}&response_type=code&redirect_uri={}" ) - self.assert_redirects( + self.assert_redirects_new( response, redirect_url.format( settings.BASE_URL, settings.oauth2_id, settings.oauth2_uri @@ -950,7 +962,7 @@ def test_auth_no_api_key_refresh_success(self, m, mock_refresh_access_token): self.assertEqual(flask.session["api_key"], new_access_token) self.assertEqual(flask.session["expires_in"], new_expiry_date) - self.assert_redirects(response, url_for("index")) + self.assert_redirects_new(response, url_for("index")) @patch("lti.refresh_access_token") def test_auth_no_api_key_refresh_fail(self, m, mock_refresh_access_token): @@ -987,7 +999,7 @@ def test_auth_no_api_key_refresh_fail(self, m, mock_refresh_access_token): redirect_url = ( "{}login/oauth2/auth?client_id={}&response_type=code&redirect_uri={}" ) - self.assert_redirects( + self.assert_redirects_new( response, redirect_url.format( settings.BASE_URL, settings.oauth2_id, settings.oauth2_uri @@ -1038,7 +1050,7 @@ def test_auth_invalid_api_key_refresh_success(self, m, mock_refresh_access_token data=payload, ) - self.assertRedirects(response, url_for("index")) + self.assert_redirects_new(response, url_for("index")) @patch("lti.refresh_access_token") def test_auth_invalid_api_key_refresh_fail(self, m, mock_refresh_access_token): @@ -1085,7 +1097,7 @@ def test_auth_invalid_api_key_refresh_fail(self, m, mock_refresh_access_token): redirect_url = ( "{}login/oauth2/auth?client_id={}&response_type=code&redirect_uri={}" ) - self.assert_redirects( + self.assert_redirects_new( response, redirect_url.format( settings.BASE_URL, settings.oauth2_id, settings.oauth2_uri @@ -1123,7 +1135,7 @@ def test_auth(self, m): data=payload, ) - self.assert_redirects(response, url_for("index")) + self.assert_redirects_new(response, url_for("index")) # get_sessionless_url def test_get_sessionless_url_is_course_nav_fail(self, m): From 894d98b8f0d219f683815775a65257aeef35e781 Mon Sep 17 00:00:00 2001 From: Matthew Emond Date: Thu, 20 Oct 2022 15:27:47 -0400 Subject: [PATCH 11/11] Update GitHub Actions checkout and setup-python versions --- .github/workflows/run-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index b2587d2..20668e8 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -14,9 +14,9 @@ jobs: python-version: ['3.7', '3.8', '3.9', '3.10'] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Install dependencies