From 2c49e59da75415ce48f04fa4a790c5b24f70239d Mon Sep 17 00:00:00 2001 From: Eric Passmore Date: Thu, 18 Apr 2024 18:16:29 -0700 Subject: [PATCH] access control for all APIs --- orchestration-service/github_oauth.py | 49 ++++++-- orchestration-service/test/run-pytest.sh | 28 ++++- .../test/test_no_auth_api.py | 41 +++++++ .../test/test_web_service.py | 106 +++++++++++------- orchestration-service/web_service.py | 31 +++-- 5 files changed, 193 insertions(+), 62 deletions(-) create mode 100644 orchestration-service/test/test_no_auth_api.py diff --git a/orchestration-service/github_oauth.py b/orchestration-service/github_oauth.py index 77e3304..58aec47 100644 --- a/orchestration-service/github_oauth.py +++ b/orchestration-service/github_oauth.py @@ -16,7 +16,7 @@ def assemble_oauth_url(state, properties): + f"&allow_signup=false" @staticmethod - def get_access_token(code, properties): + def get_oauth_access_token(code, properties): """build url for the get token request""" # construct http call to exchange tempory code for access token params = { @@ -41,10 +41,10 @@ def get_access_token(code, properties): return None @staticmethod - def get_public_profile(bearer_token, properties): + def create_auth_string(bearer_token, user_info_url): """get public profile information using token""" # https request to get public profile data, login and avatar_url - user_avatar_response = requests.get(properties.get('user_info_url'), + user_avatar_response = requests.get(user_info_url, timeout=3, headers={ 'Accept': 'application/vnd.github+json', @@ -53,11 +53,11 @@ def get_public_profile(bearer_token, properties): }) if user_avatar_response.status_code == 200: user_data = json.loads(user_avatar_response.content.decode('utf-8')) - return GitHubOauth.profile_to_str(user_data['login'],user_data['avatar_url']) + return GitHubOauth.credentials_to_str(user_data['login'],user_data['avatar_url'],bearer_token) return None @staticmethod - def is_authorized(bearer_token, login, team_string): + def check_membership(bearer_token, login, team_string): """Check for team membership""" if not login: return False @@ -79,13 +79,44 @@ def is_authorized(bearer_token, login, team_string): return False @staticmethod - def profile_to_str(login, avatar_url): + def is_authorized(cookies, header_token, user_info_url, team_string): + """check for authorized token or cookie""" + + token = None + if 'replay_auth' in cookies and cookies['replay_auth']: + token = GitHubOauth.extract_token(cookies['replay_auth']) + if header_token: + token = header_token + if not token: + return False + + auth_string = GitHubOauth.create_auth_string(token, user_info_url) + login = GitHubOauth.extract_login(auth_string) + return GitHubOauth.check_membership(token, login, team_string) + + @staticmethod + def credentials_to_str(login, avatar_url, token): """converts profile data to string sep by :""" - return login + ":" + avatar_url + return token + ":" + login + ":" + avatar_url @staticmethod - def str_to_profile(data): + def str_to_public_profile(data): """converts str to array of profile data""" if not data: return [] - return data.split(':', 1) + # return public profile data leaving off bearer token + return data.split(':', 2)[1:] + + @staticmethod + def extract_token(data): + """grabs the bearer token from string""" + if not data: + return [] + return data.split(':', 2)[0] + + @staticmethod + def extract_login(data): + """grabs the bearer token from string""" + if not data: + return [] + return data.split(':', 2)[1] diff --git a/orchestration-service/test/run-pytest.sh b/orchestration-service/test/run-pytest.sh index db9e98d..de659fb 100755 --- a/orchestration-service/test/run-pytest.sh +++ b/orchestration-service/test/run-pytest.sh @@ -23,11 +23,11 @@ fi rm ../../meta-data/test-modify-jobs.json cp ../../meta-data/test-simple-jobs.json ../../meta-data/test-modify-jobs.json -# integration tests start up service -{ python3 ../web_service.py --config "../../meta-data/test-modify-jobs.json" --host 127.0.0.1 --html-dir "../../webcontent/" > /dev/null 2>&1 & } +# make client calls to web service with access control diabled +{ python3 ../web_service.py --config "../../meta-data/test-modify-jobs.json" --host 127.0.0.1 --html-dir "../../webcontent/" --disable-auth > /dev/null 2>&1 & } WEB_SERVICE_PID=$! # prevent tests running before service is up -sleep 3 +sleep 1 # now test web service pytest test_web_service.py @@ -41,6 +41,28 @@ if [ "$DIFF_CNT" -lt 1 ]; then exit 1 fi +# shutdown and cleanup +kill "$WEB_SERVICE_PID" +rm ../../meta-data/test-modify-jobs.json + +cp ../../meta-data/test-simple-jobs.json ../../meta-data/test-modify-jobs.json +# test access control, all web calls should fail with 403 +{ python3 ../web_service.py --config "../../meta-data/test-modify-jobs.json" --host 127.0.0.1 --html-dir "../../webcontent/" > /dev/null 2>&1 & } +WEB_SERVICE_PID=$! +# prevent tests running before service is up +sleep 1 + +# now test web service +pytest test_no_auth_api.py + +sleep 1 + +DIFF_CNT=$(diff ../../meta-data/test-simple-jobs.json ../../meta-data/test-modify-jobs.json | grep "^>" | wc -l) +if [ "$DIFF_CNT" -ne 0 ]; then + echo "ERROR meta-data modified during auth check expected no changes" + exit 1 +fi + # shutdown service clean up file kill "$WEB_SERVICE_PID" rm ../../meta-data/test-modify-jobs.json diff --git a/orchestration-service/test/test_no_auth_api.py b/orchestration-service/test/test_no_auth_api.py new file mode 100644 index 0000000..b01923b --- /dev/null +++ b/orchestration-service/test/test_no_auth_api.py @@ -0,0 +1,41 @@ +"""Module tests web API expecting 403 auth failure""" +import requests +import pytest +import re +import json + +@pytest.fixture(scope="module") +def setup_module(): + """setting some constants to avoid mis-spellings""" + setup = {} + setup['base_url']='http://127.0.0.1:4000' + + setup['plain_text_headers'] = { + 'Accept': 'text/plain; charset=utf-8', + 'Content-Type': 'text/plain; charset=utf-8', + } + + setup['json_headers'] = { + 'Accept': 'application/json', + 'Content-Type': 'application/json; charset=utf-8', + } + + setup['html_headers'] = { + 'Accept': 'text/html, application/xhtml+xml', + 'Content-Type': 'text/html; charset=utf-8', + } + + return setup + +def test_api_calls(setup_module): + """Run through all API calls and make sure they fail with 403""" + cntx = setup_module + + params = { 'nextjob': 1, 'sliceid': 3 } + api_calls = ['/job', '/status', '/config', '/summary', '/progress', '/grid', '/control', '/detail', '/logout'] + for path in api_calls: + response = requests.get(cntx['base_url'] + '/job', + params=params, + timeout=3, + headers=cntx['json_headers']) + assert response.status_code == 403 diff --git a/orchestration-service/test/test_web_service.py b/orchestration-service/test/test_web_service.py index 7077043..81ffca4 100644 --- a/orchestration-service/test/test_web_service.py +++ b/orchestration-service/test/test_web_service.py @@ -24,21 +24,25 @@ def setup_module(): 'Accept': 'text/html, application/xhtml+xml', 'Content-Type': 'text/html; charset=utf-8', } - return setup + session = requests.Session() + session.cookies.set('replay_auth', 'ghb_12334dfjkhaf:foobar:https://example.com/myavatar.gif') + + return setup, session def test_job_redirect(setup_module): """Equivalent results request a job using the redirect, and request with 'nextjob' param""" - cntx = setup_module + cntx, session = setup_module # request a job test that /job follows redirect to /job?nextjob # both /job and /job?nextjob should result in the same response - follow_redir_response = requests.get(cntx['base_url'] + '/job', headers=cntx['plain_text_headers']) - assert len(follow_redir_response.headers['ETag']) > 30 + follow_redir_response = session.get(cntx['base_url'] + '/job', headers=cntx['plain_text_headers']) assert follow_redir_response.status_code == 200 + assert len(follow_redir_response.headers['ETag']) > 30 + params = { 'nextjob': 1 } - direct_response = requests.get(cntx['base_url'] + '/job', + direct_response = session.get(cntx['base_url'] + '/job', params=params, headers=cntx['plain_text_headers']) @@ -49,10 +53,11 @@ def test_job_redirect(setup_module): def test_get_nextjob(setup_module): """Validate the data returned differs by Content Type""" - cntx = setup_module + cntx, session = setup_module + # test plain text params = { 'nextjob': 1 } - response_plain = requests.get(cntx['base_url'] + '/job', params=params, headers=cntx['plain_text_headers']) + response_plain = session.get(cntx['base_url'] + '/job', params=params, headers=cntx['plain_text_headers']) assert response_plain.status_code == 200 print(response_plain.content.decode('utf-8')) @@ -62,7 +67,7 @@ def test_get_nextjob(setup_module): # test json params = { 'nextjob': 1 } - response_json = requests.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) + response_json = session.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) assert response_json.status_code == 200 # print (response_json.content.decode('utf-8')) @@ -72,10 +77,11 @@ def test_get_nextjob(setup_module): def test_update_job(setup_module): """Get a job update the status and validate the change is in place""" - cntx = setup_module + cntx, session = setup_module + # test plain text params = { 'nextjob': 1 } - response_first = requests.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) + response_first = session.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) assert response_first.status_code == 200 etag_value = response_first.headers['ETag'] @@ -91,7 +97,7 @@ def test_update_job(setup_module): # serialized dict to JSON when passing in # Add ETag Header cntx['json_headers']['ETag'] = etag_value - updated_first = requests.post(cntx['base_url'] + '/job', + updated_first = session.post(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers'], data=json.dumps(job_first_request)) @@ -101,7 +107,7 @@ def test_update_job(setup_module): # fetch again to validate update # validate status after change params = { 'jobid': job_first_request['job_id'] } - validate_job_request = requests.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) + validate_job_request = session.get(cntx['base_url'] + '/job', params=params, headers=cntx['json_headers']) validate_job = json.loads(validate_job_request.content.decode('utf-8')) assert validate_job['status'] == 'STARTED' @@ -110,7 +116,7 @@ def test_update_job(setup_module): jobparams = { 'jobid': validate_job['job_id'] } # add ETag Header cntx['json_headers']['ETag'] = etag_value - updated = requests.post(cntx['base_url'] + '/job', + updated = session.post(cntx['base_url'] + '/job', params=jobparams, headers=cntx['json_headers'], data=json.dumps(validate_job)) @@ -124,7 +130,7 @@ def test_update_job(setup_module): jobparams = { 'jobid': validate_job['job_id'] } # add previous older and bad ETag Header cntx['json_headers']['ETag'] = etag_value - updated = requests.post(cntx['base_url'] + '/job', + updated = session.post(cntx['base_url'] + '/job', params=jobparams, headers=cntx['json_headers'], data=json.dumps(validate_job)) @@ -135,7 +141,8 @@ def test_update_job(setup_module): def test_no_more_jobs(setup_module): """Using nextjob loop over the jobs updating status; eventually no more jobs and None is returned""" - cntx = setup_module + cntx, session = setup_module + params = { 'nextjob': 1 } _ok = True @@ -148,7 +155,7 @@ def test_no_more_jobs(setup_module): while _ok: nextjobparams = { 'nextjob': 1 } - response = requests.get(cntx['base_url'] + '/job', params=nextjobparams, headers=cntx['json_headers']) + response = session.get(cntx['base_url'] + '/job', params=nextjobparams, headers=cntx['json_headers']) # end on not found 404 if response.status_code == 404: _ok = False @@ -171,7 +178,7 @@ def test_no_more_jobs(setup_module): jobparams = { 'jobid': job['job_id'] } # add ETag to Request cntx['json_headers']['ETag'] = etag_value - updated = requests.post(cntx['base_url'] + '/job', + updated = session.post(cntx['base_url'] + '/job', params=jobparams, headers=cntx['json_headers'], data=json.dumps(job)) @@ -191,7 +198,7 @@ def test_no_more_jobs(setup_module): jobparams = { 'jobid': job['job_id'] } # Add ETag cntx['json_headers']['ETag'] = etag_db[job['job_id']] - updated = requests.post(cntx['base_url'] + '/job', + updated = session.post(cntx['base_url'] + '/job', params=jobparams, headers=cntx['json_headers'], data=json.dumps(job)) @@ -200,30 +207,30 @@ def test_no_more_jobs(setup_module): def test_status_reports(setup_module): """Request full status and check results""" - cntx = setup_module + cntx, session = setup_module - response = requests.get(cntx['base_url'] + '/status', headers=cntx['json_headers']) + response = session.get(cntx['base_url'] + '/status', headers=cntx['json_headers']) assert response.status_code == 200 assert len(response.content) > 500 - response = requests.get(cntx['base_url'] + '/status', headers=cntx['plain_text_headers']) + response = session.get(cntx['base_url'] + '/status', headers=cntx['plain_text_headers']) assert response.status_code == 200 assert len(response.content) > 500 - response = requests.get(cntx['base_url'] + '/status', headers=cntx['html_headers']) + response = session.get(cntx['base_url'] + '/status', headers=cntx['html_headers']) assert response.status_code == 200 assert len(response.content) > 500 # check position arg params = { 'sliceid': 1 } - response = requests.get(cntx['base_url'] + '/status', + response = session.get(cntx['base_url'] + '/status', params=params, headers=cntx['json_headers']) assert response.status_code == 200 assert len(response.content) > 50 - response = requests.get(cntx['base_url'] + '/status', + response = session.get(cntx['base_url'] + '/status', params=params, headers=cntx['plain_text_headers']) assert response.status_code == 200 assert len(response.content) > 50 - response = requests.get(cntx['base_url'] + '/status', + response = session.get(cntx['base_url'] + '/status', params=params, headers=cntx['html_headers']) assert response.status_code == 200 @@ -231,17 +238,17 @@ def test_status_reports(setup_module): def test_config_reports(setup_module): """Request full status and check results""" - cntx = setup_module + cntx, session = setup_module params = { 'sliceid': 1 } - response = requests.get(cntx['base_url'] + '/config', + response = session.get(cntx['base_url'] + '/config', params=params, headers=cntx['json_headers']) assert response.status_code == 200 assert len(response.content) > 200 params = { 'sliceid': 1 } - response = requests.get(cntx['base_url'] + '/config', + response = session.get(cntx['base_url'] + '/config', params=params, headers=cntx['html_headers']) assert response.status_code == 200 @@ -249,7 +256,7 @@ def test_config_reports(setup_module): def test_post_config(setup_module): """Update Config""" - cntx = setup_module + cntx, session = setup_module config = { "end_block_num": 324302525, @@ -257,7 +264,7 @@ def test_post_config(setup_module): } params = { 'sliceid': 3 } - responseBefore = requests.get(cntx['base_url'] + '/config', + responseBefore = session.get(cntx['base_url'] + '/config', params=params, headers=cntx['json_headers']) assert responseBefore.status_code == 200 @@ -265,14 +272,14 @@ def test_post_config(setup_module): assert configBefore['expected_integrity_hash'] != config['integrity_hash'] # make POST call; json passed in as string - update_config_response = requests.post(cntx['base_url'] + '/config', + update_config_response = session.post(cntx['base_url'] + '/config', headers=cntx['json_headers'], timeout=3, data=json.dumps(config)) assert update_config_response.status_code == 200 params = { 'sliceid': 3 } - responseAfter = requests.get(cntx['base_url'] + '/config', + responseAfter = session.get(cntx['base_url'] + '/config', params=params, headers=cntx['json_headers']) assert responseAfter.status_code == 200 @@ -283,7 +290,8 @@ def test_post_config(setup_module): def test_healthcheck(setup_module): """Test Healthcheck""" - cntx = setup_module + cntx, session = setup_module + html_response = requests.get(cntx['base_url'] + '/healthcheck',headers=cntx['html_headers']) assert html_response.status_code == 200 @@ -292,9 +300,10 @@ def test_healthcheck(setup_module): def test_summary(setup_module): """Test Summary Report Page""" - cntx = setup_module - text_response = requests.get(cntx['base_url'] + '/summary',headers=cntx['plain_text_headers']) - json_response = requests.get(cntx['base_url'] + '/summary',headers=cntx['json_headers']) + cntx, session = setup_module + + text_response = session.get(cntx['base_url'] + '/summary',headers=cntx['plain_text_headers']) + json_response = session.get(cntx['base_url'] + '/summary',headers=cntx['json_headers']) assert text_response.status_code == 200 assert json_response.status_code == 200 @@ -305,12 +314,23 @@ def test_summary(setup_module): assert text_content != json_response +def test_detail(setup_module): + """Test the Detail Job Page""" + cntx, session = setup_module + + response = session.get(cntx['base_url'] + '/detail', headers=cntx['html_headers']) + + assert response.status_code == 200 + html_content = response.content.decode('utf-8') + assert len(html_content) > 10 + def test_dynamic_html(setup_module): """Test progress , grid and controll""" - cntx = setup_module - progress_response = requests.get(cntx['base_url'] + '/progress',headers=cntx['html_headers']) - grid_response = requests.get(cntx['base_url'] + '/grid',headers=cntx['html_headers']) - control_response = requests.get(cntx['base_url'] + '/control',headers=cntx['html_headers']) + cntx, session = setup_module + + progress_response = session.get(cntx['base_url'] + '/progress',headers=cntx['html_headers']) + grid_response = session.get(cntx['base_url'] + '/grid',headers=cntx['html_headers']) + control_response = session.get(cntx['base_url'] + '/control',headers=cntx['html_headers']) assert progress_response.status_code == 200 assert grid_response.status_code == 200 @@ -320,9 +340,9 @@ def test_dynamic_html(setup_module): grid_html = grid_response.content.decode('utf-8') control_html = control_response.content.decode('utf-8') - assert len(progress_html) > 20 - assert len(grid_html) > 20 - assert len(control_html) > 20 + assert len(progress_html) > 1065 + assert len(grid_html) > 1065 + assert len(control_html) > 1065 assert progress_html != grid_html assert progress_html != control_html diff --git a/orchestration-service/web_service.py b/orchestration-service/web_service.py index 120c250..59321dc 100644 --- a/orchestration-service/web_service.py +++ b/orchestration-service/web_service.py @@ -45,6 +45,16 @@ def application(request): Content Type {request.headers.get('Content-Type')} Accept {request.headers.get('Accept')} ETag {request.headers.get('ETag')}""") + + # auth check /progress /grid /control /detail are HTML pages + # healthcheck does not require auth + # they have their own auth flow and messages, so we skip them for out auth check + # this protects API calls + if request.path not in ['/progress', '/grid', '/control', '/detail', '/healthcheck', '/oauthback'] and \ + not (ALWAYS_ALLOW or GitHubOauth.is_authorized(request.cookies, None, + env_name_values.get('user_info_url'), env_name_values.get('team'))): + return Response("Not Authorized", status=403) + if request.path == '/job': # Work through GET Requests first if request.method == 'GET': @@ -243,10 +253,12 @@ def application(request): # quote url encodes string referring_url = request.path - if 'replay_auth' in request.cookies: + if ALWAYS_ALLOW or \ + GitHubOauth.is_authorized(request.cookies, None, + env_name_values.get('user_info_url'), env_name_values.get('team')): # Retrieve the auth cookie cookie_value = request.cookies.get('replay_auth') - login, avatar_url = GitHubOauth.str_to_profile(cookie_value) + login, avatar_url = GitHubOauth.str_to_public_profile(cookie_value) html_content = html_factory.contents('header.html') \ + html_factory.profile_top_bar_html(login, avatar_url) \ + html_factory.contents('navbar.html') \ @@ -271,14 +283,16 @@ def application(request): code = request.args.get('code') # hold token for very short time - bearer_token = GitHubOauth.get_access_token(code, env_name_values) + bearer_token = GitHubOauth.get_oauth_access_token(code, env_name_values) if bearer_token: - profile_data = GitHubOauth.get_public_profile(bearer_token, env_name_values) - login, avatar_url = GitHubOauth.str_to_profile(profile_data) - is_authorized = GitHubOauth.is_authorized(bearer_token, login, env_name_values.get('team')) + profile_data = GitHubOauth.create_auth_string(bearer_token, env_name_values.get('user_info_url')) + login, avatar_url = GitHubOauth.str_to_public_profile(profile_data) + is_authorized_member = GitHubOauth.check_membership(bearer_token, + login, + env_name_values.get('team')) # wipe out token after getting profile data, and checking authorization bearer_token = None - if is_authorized: + if is_authorized_member: # Calculate the expiration time, 1 week (7 days) from now expires = datetime.utcnow() + timedelta(days=7) @@ -324,8 +338,11 @@ def application(request): help='path to static html files') parser.add_argument('--log', type=str, default="orchestration.log", help="log file for service") + parser.add_argument('--disable-auth', action='store_true', + help="when set disables access control, used for testing") args = parser.parse_args() + ALWAYS_ALLOW = args.disable_auth # setup logging logging.basicConfig(filename=args.log,