From 25db6fabbe28b390877072e85191ca15add3bb3c Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 11 Jun 2018 15:54:16 -0400 Subject: [PATCH 1/9] WIP: authentication handling for ATST --- atst/app.py | 4 +++- atst/handler.py | 33 +++++++++++++++++++++++++++++++++ atst/handlers/login.py | 10 ++++++++++ atst/handlers/main.py | 4 +++- atst/handlers/request.py | 3 ++- atst/handlers/request_new.py | 3 ++- atst/handlers/workspace.py | 6 ++---- tests/test_auth.py | 34 ++++++++++++++++++++++++++++++++++ 8 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 atst/handlers/login.py create mode 100644 tests/test_auth.py diff --git a/atst/app.py b/atst/app.py index 2cbe86f0..07f35cc2 100644 --- a/atst/app.py +++ b/atst/app.py @@ -4,6 +4,7 @@ import tornado.web from tornado.web import url from atst.handlers.main import MainHandler +from atst.handlers.login import Login from atst.handlers.workspace import Workspace from atst.handlers.request import Request from atst.handlers.request_new import RequestNew @@ -16,7 +17,8 @@ def make_app(config): authz_client = ApiClient(config['default']['AUTHZ_BASE_URL']) app = tornado.web.Application([ - url( r"/", MainHandler, {'page': 'login'}, name='login' ), + url( r"/", Login, {'page': 'login'}, name='main' ), + url( r"/login", Login, {'page': 'login'}, name='login' ), url( r"/home", MainHandler, {'page': 'home'}, name='home' ), url( r"/workspaces", Workspace, diff --git a/atst/handler.py b/atst/handler.py index baa6f0ef..8af83bf3 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -1,4 +1,5 @@ import os +import functools from webassets import Environment, Bundle import tornado.web from atst.home import home @@ -19,9 +20,41 @@ helpers = { 'assets': assets } +def authenticated(method): + @functools.wraps(method) + def wrapper(self, *args, **kwargs): + if not self.current_user: + if self.get_cookie('bearer-token'): + bearer_token = self.get_cookie('bearer-token') + if validate_login_token(bearer_token): + self._start_session() + else: + raise NotImplementedError + elif self.request.method in ("GET", "HEAD"): + url = self.get_login_url() + self.redirect(url) + return + else: + raise tornado.web.HTTPError(403) + return method(self, *args, **kwargs) + return wrapper + +def validate_login_token(token): + # check against authnid + pass + class BaseHandler(tornado.web.RequestHandler): def get_template_namespace(self): ns = super(BaseHandler, self).get_template_namespace() ns.update(helpers) return ns + + def get_current_user(self): + if self.get_secure_cookie('atst'): + return True + else: + False + + def _start_session(self): + self.set_secure_cookie('atst', 'valid-user-session') diff --git a/atst/handlers/login.py b/atst/handlers/login.py new file mode 100644 index 00000000..b3012288 --- /dev/null +++ b/atst/handlers/login.py @@ -0,0 +1,10 @@ +import tornado +from atst.handler import BaseHandler + +class Login(BaseHandler): + + def initialize(self, page): + self.page = page + + def get(self): + self.render( '%s.html.to' % self.page, page = self.page ) diff --git a/atst/handlers/main.py b/atst/handlers/main.py index 62c6fa7e..05772b19 100644 --- a/atst/handlers/main.py +++ b/atst/handlers/main.py @@ -1,9 +1,11 @@ -from atst.handler import BaseHandler +import atst +from atst.handler import BaseHandler, authenticated class MainHandler(BaseHandler): def initialize(self, page): self.page = page + @authenticated def get(self): self.render( '%s.html.to' % self.page, page = self.page ) diff --git a/atst/handlers/request.py b/atst/handlers/request.py index 3ec27477..1041cbd8 100644 --- a/atst/handlers/request.py +++ b/atst/handlers/request.py @@ -1,4 +1,4 @@ -from atst.handler import BaseHandler +from atst.handler import BaseHandler, authenticated mock_requests = [ { @@ -31,5 +31,6 @@ class Request(BaseHandler): def initialize(self, page): self.page = page + @authenticated def get(self): self.render('requests.html.to', page = self.page, requests = mock_requests ) diff --git a/atst/handlers/request_new.py b/atst/handlers/request_new.py index aa405e3a..ad20554d 100644 --- a/atst/handlers/request_new.py +++ b/atst/handlers/request_new.py @@ -1,4 +1,4 @@ -from atst.handler import BaseHandler +from atst.handler import BaseHandler, authenticated class RequestNew(BaseHandler): screens = [ @@ -22,6 +22,7 @@ class RequestNew(BaseHandler): def initialize(self, page): self.page = page + @authenticated def get(self, screen = 1): self.render( 'requests/screen-%d.html.to' % int(screen), page = self.page, diff --git a/atst/handlers/workspace.py b/atst/handlers/workspace.py index bbed8f6d..05600208 100644 --- a/atst/handlers/workspace.py +++ b/atst/handlers/workspace.py @@ -1,5 +1,4 @@ -from atst.handler import BaseHandler -import requests +from atst.handler import BaseHandler, authenticated import tornado.gen mock_workspaces = [ @@ -13,8 +12,6 @@ mock_workspaces = [ } ] -session = requests.Session() - class Workspace(BaseHandler): def initialize(self, page, authz_client): @@ -22,5 +19,6 @@ class Workspace(BaseHandler): self.authz_client = authz_client @tornado.gen.coroutine + @authenticated def get(self): self.render( 'workspaces.html.to', page = self.page, workspaces = mock_workspaces ) diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 00000000..edcd356e --- /dev/null +++ b/tests/test_auth.py @@ -0,0 +1,34 @@ +import pytest +import tornado.web + + +@pytest.mark.gen_test +def test_redirects_when_not_logged_in(http_client, base_url): + response = yield http_client.fetch( + base_url + "/home", raise_error=False, follow_redirects=False + ) + assert response.code == 302 + assert response.error + assert response.headers["Location"] == "/login" + + +@pytest.mark.gen_test +def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): + monkeypatch.setattr("atst.handler.validate_login_token", lambda t: True) + response = yield http_client.fetch( + base_url + "/home", headers={"Cookie": "bearer-token=anything"} + ) + assert response.headers['Set-Cookie'].startswith('atst') + assert response.code == 200 + assert not response.error + + +@pytest.mark.gen_test +@pytest.mark.skip(reason="need to work out auth error user paths") +def test_login_with_invalid_bearer_token(monkeypatch, http_client, base_url): + monkeypatch.setattr("atst.handler.validate_login_token", lambda t: False) + response = yield http_client.fetch( + base_url + "/home", + raise_error=False, + headers={"Cookie": "bearer-token=anything"}, + ) From d573c5459bcfc241e0611939f0cb4d6151d3f5b1 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 11 Jun 2018 16:24:08 -0400 Subject: [PATCH 2/9] provide dev access to app --- atst/app.py | 40 +++++++++++++++++++++++++--------------- atst/handler.py | 1 + atst/handlers/dev.py | 13 +++++++++++++ tests/test_auth.py | 9 ++++++++- 4 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 atst/handlers/dev.py diff --git a/atst/app.py b/atst/app.py index 07f35cc2..25721153 100644 --- a/atst/app.py +++ b/atst/app.py @@ -8,29 +8,39 @@ from atst.handlers.login import Login from atst.handlers.workspace import Workspace from atst.handlers.request import Request from atst.handlers.request_new import RequestNew +from atst.handlers.dev import Dev from atst.home import home from atst.api_client import ApiClient +routes = [ + url(r"/", Login, {"page": "login"}, name="main"), + url(r"/login", Login, {"page": "login"}, name="login"), + url(r"/home", MainHandler, {"page": "home"}, name="home"), + url(r"/workspaces", Workspace, {"page": "workspaces"}, name="workspaces"), + url(r"/requests", Request, {"page": "requests"}, name="requests"), + url(r"/requests/new", RequestNew, {"page": "requests_new"}, name="request_new"), + url( + r"/requests/new/([0-9])", + RequestNew, + {"page": "requests_new"}, + name="request_form", + ), + url(r"/users", MainHandler, {"page": "users"}, name="users"), + url(r"/reports", MainHandler, {"page": "reports"}, name="reports"), + url(r"/calculator", MainHandler, {"page": "calculator"}, name="calculator"), +] + +env = os.getenv("TORNADO_ENV", "development") +if not env == "production": + routes += [url(r"/login-dev", Dev, {"action": "login"}, name="dev-login")] + def make_app(config): authz_client = ApiClient(config['default']['AUTHZ_BASE_URL']) - app = tornado.web.Application([ - url( r"/", Login, {'page': 'login'}, name='main' ), - url( r"/login", Login, {'page': 'login'}, name='login' ), - url( r"/home", MainHandler, {'page': 'home'}, name='home' ), - url( r"/workspaces", - Workspace, - {'page': 'workspaces', 'authz_client': authz_client}, - name='workspaces'), - url( r"/requests", Request, {'page': 'requests'}, name='requests' ), - url( r"/requests/new", RequestNew, {'page': 'requests_new'}, name='request_new' ), - url( r"/requests/new/([0-9])", RequestNew, {'page': 'requests_new'}, name='request_form' ), - url( r"/users", MainHandler, {'page': 'users'}, name='users' ), - url( r"/reports", MainHandler, {'page': 'reports'}, name='reports' ), - url( r"/calculator", MainHandler, {'page': 'calculator'}, name='calculator' ), - ], + app = tornado.web.Application( + routes, template_path = home.child('templates'), static_path = home.child('static'), debug=config['default'].getboolean('DEBUG') diff --git a/atst/handler.py b/atst/handler.py index 8af83bf3..446c8df9 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -56,5 +56,6 @@ class BaseHandler(tornado.web.RequestHandler): else: False + # this is a temporary implementation until we have real sessions def _start_session(self): self.set_secure_cookie('atst', 'valid-user-session') diff --git a/atst/handlers/dev.py b/atst/handlers/dev.py new file mode 100644 index 00000000..e6aee9f3 --- /dev/null +++ b/atst/handlers/dev.py @@ -0,0 +1,13 @@ +from atst.handler import BaseHandler, authenticated + +class Dev(BaseHandler): + def initialize(self, action): + self.action = action + + def get(self): + if self.action == 'login': + self._login() + + def _login(self): + self._start_session() + self.redirect("/home") diff --git a/tests/test_auth.py b/tests/test_auth.py index edcd356e..756e9c00 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -11,7 +11,6 @@ def test_redirects_when_not_logged_in(http_client, base_url): assert response.error assert response.headers["Location"] == "/login" - @pytest.mark.gen_test def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): monkeypatch.setattr("atst.handler.validate_login_token", lambda t: True) @@ -22,6 +21,14 @@ def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): assert response.code == 200 assert not response.error +@pytest.mark.gen_test +def test_login_with_via_dev_endpoint(app, monkeypatch, http_client, base_url): + response = yield http_client.fetch( + base_url + "/login-dev", raise_error=False, follow_redirects=False + ) + assert response.headers['Set-Cookie'].startswith('atst') + assert response.code == 302 + assert response.headers["Location"] == "/home" @pytest.mark.gen_test @pytest.mark.skip(reason="need to work out auth error user paths") From 7e689dd120223f26f8e684d53981c2382e9ded8c Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 12 Jun 2018 09:17:45 -0400 Subject: [PATCH 3/9] cleanup from the rebase; use config for cookies, login values --- Pipfile | 1 - Pipfile.lock | 38 +----------------------------- atst/app.py | 62 ++++++++++++++++++++++++++----------------------- config/base.ini | 1 + 4 files changed, 35 insertions(+), 67 deletions(-) diff --git a/Pipfile b/Pipfile index 1cedb3f5..8d79d3aa 100644 --- a/Pipfile +++ b/Pipfile @@ -7,7 +7,6 @@ name = "pypi" tornado = "==5.0.2" webassets = "==0.12.1" Unipath = "==1.1" -requests = "*" [dev-packages] pytest = "==3.6.0" diff --git a/Pipfile.lock b/Pipfile.lock index d4ccb88b..4edfecc2 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "68a0d5093979093899f0f86faa82eb55f90f9a67a16b11a5701ea85096e72ee8" + "sha256": "391e254ddb902877afca9c07aa2306710ce6d1e207b029c1a8b5dc0115ee99a5" }, "pipfile-spec": 6, "requires": { @@ -16,35 +16,6 @@ ] }, "default": { - "certifi": { - "hashes": [ - "sha256:13e698f54293db9f89122b0581843a782ad0934a4fe0172d2a980ba77fc61bb7", - "sha256:9fa520c1bacfb634fa7af20a76bcbd3d5fb390481724c597da32c719a7dca4b0" - ], - "version": "==2018.4.16" - }, - "chardet": { - "hashes": [ - "sha256:84ab92ed1c4d4f16916e05906b6b75a6c0fb5db821cc65e70cbd64a3e2a5eaae", - "sha256:fc323ffcaeaed0e0a02bf4d117757b98aed530d9ed4531e3e15460124c106691" - ], - "version": "==3.0.4" - }, - "idna": { - "hashes": [ - "sha256:2c6a5de3089009e3da7c5dde64a141dbc8551d5b7f6cf4ed7c2568d0cc520a8f", - "sha256:8c7309c718f94b3a625cb648ace320157ad16ff131ae0af362c9f21b80ef6ec4" - ], - "version": "==2.6" - }, - "requests": { - "hashes": [ - "sha256:6a1b267aa90cac58ac3a765d067950e7dbbf75b1da07e895d1f594193a40a38b", - "sha256:9c443e7324ba5b85070c4a818ade28bfabedf16ea10206da1132edaa6dda237e" - ], - "index": "pypi", - "version": "==2.18.4" - }, "tornado": { "hashes": [ "sha256:1b83d5c10550f2653380b4c77331d6f8850f287c4f67d7ce1e1c639d9222fbc7", @@ -64,13 +35,6 @@ "index": "pypi", "version": "==1.1" }, - "urllib3": { - "hashes": [ - "sha256:06330f386d6e4b195fbfc736b297f58c5a892e4440e54d294d7004e3a9bbea1b", - "sha256:cc44da8e1145637334317feebd728bd869a35285b93cbb4cca2577da7e62db4f" - ], - "version": "==1.22" - }, "webassets": { "hashes": [ "sha256:e7d9c8887343123fd5b32309b33167428cb1318cdda97ece12d0907fd69d38db" diff --git a/atst/app.py b/atst/app.py index 25721153..4c09faf0 100644 --- a/atst/app.py +++ b/atst/app.py @@ -12,51 +12,55 @@ from atst.handlers.dev import Dev from atst.home import home from atst.api_client import ApiClient - -routes = [ - url(r"/", Login, {"page": "login"}, name="main"), - url(r"/login", Login, {"page": "login"}, name="login"), - url(r"/home", MainHandler, {"page": "home"}, name="home"), - url(r"/workspaces", Workspace, {"page": "workspaces"}, name="workspaces"), - url(r"/requests", Request, {"page": "requests"}, name="requests"), - url(r"/requests/new", RequestNew, {"page": "requests_new"}, name="request_new"), - url( - r"/requests/new/([0-9])", - RequestNew, - {"page": "requests_new"}, - name="request_form", - ), - url(r"/users", MainHandler, {"page": "users"}, name="users"), - url(r"/reports", MainHandler, {"page": "reports"}, name="reports"), - url(r"/calculator", MainHandler, {"page": "calculator"}, name="calculator"), -] - -env = os.getenv("TORNADO_ENV", "development") -if not env == "production": - routes += [url(r"/login-dev", Dev, {"action": "login"}, name="dev-login")] +ENV = os.getenv("TORNADO_ENV", "dev") def make_app(config): - authz_client = ApiClient(config['default']['AUTHZ_BASE_URL']) + authz_client = ApiClient(config["default"]["AUTHZ_BASE_URL"]) + + routes = [ + url(r"/", Login, {"page": "login"}, name="main"), + url(r"/login", Login, {"page": "login"}, name="login"), + url(r"/home", MainHandler, {"page": "home"}, name="home"), + url( + r"/workspaces", + Workspace, + {"page": "workspaces", "authz_client": authz_client}, + name="workspaces", + ), + url(r"/requests", Request, {"page": "requests"}, name="requests"), + url(r"/requests/new", RequestNew, {"page": "requests_new"}, name="request_new"), + url( + r"/requests/new/([0-9])", + RequestNew, + {"page": "requests_new"}, + name="request_form", + ), + url(r"/users", MainHandler, {"page": "users"}, name="users"), + url(r"/reports", MainHandler, {"page": "reports"}, name="reports"), + url(r"/calculator", MainHandler, {"page": "calculator"}, name="calculator"), + ] + + if not ENV == "production": + routes += [url(r"/login-dev", Dev, {"action": "login"}, name="dev-login")] app = tornado.web.Application( routes, + login_url="/login", template_path = home.child('templates'), static_path = home.child('static'), + cookie_secret=config["default"]["COOKIE_SECRET"], debug=config['default'].getboolean('DEBUG') ) return app def make_config(): - BASE_CONFIG_FILENAME = os.path.join( - os.path.dirname(__file__), - '../config/base.ini', - ) + BASE_CONFIG_FILENAME = os.path.join(os.path.dirname(__file__), "../config/base.ini") ENV_CONFIG_FILENAME = os.path.join( os.path.dirname(__file__), - '../config/', - '{}.ini'.format(os.getenv('TORNADO_ENV', 'dev').lower()) + "../config/", + "{}.ini".format(ENV.lower()), ) config = ConfigParser() diff --git a/config/base.ini b/config/base.ini index 98d53555..fb4a4daa 100644 --- a/config/base.ini +++ b/config/base.ini @@ -3,3 +3,4 @@ ENVIRONMENT = dev DEBUG = true AUTHZ_BASE_URL = http://localhost PORT = 8000 +COOKIE_SECRET = some-secret-please-replace From 234bbcea0fba7db0883fd178e9721e0fb6c64886 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 12 Jun 2018 13:09:04 -0400 Subject: [PATCH 4/9] validate bearer tokens against authnid --- atst/api_client.py | 2 +- atst/app.py | 2 ++ atst/handler.py | 17 +++++++++++---- config/base.ini | 3 ++- tests/test_auth.py | 53 +++++++++++++++++++++++++++++++++++++++------- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/atst/api_client.py b/atst/api_client.py index 3a68f707..c7e35fe7 100644 --- a/atst/api_client.py +++ b/atst/api_client.py @@ -33,7 +33,7 @@ class ApiClient(object): kwargs['body'] = dumps(kwargs['json']) del kwargs['json'] headers = kwargs.get('headers', {}) - headers['Content-Type'] = 'application-json' + headers['Content-Type'] = 'application/json' kwargs['headers'] = headers response = yield self.client.fetch(url, method=method, **kwargs) diff --git a/atst/app.py b/atst/app.py index 4c09faf0..92d188db 100644 --- a/atst/app.py +++ b/atst/app.py @@ -17,6 +17,7 @@ ENV = os.getenv("TORNADO_ENV", "dev") def make_app(config): authz_client = ApiClient(config["default"]["AUTHZ_BASE_URL"]) + authnid_client = ApiClient(config["default"]["AUTHNID_BASE_URL"]) routes = [ url(r"/", Login, {"page": "login"}, name="main"), @@ -52,6 +53,7 @@ def make_app(config): cookie_secret=config["default"]["COOKIE_SECRET"], debug=config['default'].getboolean('DEBUG') ) + app.authnid_client = authnid_client return app diff --git a/atst/handler.py b/atst/handler.py index 446c8df9..dbd95430 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -22,11 +22,13 @@ helpers = { def authenticated(method): @functools.wraps(method) + @tornado.gen.coroutine def wrapper(self, *args, **kwargs): if not self.current_user: if self.get_cookie('bearer-token'): bearer_token = self.get_cookie('bearer-token') - if validate_login_token(bearer_token): + valid = yield validate_login_token(self.application.authnid_client, bearer_token) + if valid: self._start_session() else: raise NotImplementedError @@ -39,9 +41,16 @@ def authenticated(method): return method(self, *args, **kwargs) return wrapper -def validate_login_token(token): - # check against authnid - pass +@tornado.gen.coroutine +def validate_login_token(client, token): + try: + response = yield client.post('/api/v1/validate', raise_error=False, json={"token": token}) + return response.code == 200 + except tornado.httpclient.HTTPError as error: + if error.response.code == 401: + return False + else: + raise error class BaseHandler(tornado.web.RequestHandler): diff --git a/config/base.ini b/config/base.ini index fb4a4daa..9747c782 100644 --- a/config/base.ini +++ b/config/base.ini @@ -1,6 +1,7 @@ [default] +PORT=8000 ENVIRONMENT = dev DEBUG = true AUTHZ_BASE_URL = http://localhost -PORT = 8000 +AUTHNID_BASE_URL= http://localhost COOKIE_SECRET = some-secret-please-replace diff --git a/tests/test_auth.py b/tests/test_auth.py index 756e9c00..fa636b67 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,35 @@ import pytest import tornado.web +from concurrent.futures import ThreadPoolExecutor +from atst.handler import validate_login_token + + +class MockApiResponse(): + + def __init__(self, code, json): + self.code = code + self.json = json + + +@pytest.mark.gen_test +def test_successful_validate_login_token(monkeypatch, app): + monkeypatch.setattr( + "atst.api_client.ApiClient.get", + lambda x, + y, + json=None: MockApiResponse(200, {"status": "success"}), + ) + assert validate_login_token(app.authnid_client, "abc-123") + + +@pytest.mark.gen_test +def test_unsuccessful_validate_login_token(monkeypatch, app): + monkeypatch.setattr( + "atst.api_client.ApiClient.get", + lambda x,y,json=None: MockApiResponse(401, {"status": "error"}), + ) + valid = yield validate_login_token(app.authnid_client, "abc-123") + assert not valid @pytest.mark.gen_test @@ -11,25 +41,32 @@ def test_redirects_when_not_logged_in(http_client, base_url): assert response.error assert response.headers["Location"] == "/login" + @pytest.mark.gen_test def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): - monkeypatch.setattr("atst.handler.validate_login_token", lambda t: True) - response = yield http_client.fetch( - base_url + "/home", headers={"Cookie": "bearer-token=anything"} - ) - assert response.headers['Set-Cookie'].startswith('atst') - assert response.code == 200 - assert not response.error + with ThreadPoolExecutor(max_workers=1) as executor: + monkeypatch.setattr( + "atst.handler.validate_login_token", + lambda c,t: executor.submit(lambda: True), + ) + response = yield http_client.fetch( + base_url + "/home", headers={"Cookie": "bearer-token=anything"} + ) + assert response.headers["Set-Cookie"].startswith("atst") + assert response.code == 200 + assert not response.error + @pytest.mark.gen_test def test_login_with_via_dev_endpoint(app, monkeypatch, http_client, base_url): response = yield http_client.fetch( base_url + "/login-dev", raise_error=False, follow_redirects=False ) - assert response.headers['Set-Cookie'].startswith('atst') + assert response.headers["Set-Cookie"].startswith("atst") assert response.code == 302 assert response.headers["Location"] == "/home" + @pytest.mark.gen_test @pytest.mark.skip(reason="need to work out auth error user paths") def test_login_with_invalid_bearer_token(monkeypatch, http_client, base_url): From 4e61b08330f25892b76de792728da286c8c3d28a Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 12 Jun 2018 15:07:45 -0400 Subject: [PATCH 5/9] handle auth via redirect with parameter --- atst/app.py | 5 +++-- atst/handler.py | 21 +-------------------- atst/handlers/home.py | 10 ++++++++++ atst/handlers/login.py | 29 ++++++++++++++++++++++++++++- tests/test_auth.py | 34 +++++++--------------------------- 5 files changed, 49 insertions(+), 50 deletions(-) create mode 100644 atst/handlers/home.py diff --git a/atst/app.py b/atst/app.py index 92d188db..0d74ce47 100644 --- a/atst/app.py +++ b/atst/app.py @@ -4,6 +4,7 @@ import tornado.web from tornado.web import url from atst.handlers.main import MainHandler +from atst.handlers.home import Home from atst.handlers.login import Login from atst.handlers.workspace import Workspace from atst.handlers.request import Request @@ -20,7 +21,7 @@ def make_app(config): authnid_client = ApiClient(config["default"]["AUTHNID_BASE_URL"]) routes = [ - url(r"/", Login, {"page": "login"}, name="main"), + url(r"/", Home, {"page": "login"}, name="main"), url(r"/login", Login, {"page": "login"}, name="login"), url(r"/home", MainHandler, {"page": "home"}, name="home"), url( @@ -47,7 +48,7 @@ def make_app(config): app = tornado.web.Application( routes, - login_url="/login", + login_url="/", template_path = home.child('templates'), static_path = home.child('static'), cookie_secret=config["default"]["COOKIE_SECRET"], diff --git a/atst/handler.py b/atst/handler.py index dbd95430..b38be79b 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -22,17 +22,9 @@ helpers = { def authenticated(method): @functools.wraps(method) - @tornado.gen.coroutine def wrapper(self, *args, **kwargs): if not self.current_user: - if self.get_cookie('bearer-token'): - bearer_token = self.get_cookie('bearer-token') - valid = yield validate_login_token(self.application.authnid_client, bearer_token) - if valid: - self._start_session() - else: - raise NotImplementedError - elif self.request.method in ("GET", "HEAD"): + if self.request.method in ("GET", "HEAD"): url = self.get_login_url() self.redirect(url) return @@ -41,17 +33,6 @@ def authenticated(method): return method(self, *args, **kwargs) return wrapper -@tornado.gen.coroutine -def validate_login_token(client, token): - try: - response = yield client.post('/api/v1/validate', raise_error=False, json={"token": token}) - return response.code == 200 - except tornado.httpclient.HTTPError as error: - if error.response.code == 401: - return False - else: - raise error - class BaseHandler(tornado.web.RequestHandler): def get_template_namespace(self): diff --git a/atst/handlers/home.py b/atst/handlers/home.py new file mode 100644 index 00000000..d600dfaa --- /dev/null +++ b/atst/handlers/home.py @@ -0,0 +1,10 @@ +import tornado +from atst.handler import BaseHandler + +class Home(BaseHandler): + + def initialize(self, page): + self.page = page + + def get(self): + self.render( '%s.html.to' % self.page, page = self.page ) diff --git a/atst/handlers/login.py b/atst/handlers/login.py index b3012288..80c96fdf 100644 --- a/atst/handlers/login.py +++ b/atst/handlers/login.py @@ -1,10 +1,37 @@ import tornado from atst.handler import BaseHandler + class Login(BaseHandler): def initialize(self, page): self.page = page + @tornado.gen.coroutine def get(self): - self.render( '%s.html.to' % self.page, page = self.page ) + token = self.get_query_argument("bearer-token") + if token: + valid = yield self._validate_login_token(token) + if valid: + self._start_session() + self.redirect("/home") + return + + url = self.get_login_url() + self.redirect(url) + return + + @tornado.gen.coroutine + def _validate_login_token(self, token): + try: + response = yield self.application.authnid_client.post( + "/api/v1/validate", json={"token": token} + ) + return response.code == 200 + + except tornado.httpclient.HTTPError as error: + if error.response.code == 401: + return False + + else: + raise error diff --git a/tests/test_auth.py b/tests/test_auth.py index fa636b67..a9c2e5de 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,7 +1,6 @@ import pytest import tornado.web from concurrent.futures import ThreadPoolExecutor -from atst.handler import validate_login_token class MockApiResponse(): @@ -11,27 +10,6 @@ class MockApiResponse(): self.json = json -@pytest.mark.gen_test -def test_successful_validate_login_token(monkeypatch, app): - monkeypatch.setattr( - "atst.api_client.ApiClient.get", - lambda x, - y, - json=None: MockApiResponse(200, {"status": "success"}), - ) - assert validate_login_token(app.authnid_client, "abc-123") - - -@pytest.mark.gen_test -def test_unsuccessful_validate_login_token(monkeypatch, app): - monkeypatch.setattr( - "atst.api_client.ApiClient.get", - lambda x,y,json=None: MockApiResponse(401, {"status": "error"}), - ) - valid = yield validate_login_token(app.authnid_client, "abc-123") - assert not valid - - @pytest.mark.gen_test def test_redirects_when_not_logged_in(http_client, base_url): response = yield http_client.fetch( @@ -39,22 +17,24 @@ def test_redirects_when_not_logged_in(http_client, base_url): ) assert response.code == 302 assert response.error - assert response.headers["Location"] == "/login" + assert response.headers["Location"] == "/" @pytest.mark.gen_test def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): with ThreadPoolExecutor(max_workers=1) as executor: monkeypatch.setattr( - "atst.handler.validate_login_token", + "atst.handlers.login.Login._validate_login_token", lambda c,t: executor.submit(lambda: True), ) response = yield http_client.fetch( - base_url + "/home", headers={"Cookie": "bearer-token=anything"} + base_url + "/login?bearer-token=abc-123", + follow_redirects=False, + raise_error=False ) assert response.headers["Set-Cookie"].startswith("atst") - assert response.code == 200 - assert not response.error + assert response.headers['Location'] == '/home' + assert response.code == 302 @pytest.mark.gen_test From 34f3c7776be354584aa12eefebe7f8e3acb90cd0 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 12 Jun 2018 15:13:22 -0400 Subject: [PATCH 6/9] fall back to regular tornado authentication decorator --- atst/handler.py | 13 ------------- atst/handlers/dev.py | 2 +- atst/handlers/main.py | 5 +++-- atst/handlers/request.py | 5 +++-- atst/handlers/request_new.py | 5 +++-- atst/handlers/workspace.py | 5 +++-- tests/test_auth.py | 4 +++- 7 files changed, 16 insertions(+), 23 deletions(-) diff --git a/atst/handler.py b/atst/handler.py index b38be79b..b79cfbc3 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -20,19 +20,6 @@ helpers = { 'assets': assets } -def authenticated(method): - @functools.wraps(method) - def wrapper(self, *args, **kwargs): - if not self.current_user: - if self.request.method in ("GET", "HEAD"): - url = self.get_login_url() - self.redirect(url) - return - else: - raise tornado.web.HTTPError(403) - return method(self, *args, **kwargs) - return wrapper - class BaseHandler(tornado.web.RequestHandler): def get_template_namespace(self): diff --git a/atst/handlers/dev.py b/atst/handlers/dev.py index e6aee9f3..a22fd456 100644 --- a/atst/handlers/dev.py +++ b/atst/handlers/dev.py @@ -1,4 +1,4 @@ -from atst.handler import BaseHandler, authenticated +from atst.handler import BaseHandler class Dev(BaseHandler): def initialize(self, action): diff --git a/atst/handlers/main.py b/atst/handlers/main.py index 05772b19..71ff1395 100644 --- a/atst/handlers/main.py +++ b/atst/handlers/main.py @@ -1,11 +1,12 @@ import atst -from atst.handler import BaseHandler, authenticated +import tornado +from atst.handler import BaseHandler class MainHandler(BaseHandler): def initialize(self, page): self.page = page - @authenticated + @tornado.web.authenticated def get(self): self.render( '%s.html.to' % self.page, page = self.page ) diff --git a/atst/handlers/request.py b/atst/handlers/request.py index 1041cbd8..9ce3da34 100644 --- a/atst/handlers/request.py +++ b/atst/handlers/request.py @@ -1,4 +1,5 @@ -from atst.handler import BaseHandler, authenticated +import tornado +from atst.handler import BaseHandler mock_requests = [ { @@ -31,6 +32,6 @@ class Request(BaseHandler): def initialize(self, page): self.page = page - @authenticated + @tornado.web.authenticated def get(self): self.render('requests.html.to', page = self.page, requests = mock_requests ) diff --git a/atst/handlers/request_new.py b/atst/handlers/request_new.py index ad20554d..55a39bc9 100644 --- a/atst/handlers/request_new.py +++ b/atst/handlers/request_new.py @@ -1,4 +1,5 @@ -from atst.handler import BaseHandler, authenticated +import tornado +from atst.handler import BaseHandler class RequestNew(BaseHandler): screens = [ @@ -22,7 +23,7 @@ class RequestNew(BaseHandler): def initialize(self, page): self.page = page - @authenticated + @tornado.web.authenticated def get(self, screen = 1): self.render( 'requests/screen-%d.html.to' % int(screen), page = self.page, diff --git a/atst/handlers/workspace.py b/atst/handlers/workspace.py index 05600208..5e05ad1d 100644 --- a/atst/handlers/workspace.py +++ b/atst/handlers/workspace.py @@ -1,4 +1,5 @@ -from atst.handler import BaseHandler, authenticated +from atst.handler import BaseHandler +import tornado import tornado.gen mock_workspaces = [ @@ -19,6 +20,6 @@ class Workspace(BaseHandler): self.authz_client = authz_client @tornado.gen.coroutine - @authenticated + @tornado.web.authenticated def get(self): self.render( 'workspaces.html.to', page = self.page, workspaces = mock_workspaces ) diff --git a/tests/test_auth.py b/tests/test_auth.py index a9c2e5de..50d0acf7 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,3 +1,4 @@ +import re import pytest import tornado.web from concurrent.futures import ThreadPoolExecutor @@ -15,9 +16,10 @@ def test_redirects_when_not_logged_in(http_client, base_url): response = yield http_client.fetch( base_url + "/home", raise_error=False, follow_redirects=False ) + location = response.headers['Location'] assert response.code == 302 assert response.error - assert response.headers["Location"] == "/" + assert re.match('/\??', location) @pytest.mark.gen_test From 261a00adb284804138f113c1922c7e0f23f25bf3 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 12 Jun 2018 15:57:27 -0400 Subject: [PATCH 7/9] pass authnid_client directly to Login handler and other small cleanup --- atst/app.py | 16 ++++++++++++---- atst/handler.py | 1 - atst/handlers/login.py | 6 +++--- tests/test_auth.py | 7 ------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/atst/app.py b/atst/app.py index 0d74ce47..9d8ddc00 100644 --- a/atst/app.py +++ b/atst/app.py @@ -15,6 +15,7 @@ from atst.api_client import ApiClient ENV = os.getenv("TORNADO_ENV", "dev") + def make_app(config): authz_client = ApiClient(config["default"]["AUTHZ_BASE_URL"]) @@ -22,7 +23,12 @@ def make_app(config): routes = [ url(r"/", Home, {"page": "login"}, name="main"), - url(r"/login", Login, {"page": "login"}, name="login"), + url( + r"/login", + Login, + {"authnid_client": authnid_client}, + name="login", + ), url(r"/home", MainHandler, {"page": "home"}, name="home"), url( r"/workspaces", @@ -54,16 +60,18 @@ def make_app(config): cookie_secret=config["default"]["COOKIE_SECRET"], debug=config['default'].getboolean('DEBUG') ) - app.authnid_client = authnid_client return app def make_config(): - BASE_CONFIG_FILENAME = os.path.join(os.path.dirname(__file__), "../config/base.ini") + BASE_CONFIG_FILENAME = os.path.join( + os.path.dirname(__file__), + "../config/base.ini" + ) ENV_CONFIG_FILENAME = os.path.join( os.path.dirname(__file__), "../config/", - "{}.ini".format(ENV.lower()), + "{}.ini".format(ENV.lower()) ) config = ConfigParser() diff --git a/atst/handler.py b/atst/handler.py index b79cfbc3..26595b3d 100644 --- a/atst/handler.py +++ b/atst/handler.py @@ -1,5 +1,4 @@ import os -import functools from webassets import Environment, Bundle import tornado.web from atst.home import home diff --git a/atst/handlers/login.py b/atst/handlers/login.py index 80c96fdf..3a141340 100644 --- a/atst/handlers/login.py +++ b/atst/handlers/login.py @@ -4,8 +4,8 @@ from atst.handler import BaseHandler class Login(BaseHandler): - def initialize(self, page): - self.page = page + def initialize(self, authnid_client): + self.authnid_client = authnid_client @tornado.gen.coroutine def get(self): @@ -24,7 +24,7 @@ class Login(BaseHandler): @tornado.gen.coroutine def _validate_login_token(self, token): try: - response = yield self.application.authnid_client.post( + response = yield self.authnid_client.post( "/api/v1/validate", json={"token": token} ) return response.code == 200 diff --git a/tests/test_auth.py b/tests/test_auth.py index 50d0acf7..cc16fe5a 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -4,13 +4,6 @@ import tornado.web from concurrent.futures import ThreadPoolExecutor -class MockApiResponse(): - - def __init__(self, code, json): - self.code = code - self.json = json - - @pytest.mark.gen_test def test_redirects_when_not_logged_in(http_client, base_url): response = yield http_client.fetch( From e89be59d3e7cc15650a9c70db1eb36225b6a15ca Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Jun 2018 16:56:50 -0400 Subject: [PATCH 8/9] remove unused monkeypatch --- tests/test_auth.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_auth.py b/tests/test_auth.py index cc16fe5a..652f3916 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -33,7 +33,7 @@ def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): @pytest.mark.gen_test -def test_login_with_via_dev_endpoint(app, monkeypatch, http_client, base_url): +def test_login_via_dev_endpoint(app, http_client, base_url): response = yield http_client.fetch( base_url + "/login-dev", raise_error=False, follow_redirects=False ) @@ -44,8 +44,7 @@ def test_login_with_via_dev_endpoint(app, monkeypatch, http_client, base_url): @pytest.mark.gen_test @pytest.mark.skip(reason="need to work out auth error user paths") -def test_login_with_invalid_bearer_token(monkeypatch, http_client, base_url): - monkeypatch.setattr("atst.handler.validate_login_token", lambda t: False) +def test_login_with_invalid_bearer_token(http_client, base_url): response = yield http_client.fetch( base_url + "/home", raise_error=False, From 606bd61d3ac8d11a3d927c8aa2f9f6608da988ad Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 14 Jun 2018 13:10:19 -0400 Subject: [PATCH 9/9] Simplify test by removing ThreadPool --- tests/test_auth.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/tests/test_auth.py b/tests/test_auth.py index 652f3916..242083af 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,7 +1,7 @@ import re import pytest import tornado.web -from concurrent.futures import ThreadPoolExecutor +import tornado.gen @pytest.mark.gen_test @@ -17,19 +17,22 @@ def test_redirects_when_not_logged_in(http_client, base_url): @pytest.mark.gen_test def test_login_with_valid_bearer_token(app, monkeypatch, http_client, base_url): - with ThreadPoolExecutor(max_workers=1) as executor: - monkeypatch.setattr( - "atst.handlers.login.Login._validate_login_token", - lambda c,t: executor.submit(lambda: True), - ) - response = yield http_client.fetch( - base_url + "/login?bearer-token=abc-123", - follow_redirects=False, - raise_error=False - ) - assert response.headers["Set-Cookie"].startswith("atst") - assert response.headers['Location'] == '/home' - assert response.code == 302 + @tornado.gen.coroutine + def _validate_login_token(c, t): + return True + + monkeypatch.setattr( + "atst.handlers.login.Login._validate_login_token", + _validate_login_token + ) + response = yield http_client.fetch( + base_url + "/login?bearer-token=abc-123", + follow_redirects=False, + raise_error=False + ) + assert response.headers["Set-Cookie"].startswith("atst") + assert response.headers['Location'] == '/home' + assert response.code == 302 @pytest.mark.gen_test