Skip to content

Commit

Permalink
Show permission warning on waiver form (#70)
Browse files Browse the repository at this point in the history
Shows warning if testcase parameter is used and current user cannot
waive it.

JIRA: RHELWF-9056
  • Loading branch information
hluk authored May 4, 2023
1 parent 215ba27 commit d8ddde1
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 36 deletions.
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def app():


@pytest.fixture
def session(monkeypatch):
def session(monkeypatch, app):
"""Patch Flask-SQLAlchemy to use a specific connection"""
db.drop_all()
db.create_all()
Expand Down
82 changes: 65 additions & 17 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,29 @@
from werkzeug.exceptions import Unauthorized
import waiverdb.auth
import flask_oidc
from flask import g


@pytest.fixture
def oidc_token(app):
with mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info') as mocked:
mocked.return_value = {
'active': True, 'username': 'testuser', 'scope': 'openid waiverdb_scope'}
with app.app_context():
g.oidc_id_token = mocked()
yield g.oidc_id_token


@pytest.fixture
def verify_authorization():
with mock.patch("waiverdb.api_v1.verify_authorization") as mocked:
yield mocked


@pytest.fixture
def permissions():
with mock.patch("waiverdb.api_v1.permissions") as mocked:
yield mocked


@pytest.mark.usefixtures('enable_kerberos')
Expand Down Expand Up @@ -64,12 +87,8 @@ def test_get_user_without_token(self, session):
waiverdb.auth.get_user(request)
assert self.auth_missing_error in str(excinfo.value)

@mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info')
def test_get_user_with_invalid_token(self, mocked_get_token, session):
# http://vsbattles.wikia.com/wiki/Son_Goku
name = 'Son Goku'
mocked_get_token.return_value = {'active': False, 'username': name,
'scope': 'openid waiverdb_scope'}
def test_get_user_with_invalid_token(self, oidc_token, session):
oidc_token["active"] = False
headers = {'Authorization': 'Bearer invalid'}
request = mock.MagicMock()
request.headers.return_value = mock.MagicMock(spec_set=dict)
Expand All @@ -80,20 +99,51 @@ def test_get_user_with_invalid_token(self, mocked_get_token, session):
waiverdb.auth.get_user(request)
assert self.invalid_token_error in excinfo.value.get_description()

@mock.patch.object(flask_oidc.OpenIDConnect, '_get_token_info')
def test_get_user_good(self, mocked_get_token, session):
# http://vsbattles.wikia.com/wiki/Son_Goku
name = 'Son Goku'
mocked_get_token.return_value = {'active': True, 'username': name,
'scope': 'openid waiverdb_scope'}
def test_get_user_good(self, oidc_token, session):
headers = {'Authorization': 'Bearer foobar'}
request = mock.MagicMock()
request.headers.return_value = mock.MagicMock(spec_set=dict)
request.headers.__getitem__.side_effect = headers.__getitem__
request.headers.__setitem__.side_effect = headers.__setitem__
request.headers.__contains__.side_effect = headers.__contains__
user, header = waiverdb.auth.get_user(request)
assert user == name
assert user == oidc_token["username"]

def test_warning_banner(
self,
verify_authorization,
permissions,
oidc_token,
session,
client,
):
verify_authorization.side_effect = Unauthorized("Unauthorized")
permissions.return_value = [{"testcases": ["a.b.c"], "groups": []}]
headers = {'Authorization': 'Bearer foobar'}
r = client.get('/api/v1.0/waivers/new?testcase=a.b.c', headers=headers)
warning_banner = (
'<div class="alert alert-danger" role="alert">'
'401 Unauthorized: Unauthorized<br>'
'<a href=/api/v1.0/permissions?testcase=a.b.c>'
'See who has permission to waive a.b.c test case.</a></div>'
)
assert warning_banner in r.text
assert r.status_code == 200

def test_no_warning_banner(
self,
verify_authorization,
permissions,
oidc_token,
session,
client,
):
verify_authorization.return_value = True
permissions.return_value = [{"testcases": ["a.b.c"], "groups": []}]
headers = {'Authorization': 'Bearer foobar'}
r = client.get('/api/v1.0/waivers/new?testcase=a.b.c', headers=headers)
assert "alert" not in r.text
assert r.status_code == 200


@pytest.mark.usefixtures('enable_ssl')
Expand All @@ -112,15 +162,13 @@ def test_SSL_CLIENT_S_DN_is_not_set_should_raise_error(self):
in excinfo.value.get_description()

def test_good_ssl_cert(self):
# http://vsbattles.wikia.com/wiki/Son_Goku
name = 'Son Goku'
ssl = {
'SSL_CLIENT_VERIFY': 'SUCCESS',
'SSL_CLIENT_S_DN': name
'SSL_CLIENT_S_DN': 'testuser',
}
request = mock.MagicMock(environ=ssl)
user, header = waiverdb.auth.get_user(request)
assert user == name
assert user == 'testuser'


@pytest.mark.usefixtures('enable_kerberos_oidc_fallback')
Expand Down
64 changes: 46 additions & 18 deletions waiverdb/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@
import logging

import requests
from flask import Blueprint, render_template, request, current_app, Response
from flask import (
Blueprint,
Response,
current_app,
escape,
render_template,
request,
url_for,
)
from flask_oidc import OpenIDConnect
from flask_restful import Resource, Api, reqparse, marshal_with, marshal
from werkzeug.exceptions import (
BadRequest,
Forbidden,
ServiceUnavailable,
Unauthorized,
)
from sqlalchemy.sql.expression import func, and_, or_

Expand Down Expand Up @@ -134,6 +143,39 @@ def _filter_out_obsolete_waivers(query):
return query.filter(Waiver.id.in_(subquery))


def _verify_authorization(user, testcase):
if not permissions():
return True

ldap_host = current_app.config.get('LDAP_HOST')
ldap_searches = current_app.config.get('LDAP_SEARCHES')
if not ldap_searches:
ldap_base = current_app.config.get('LDAP_BASE')
if ldap_base:
ldap_search_string = current_app.config.get(
'LDAP_SEARCH_STRING', '(memberUid={user})'
)
ldap_searches = [{'BASE': ldap_base, 'SEARCH_STRING': ldap_search_string}]
return verify_authorization(user, testcase, permissions(), ldap_host, ldap_searches)


def _authorization_warning(request):
testcase = request.args.get("testcase")
if testcase:
user, _headers = waiverdb.auth.get_user(request)
try:
_verify_authorization(user, testcase)
except Unauthorized as e:
permissions_url = url_for('api_v1.permissionsresource', testcase=testcase)
return (
f"{escape(str(e))}<br>"
f"<a href={permissions_url}>See who has permission to"
f" waive {escape(testcase)} test case."
"</a>"
)
return None


# RP contains request parsers (reqparse.RequestParser).
# Parsers are added in each 'resource section' for better readability
RP = {}
Expand Down Expand Up @@ -373,21 +415,6 @@ def post(self):

return result, 201, headers

def _verify_authorization(self, user, testcase):
if not permissions():
return True

ldap_host = current_app.config.get('LDAP_HOST')
ldap_searches = current_app.config.get('LDAP_SEARCHES')
if not ldap_searches:
ldap_base = current_app.config.get('LDAP_BASE')
if ldap_base:
ldap_search_string = current_app.config.get(
'LDAP_SEARCH_STRING', '(memberUid={user})'
)
ldap_searches = [{'BASE': ldap_base, 'SEARCH_STRING': ldap_search_string}]
return verify_authorization(user, testcase, permissions(), ldap_host, ldap_searches)

def _create_waiver(self, args, user):
proxied_by = None
if args.get('username'):
Expand Down Expand Up @@ -444,7 +471,7 @@ def _create_waiver(self, args, user):
if not args['testcase']:
raise BadRequest({'testcase': 'Missing required parameter in the JSON body'})

self._verify_authorization(user, args['testcase'])
_verify_authorization(user, args['testcase'])

# brew-build is an alias for koji_build
if args['subject_type'] == 'brew-build':
Expand Down Expand Up @@ -482,7 +509,8 @@ def get(self):
:statuscode 200: The HTML with the form is returned.
"""
html = render_template('new_waiver.html', request_args=request.args)
warning = _authorization_warning(request)
html = render_template('new_waiver.html', warning=warning, request_args=request.args)
return Response(html, mimetype='text/html')

@oidc.require_login
Expand Down
3 changes: 3 additions & 0 deletions waiverdb/templates/new_waiver.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
{% endblock %}

{% block body %}
{% if warning %}
<div class="alert alert-danger" role="alert">{{ warning | safe }}</div>
{% endif %}
<form method="post">
<div class="mb-3">
<label for="subject_type" class="form-label">Subject Type</label>
Expand Down

0 comments on commit d8ddde1

Please sign in to comment.