Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Commit

Permalink
Merge pull request #141 from communitiesuk/SMD-646/feature/fund-path-…
Browse files Browse the repository at this point in the history
…routing

[SMD-646] add fund code to Auth and add coded routes
  • Loading branch information
bobbi-hbrown authored Mar 12, 2024
2 parents ed521d4 + bdb29b2 commit 2bb8379
Show file tree
Hide file tree
Showing 12 changed files with 224 additions and 100 deletions.
70 changes: 30 additions & 40 deletions app/main/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,59 +2,49 @@

from flask import abort, current_app, g

from app.main.authorisation import AuthBase, AuthMapping
from app.main.authorisation import AuthBase
from app.main.fund import FundConfig


def auth_required(func):
@wraps(func)
def decorated_function(*args, **kwargs):
"""Checks that the user is authorised to submit.
Checks if a user is authorised to submit by:
1. deriving the fund they're submitting for from their role
2. fetching the relevant auth mappings for that fund
3. retrieving the auth for that user from the relevant auth mapping
class Access:
fund: FundConfig
auth: AuthBase

If the fund window is active and user is authorised to submit, two attributes are added to the request context:
- g.fund: contains fund specific context used throughout the application
- g.auth: contains information that determine what the user is allowed to submit
def __init__(self, fund: FundConfig, auth: AuthBase):
self.fund = fund
self.auth = auth

Otherwise, aborts and redirects to 401 (unauthorised) page.

TODO: As mentioned in app/__init__.py, going forwards we should look to extract and encapsulate this "fund" and
"auth" data in separate microservices.
:raises 401 Unauthorized: If the user has an invalid role(s) or no auth.
"""
funds: list[FundConfig] = current_app.config["FUND_CONFIGS"].get_active_funds(g.user.roles)

if not funds:
current_app.logger.info(f"User: {g.user.email} is not linked with any active funds.")
def set_user_access(func):
@wraps(func)
def decorated_function(*args, **kwargs):
available_funds = current_app.config["FUND_CONFIGS"].get_active_funds(g.user.roles)
access = {}
for fund in available_funds:
auth_mapping = current_app.config["AUTH_MAPPINGS"].get_auth(fund.fund_name)
auth = auth_mapping.get_auth(g.user.email)
if auth is None:
current_app.logger.info(f"User: {g.user.email} is not authorised to submit for fund: {fund.fund_name}")
continue
access[fund.fund_code] = Access(fund, auth)
if not access:
current_app.logger.info(f"User: {g.user.email} is not authorised for any active funds.")
# TODO: Replace with a more suitable error screen than unauthorised
abort(401)
elif len(funds) > 1:
current_app.logger.error(
f"User: {g.user.email} can Submit for multiple active funds, {[fund.fund_name for fund in funds]}"
)
abort(401)

fund = funds[0] # we currently only support a user submitting for a single active fund

auth_mapping: AuthMapping = current_app.config["AUTH_MAPPINGS"].get_auth(fund.fund_name)
auth: AuthBase = auth_mapping.get_auth(g.user.email)

if auth is None:
current_app.logger.error(f"User: {g.user.email} has not been assigned any authorisation")
abort(401)
g.access = access

current_app.logger.info(
f"User: {g.user.email} from {', '.join(auth.get_organisations())} is authorised for: {auth.get_auth_dict()}"
{
"Detail": "User authorised for funds. Adding access to request context.",
"User": g.user.email,
"Funds": [
{"Fund": access_obj.fund.fund_name, "Organisations": access_obj.auth.get_organisations()}
for access_obj in access.values()
],
}
)

g.fund = fund
g.auth = auth

return func(*args, **kwargs)

return decorated_function
4 changes: 4 additions & 0 deletions app/main/fund.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class FundConfig:
def __init__(
self,
fund_name: str,
fund_code: str,
user_role: str,
email: str,
active: bool,
Expand Down Expand Up @@ -61,6 +62,7 @@ def __init__(
assert isinstance(current_deadline, datetime.date), "Deadline must be a datetime.date"

self.fund_name = fund_name
self.fund_code = fund_code
self.user_role = user_role
self.email = email
self.active = active
Expand Down Expand Up @@ -101,6 +103,7 @@ def get_active_funds(self, roles: list[str]):
email=Config.TF_CONFIRMATION_EMAIL_ADDRESS,
active=True,
auth_class=TFAuth,
fund_code="TF",
)

PATHFINDERS_APP_CONFIG = FundConfig(
Expand All @@ -112,4 +115,5 @@ def get_active_funds(self, roles: list[str]):
email=Config.PF_CONFIRMATION_EMAIL_ADDRESS,
active=True,
auth_class=PFAuth,
fund_code="PF",
)
62 changes: 39 additions & 23 deletions app/main/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from fsd_utils.authentication.config import SupportedApp
from fsd_utils.authentication.decorators import login_requested, login_required
from werkzeug.datastructures import FileStorage
from werkzeug.exceptions import HTTPException
from werkzeug.exceptions import HTTPException, abort

from app.const import (
MIMETYPE,
Expand All @@ -16,7 +16,7 @@
)
from app.main import bp
from app.main.data_requests import post_ingest
from app.main.decorators import auth_required
from app.main.decorators import set_user_access
from app.main.notify import send_confirmation_emails
from app.utils import days_between_dates, is_load_enabled
from config import Config
Expand All @@ -28,24 +28,41 @@ def index():
if not g.is_authenticated:
return redirect(url_for("main.login"))
else:
return redirect(url_for("main.upload"))
return redirect(url_for("main.dashboard"))


@bp.route("/login", methods=["GET"])
def login():
return render_template("login.html")


@bp.route("/upload", methods=["GET", "POST"])
@bp.route("/dashboard", methods=["GET"])
@login_required(return_app=SupportedApp.POST_AWARD_SUBMIT)
@auth_required
def upload():
@set_user_access
def dashboard():
return render_template("dashboard.html", authorised_funds=g.access.items())


@bp.route("/upload/<fund_code>/<round>", methods=["GET", "POST"])
@login_required(return_app=SupportedApp.POST_AWARD_SUBMIT)
@set_user_access
def upload(fund_code, round):

if fund_code not in g.access:
abort(401)

fund = g.access[fund_code].fund
auth = g.access[fund_code].auth

if request.method == "GET":
return render_template(
"upload.html",
days_to_deadline=days_between_dates(datetime.now().date(), g.fund.current_deadline),
reporting_period=g.fund.current_reporting_period,
fund=g.fund.fund_name,
days_to_deadline=days_between_dates(datetime.now().date(), fund.current_deadline),
reporting_period=fund.current_reporting_period,
fund_name=fund.fund_name,
fund_code=fund.fund_code,
current_reporting_round=fund.current_reporting_round,
local_authorities=auth.get_organisations(),
)

if request.method == "POST":
Expand All @@ -57,9 +74,9 @@ def upload():
pre_errors, validation_errors, metadata = post_ingest(
excel_file,
{
"fund_name": g.fund.fund_name,
"reporting_round": g.fund.current_reporting_round,
"auth": json.dumps(g.auth.get_auth_dict()),
"fund_name": fund.fund_name,
"reporting_round": fund.current_reporting_round,
"auth": json.dumps(auth.get_auth_dict()),
"do_load": is_load_enabled(),
},
)
Expand All @@ -75,9 +92,11 @@ def upload():
return render_template(
"upload.html",
pre_error=pre_errors,
days_to_deadline=days_between_dates(datetime.now().date(), g.fund.current_deadline),
reporting_period=g.fund.current_reporting_period,
fund=g.fund.fund_name,
days_to_deadline=days_between_dates(datetime.now().date(), fund.current_deadline),
fund_name=fund.fund_name,
fund_code=fund.fund_code,
current_reporting_round=fund.current_reporting_round,
local_authorities=auth.get_organisations(),
)
elif validation_errors:
# Validation failure
Expand All @@ -87,19 +106,16 @@ def upload():
else:
current_app.logger.info(VALIDATION_LOG)

return render_template(
"validation-errors.html",
validation_errors=validation_errors,
)
return render_template("validation-errors.html", validation_errors=validation_errors, fund=fund)
else:
# Success
# TODO: enable confirmation emails for PF once template changes are confirmed
if Config.SEND_CONFIRMATION_EMAILS and g.fund.fund_name != "Pathfinders":
if Config.SEND_CONFIRMATION_EMAILS and fund.fund_name != "Pathfinders":
send_confirmation_emails(
excel_file,
fund=g.fund.fund_name,
reporting_period=g.fund.current_reporting_period,
fund_email=g.fund.email,
fund=fund.fund_name,
reporting_period=fund.current_reporting_period,
fund_email=fund.email,
user_email=g.user.email,
metadata=metadata,
)
Expand Down
43 changes: 43 additions & 0 deletions app/templates/main/dashboard.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{% extends "base.html" %}

{%- from "help-links-dropdown.html" import helpLinksDropdown -%}
{%- from 'govuk_frontend_jinja/components/button/macro.html' import govukLink -%}

{% block beforeContent %}
{{ super() }}
{% endblock beforeContent %}

{% block content %}
<h1 class="govuk-heading-xl">Submit monitoring and evaluation data dashboard</h1>

<h2 class="govuk-heading-m">All funds ({{ authorised_funds | length }})</h2>
<hr class="govuk-section-break govuk-section-break--s govuk-section-break--visible">
<dl class="govuk-summary-list">
{% for fund_code, fund_data in authorised_funds %}

{% set local_authorities = fund_data.auth.get_organisations() %}
{% set fund = fund_data.fund %}

<div class="govuk-summary-list__row">
<dt class="govuk-summary-list__key">
<a class="govuk-heading-m govuk-link--no-visited-state" href={{ url_for('main.upload', fund_code=fund_code, round=fund.current_reporting_round) }}> {{ fund.fund_name }}</a>
<p class="govuk-body"><b>Local Authority</b></p>
<p class="govuk-body"><b>Reporting Period</b></p>
</dt>
<dd class="govuk-summary-list__value">
<br>
<br>
<p class="govuk-body"> {{ ", ".join(local_authorities) if local_authorities|length > 1 else local_authorities[0] }} </p>
<p class="govuk-body"> {{ fund.current_reporting_period }}</p>

</dd>

</div>

{% endfor %}

</dl>

{{ helpLinksDropdown() }}

{% endblock content %}
5 changes: 2 additions & 3 deletions app/templates/main/upload.html
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@
"classes": notificationClass
}) }}
{% endif %}

{{ uploadTable(g.auth.get_organisations(), fund, reporting_period) }}
{{ uploadTable(local_authorities, fund_name, reporting_period) }}

<div class="upload-data-container govuk-!-margin-top-5">
<form method="post" action="{{ url_for('main.upload') }}" enctype="multipart/form-data">
<form method="post" action="{{ url_for('main.upload', fund_code=fund_code, round=current_reporting_round) }}" enctype="multipart/form-data">
<label class="govuk-heading-m">Upload your data return</label>
{% if pre_error %}
{{ govukFileUpload({'id': 'ingest_spreadsheet',
Expand Down
4 changes: 2 additions & 2 deletions app/templates/main/uploadTable.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{%- from "govuk_frontend_jinja/components/table/macro.html" import govukTable -%}

{% macro uploadTable(local_authorities, fund, reporting_period) %}
{% macro uploadTable(local_authorities, fund_name, reporting_period) %}

{% set localAuthorities %} {{ ", ".join(local_authorities) if local_authorities|length > 1 else local_authorities[0] }} {% endset %}

Expand All @@ -22,7 +22,7 @@ <h1 class="govuk-heading-l">Submit monitoring and evaluation data</h1>
"text": "Fund"
},
{
"html": fund
"html": fund_name
}
],
[
Expand Down
2 changes: 1 addition & 1 deletion app/templates/main/validation-errors.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

{% block content %}
<div class="upload-data-container govuk-!-margin-top-5">
<form method="post" action="{{ url_for('main.upload') }}" enctype="multipart/form-data">
<form method="post" action="{{ url_for('main.upload', fund_code=fund.fund_code, round=fund.current_reporting_round) }}" enctype="multipart/form-data">
<h1 class="govuk-heading-l">There are errors in your return</h1>
<p class="govuk-body">Fix these errors and re-upload your return.</p>
{{ errorTable(validation_errors) }}
Expand Down
2 changes: 1 addition & 1 deletion config/envs/development.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ class DevelopmentConfig(DefaultConfig):
DEBUG_USER = {
"full_name": "Development User",
"email": "dev@communities.gov.uk",
"roles": [DEBUG_USER_ROLE],
"roles": ["PF_MONITORING_RETURN_SUBMITTER", "TF_MONITORING_RETURN_SUBMITTER"],
"highest_role_map": {},
}
4 changes: 4 additions & 0 deletions config/envs/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class UnitTestConfig(DefaultConfig):
),
}

PF_ADDITIONAL_EMAIL_LOOKUPS = {
"multiple_orgs@contractor.com": (("Rotherham Metropolitan Borough Council",),),
}

# notify client passes init with this key and is then mocked
NOTIFY_API_KEY = "fake_key-0ab1234a-12a3-12ab-12a3-a1b2cd3e4f5g-a123b456-1a23-1abv-a1bc-123a45678910"
AUTO_BUILD_ASSETS = True
13 changes: 13 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ def mocked_pf_auth(mocker):
)


@pytest.fixture()
def mocked_pf_and_tf_auth(mocker):
# mock authorised user with Pathfinders role
mocker.patch(
"fsd_utils.authentication.decorators._check_access_token",
return_value={
"accountId": "pf-tf-test-user",
"roles": ["PF_MONITORING_RETURN_SUBMITTER", "TF_MONITORING_RETURN_SUBMITTER"],
"email": "test-user@wigan.gov.uk",
},
)


@pytest.fixture()
def flask_test_client(mocked_auth) -> FlaskClient:
"""
Expand Down
2 changes: 2 additions & 0 deletions tests/test_fund.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def test_fund_service():
active=True,
auth_class=AuthBase,
user_role="Test Role",
fund_code="TF",
)
}
)
Expand Down Expand Up @@ -56,6 +57,7 @@ def test_fund_config_validations():
active=True,
auth_class=AuthBase,
user_role="Test Role",
fund_code="TF",
)

# success
Expand Down
Loading

0 comments on commit 2bb8379

Please sign in to comment.