Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to send headers to proxied documents #42

Merged
merged 4 commits into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/h_vialib/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from webob.multidict import MultiDict

from h_vialib.secure import ViaSecureURL
from h_vialib.secure import Encryption, ViaSecureURL


class ViaDoc:
Expand Down Expand Up @@ -45,6 +45,7 @@ def __init__(self, secret, service_url=None, html_service_url=None):
:param service_url: Location of the via server
:param html_service_url: Location of the Via HTML presenter
"""
self._secure_secrets = Encryption(secret.encode("utf-8"))
self._secure_url = ViaSecureURL(secret)
self._service_url = urlparse(service_url) if service_url else None
self._html_service_url = html_service_url
Expand All @@ -56,7 +57,10 @@ def __init__(self, secret, service_url=None, html_service_url=None):
"via.external_link_mode": "new-tab",
}

def url_for(self, url, content_type=None, options=None, blocked_for=None):
# pylint:disable=too-many-arguments
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon this might keep growing and have a similar interface to requests.request.

Not a problem IMO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def url_for(
self, url, content_type=None, options=None, blocked_for=None, headers=None
):
Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headers are only supported by certain Via endpoints, right? The ones that proxy in Python, not the ones that proxy in NGINX. Are there some url's where url_for(url, headers=my_headers) will return a URL containing headers that Via will actually ignore because it's going to proxy that URL through NGINX?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will return a URL containing headers that Via will actually ignore because it's going to proxy that URL through NGINX?

Yes, all HTML ones and pdfs not proxied by python (so all of them except one drive and d2l ones).

Applying the headers to everything will be a big tasks (not sure even if possible without writing nginx lua code).

Maybe we could just explain the limitation on the method's docstring?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment about this. Not ideal but naming this headers_for_python_proxing is not better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could make Via throw some sort of "not implemented" error if any headers are passed to it and it would have proxied the request through NGINX rather than through Python?

"""Generate a Via URL to display a given URL.

If provided, the options will be merged with default Via options.
Expand All @@ -66,6 +70,10 @@ def url_for(self, url, content_type=None, options=None, blocked_for=None):
or "html")
:param options: Any additional params to add to the URL
:param blocked_for: context for the blocked pages
:param headers: Any headers needed to make the request to `url`.
The headers are sent encrypted to Via.
Note that the headers are only used when the content is proxied with
python, ie only PDFs from certain sources.
:return: Full Via URL suitable for redirecting a user to
"""
doc = ViaDoc(url, content_type)
Expand All @@ -74,6 +82,9 @@ def url_for(self, url, content_type=None, options=None, blocked_for=None):
if options:
query.update(options)

if headers:
query["via.secret.headers"] = self._secure_secrets.encrypt_dict(headers)
marcospri marked this conversation as resolved.
Show resolved Hide resolved

if blocked_for:
query["via.blocked_for"] = blocked_for

Expand Down
1 change: 1 addition & 0 deletions src/h_vialib/secure/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Security helpers."""

from h_vialib.secure.encryption import Encryption
from h_vialib.secure.expiry import quantized_expiry
from h_vialib.secure.token import SecureToken
from h_vialib.secure.url import SecureURL, ViaSecureURL
26 changes: 26 additions & 0 deletions src/h_vialib/secure/encryption.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import json

from jose import constants, jwe


class Encryption:
JWE_ALGORITHM = constants.ALGORITHMS.DIR
JWE_ENCRYPTION = constants.ALGORITHMS.A128CBC_HS256

def __init__(self, secret: bytes):
self._secret = secret.ljust(32)[:32]

def encrypt_dict(self, payload: dict) -> str:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using JWE simplified this a lot.

"""Encrypt a dictionary as a JWE."""

return jwe.encrypt(
json.dumps(payload),
self._secret,
algorithm=self.JWE_ALGORITHM,
encryption=self.JWE_ENCRYPTION,
).decode("utf-8")

def decrypt_dict(self, payload: str) -> dict:
"""Decrypts payloads created by `encrypt_dict`."""

return json.loads(jwe.decrypt(payload, self._secret))
17 changes: 16 additions & 1 deletion tests/unit/h_vialib/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ def test_url_for_allows_you_to_override_options(self, client, content_type):

assert final_url == Any.url.containing_query(override)

def test_url_for_with_headers(self, client, Encryption):
headers = {"some": "header"}
Encryption.return_value.encrypt_dict.return_value = "secure headers"

final_url = client.url_for("http://example.com", headers=headers)

Encryption.return_value.encrypt_dict.assert_called_once_with(headers)
assert final_url == Any.url.containing_query(
{"via.secret.headers": "secure headers"}
)

@pytest.mark.parametrize("content_type", (None, "pdf", "html"))
def test_url_for_raises_without_a_service_url(self, content_type):
client = ViaClient(
Expand Down Expand Up @@ -147,7 +158,11 @@ def test_it_fixes_html_urls_with_bare_hostnames(self, client, url, path):
assert signed_url == Any.url.with_path(path)

@pytest.fixture
def client(self):
def Encryption(self, patch):
return patch("h_vialib.client.Encryption")

@pytest.fixture
def client(self, Encryption): # pylint:disable=unused-argument
return ViaClient(
service_url=self.VIA_URL,
html_service_url=self.VIAHTML_URL,
Expand Down
50 changes: 50 additions & 0 deletions tests/unit/h_vialib/secure/encryption_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import pytest
from jose import constants

from h_vialib.secure import Encryption


class TestEncryption:
def test_encrypt_dict_round_trip(self, encryption):
payload_dict = {"some": "data"}

encrypted = encryption.encrypt_dict(payload_dict)

assert encryption.decrypt_dict(encrypted) == payload_dict

def test_encrypt_dict(self, encryption, secret, json, jwe):
payload_dict = {"some": "data"}

encrypted = encryption.encrypt_dict(payload_dict)

json.dumps.assert_called_with(payload_dict)
jwe.encrypt.assert_called_once_with(
json.dumps.return_value,
secret.ljust(32),
algorithm=constants.ALGORITHMS.DIR,
encryption=constants.ALGORITHMS.A128CBC_HS256,
)
assert encrypted == jwe.encrypt.return_value.decode.return_value

def test_decrypt_dict(self, encryption, secret, jwe, json):
plain_text_dict = encryption.decrypt_dict("payload")

jwe.decrypt.assert_called_once_with("payload", secret.ljust(32))
json.loads.assert_called_once_with(jwe.decrypt.return_value)
assert plain_text_dict == json.loads.return_value

@pytest.fixture
def secret(self):
return b"VERY SECRET"

@pytest.fixture
def encryption(self, secret):
return Encryption(secret)

@pytest.fixture
def json(self, patch):
return patch("h_vialib.secure.encryption.json")

@pytest.fixture
def jwe(self, patch):
return patch("h_vialib.secure.encryption.jwe")