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

Add BasicAuth to transfer API requests #68

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ Developed by [Torchbox](https://torchbox.com/) and sponsored by [The Motley Fool
`WAGTAILTRANSFER_SECRET_KEY` and the per-source `SECRET_KEY` settings are used to authenticate the communication between the source and destination instances; this prevents unauthorised users from using this API to retrieve sensitive data such as password hashes. The `SECRET_KEY` for each entry in `WAGTAILTRANSFER_SOURCES` must match that instance's `WAGTAILTRANSFER_SECRET_KEY`.

* Create a user group (Wagtail admin > Settings > Groups > Add a group) with the permission "Can import pages and snippets from other sites". The "Import" menu item, and the associated import views, will only be available to members of this group (and superusers).
* If any source sites use Basic Access Authentication insert an additional dict key `BASIC_AUTH` with the (username, password) tuple:

WAGTAILTRANSFER_SOURCES = {
'staging': {
'BASE_URL': 'https://staging.example.com/wagtail-transfer/',
'SECRET_KEY': '4ac4822149691395773b2a8942e1a472',
'BASIC_AUTH_SECRET': ('testing', 'super-secret'),
},
}

## Configuration

Expand Down
7 changes: 7 additions & 0 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@
'SECRET_KEY': 'i-am-the-local-secret-key',
}
}
WAGTAILTRANSFER_SOURCES_BASIC_AUTH = {
'staging': {
'BASE_URL': 'https://www.example.com/wagtail-transfer/',
'SECRET_KEY': 'i-am-the-staging-example-secret-key',
'BASIC_AUTH_SECRET': ('staging-user', 'staging-pass'),
},
}

WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items', True), ('tests.advert', 'tagged_items', True)]

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ def test(self, get):

digest = digest_for_source('staging', 'bar=baz')

get.assert_called_once_with(f'https://www.example.com/wagtail-transfer/api/chooser/pages/foo?bar=baz&digest={digest}', headers={'Accept': 'application/json'}, timeout=5)
get.assert_called_once_with(f'https://www.example.com/wagtail-transfer/api/chooser/pages/foo?bar=baz&digest={digest}', auth=None, headers={'Accept': 'application/json'}, timeout=5)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.content, b'test content')
Expand Down
111 changes: 74 additions & 37 deletions tests/tests/test_import.py

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions wagtail_transfer/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,6 @@ def digest_for_source(source, message):
message = message.encode()

return hmac.new(key, message, hashlib.sha1).hexdigest()

def requests_auth(source):
Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty broad try/except for just retrieving a value or None. Could you catch eg KeyError instead?

Copy link
Author

Choose a reason for hiding this comment

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

I guess so... presumably any other errors, if present, will pop up in the settings or requests modules.

Another option, if you prefer it, is to use dict.get() with a default:

return settings.WAGTAILTRANSFER_SOURCES[source].get('BASIC_AUTH', None)

Copy link
Author

Choose a reason for hiding this comment

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

Hi Jacob, it's been a busy week... finally made the above change.

return settings.WAGTAILTRANSFER_SOURCES[source].get('BASIC_AUTH_SECRET', None)
2 changes: 1 addition & 1 deletion wagtail_transfer/field_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def populate_field(self, instance, value, context):
name = pathlib.PurePosixPath(urlparse(value['download_url']).path).name
local_filename = self.field.generate_filename(instance, name)

_file = File(local_filename, value['size'], value['hash'], value['download_url'])
_file = File(local_filename, value['size'], value['hash'], value['download_url'], context.source_site)
try:
imported_file = _file.transfer()
except FileTransferError:
Expand Down
6 changes: 4 additions & 2 deletions wagtail_transfer/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.core.files.base import ContentFile

from .models import ImportedFile
from .auth import requests_auth


@contextmanager
Expand Down Expand Up @@ -96,14 +97,15 @@ class File:

Note that local_filename is only a guideline, it may be changed to avoid conflict with an existing file
"""
def __init__(self, local_filename, size, hash, source_url):
def __init__(self, local_filename, size, hash, source_url, source_site):
self.local_filename = local_filename
self.size = size
self.hash = hash
self.source_url = source_url
self.source_site = source_site

def transfer(self):
response = requests.get(self.source_url)
response = requests.get(self.source_url, auth=requests_auth(self.source_site))

if response.status_code != 200:
raise FileTransferError("Non-200 response from image URL")
Expand Down
17 changes: 10 additions & 7 deletions wagtail_transfer/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class ImportContext:
(for example, once a page is created at the destination, we add its ID mapping so that we
can handle references to it that appear in other imported pages).
"""
def __init__(self):
def __init__(self, source_site):
# A mapping of objects on the source site to their IDs on the destination site.
# Keys are tuples of (model_class, source_id); values are destination IDs.
# model_class must be the highest concrete model in the inheritance tree - i.e.
Expand All @@ -121,9 +121,12 @@ def __init__(self):
# Mapping of source_urls to instances of ImportedFile
self.imported_files_by_source_url = {}

# Source name
self.source_site = source_site


class ImportPlanner:
def __init__(self, root_page_source_pk=None, destination_parent_id=None, model=None):
def __init__(self, root_page_source_pk=None, destination_parent_id=None, model=None, source_site=None):

if root_page_source_pk or destination_parent_id:
self.import_type = 'page'
Expand All @@ -138,7 +141,7 @@ def __init__(self, root_page_source_pk=None, destination_parent_id=None, model=N
else:
raise NotImplementedError("Missing page kwargs or specified model kwarg")

self.context = ImportContext()
self.context = ImportContext(source_site)

self.objectives = set()

Expand Down Expand Up @@ -189,12 +192,12 @@ def __init__(self, root_page_source_pk=None, destination_parent_id=None, model=N
self.failed_creations = set()

@classmethod
def for_page(cls, source, destination):
return cls(root_page_source_pk=source, destination_parent_id=destination)
def for_page(cls, source, destination, source_site):
return cls(root_page_source_pk=source, destination_parent_id=destination, source_site=source_site)

@classmethod
def for_model(cls, model):
return cls(model=model)
def for_model(cls, model, source_site):
return cls(model=model, source_site=source_site)

def add_json(self, json_data):
"""
Expand Down
27 changes: 18 additions & 9 deletions wagtail_transfer/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from rest_framework.fields import ReadOnlyField
from wagtail.models import Page

from .auth import check_digest, digest_for_source
from .auth import check_digest, digest_for_source, requests_auth
from .locators import get_locator_for_model
from .models import get_model_for_path
from .operations import ImportPlanner
Expand Down Expand Up @@ -200,9 +200,12 @@ def chooser_api_proxy(request, source_name, path):
message = request.GET.urlencode()
digest = digest_for_source(source_name, message)

response = requests.get(f"{base_url}{path}?{message}&digest={digest}", headers={
'Accept': request.META['HTTP_ACCEPT'],
}, timeout=api_proxy_timeout_seconds)
response = requests.get(
f"{base_url}{path}?{message}&digest={digest}",
auth=requests_auth(source_name),
headers={'Accept': request.META['HTTP_ACCEPT'],},
timeout=api_proxy_timeout_seconds
)

return HttpResponse(response.content, status=response.status_code)

Expand Down Expand Up @@ -240,7 +243,9 @@ def import_missing_object_data(source, importer: ImportPlanner):

# request the missing object data and add to the import plan
response = requests.post(
f"{base_url}api/objects/", params={'digest': digest}, data=request_data
f"{base_url}api/objects/", params={'digest': digest},
auth=requests_auth(source),
data=request_data
)
importer.add_json(response.content)
importer.run()
Expand All @@ -252,10 +257,14 @@ def import_page(request):
base_url = settings.WAGTAILTRANSFER_SOURCES[source]['BASE_URL']
digest = digest_for_source(source, str(request.POST['source_page_id']))

response = requests.get(f"{base_url}api/pages/{request.POST['source_page_id']}/", params={'digest': digest})
response = requests.get(
f"{base_url}api/pages/{request.POST['source_page_id']}/",
auth=requests_auth(source),
params={'digest': digest}
)

dest_page_id = request.POST['dest_page_id'] or None
importer = ImportPlanner.for_page(source=request.POST['source_page_id'], destination=dest_page_id)
importer = ImportPlanner.for_page(source=request.POST['source_page_id'], destination=dest_page_id, source_site=source)
importer.add_json(response.content)
importer = import_missing_object_data(source, importer)

Expand All @@ -276,8 +285,8 @@ def import_model(request):
source_model_object_id = request.POST.get("source_model_object_id")
url = f"{url}{source_model_object_id}/"

response = requests.get(url, params={'digest': digest})
importer = ImportPlanner.for_model(model=model)
response = requests.get(url, auth=requests_auth(source), params={'digest': digest})
importer = ImportPlanner.for_model(model=model, source_site=source)
importer.add_json(response.content)
importer = import_missing_object_data(source, importer)

Expand Down