Skip to content

Commit

Permalink
Show permission warning on waiver form
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 committed Apr 21, 2023
1 parent 078a639 commit 3b66da4
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 3b66da4

Please sign in to comment.