Skip to content

Commit

Permalink
Merge pull request #66 from geoadmin/feat-PB-737-block-shortening-of-…
Browse files Browse the repository at this point in the history
…external-urls

PB-737: Block shortening of external urls
  • Loading branch information
LukasJoss authored Aug 8, 2024
2 parents 19c1eb0 + 2e98761 commit e45f03a
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 10 deletions.
6 changes: 3 additions & 3 deletions .env.default
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
AWS_ACCESS_KEY_ID=dummy-123
AWS_SECRET_ACCESS_KEY=dummy-123
AWS_ACCESS_KEY_ID=dummy123
AWS_SECRET_ACCESS_KEY=dummy123
AWS_ENDPOINT_URL=http://localhost:8080
AWS_DEFAULT_REGION=eu-central-1
AWS_DYNAMODB_TABLE_NAME=test-db
ALLOWED_DOMAINS=.*localhost,.*admin\.ch,.*bgdi\.ch
ALLOWED_DOMAINS=.*localhost((:[0-9]*)?|\/)?,.*admin\.ch,.*bgdi\.ch
STAGING=local
4 changes: 2 additions & 2 deletions .env.testing
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ALLOWED_DOMAINS=.*\.geo\.admin\.ch,.*\.bgdi\.ch,http://localhost
ALLOWED_DOMAINS=.*\.geo\.admin\.ch,.*\.bgdi\.ch,http://localhost((:[0-9]*)?|\/)?
AWS_ACCESS_KEY_ID=testing
AWS_SECRET_ACCESS_KEY=testing
AWS_SECURITY_TOKEN=testing
Expand All @@ -8,4 +8,4 @@ AWS_SESSION_TOKEN=testing
AWS_DEFAULT_REGION=us-east-1
AWS_DYNAMODB_TABLE_NAME=test-db

STAGING=test
STAGING=test
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,17 @@ This will serve the application with the Gunicorn layer in front of the applicat
make dockerrun

This will serve the application with the wsgi server, inside a container.

To stop serving through containers,

make shutdown

Is the command you're looking for.

A curl example for testing the generation of shortlinks on the local db is:

curl -X POST -H "Content-Type: application/json" -H "Origin: http://localhost:8000" -d '{"url":"http://localhost:8000"}' http://localhost:5000

### Docker helpers

From each github PR that is merged into `master` or into `develop`, one Docker image is built and pushed on AWS ECR with the following tag:
Expand Down
2 changes: 1 addition & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@


def is_domain_allowed(domain):
return re.match(ALLOWED_DOMAINS_PATTERN, domain) is not None
return re.fullmatch(ALLOWED_DOMAINS_PATTERN, domain) is not None


@app.before_request
Expand Down
3 changes: 2 additions & 1 deletion app/helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import re
from itertools import chain
from pathlib import Path
from urllib.parse import urlparse

import validators
import yaml
Expand Down Expand Up @@ -112,7 +113,7 @@ def get_url():
f"The url given as parameter was too long. (limit is 2046 "
f"characters, {len(url)} given)"
)
if not re.match(ALLOWED_DOMAINS_PATTERN, url):
if not re.fullmatch(ALLOWED_DOMAINS_PATTERN, urlparse(url).netloc):
logger.error('URL(%s) given as a parameter is not allowed', url)
abort(400, 'URL given as a parameter is not allowed.')

Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,6 @@ services:
links:
- dynamodb-local
environment:
- AWS_ACCESS_KEY_ID=dummy-123
- AWS_SECRET_ACCESS_KEY=dummy-123
- AWS_ACCESS_KEY_ID=dummy123
- AWS_SECRET_ACCESS_KEY=dummy123
- AWS_DEFAULT_REGION=wonderland
2 changes: 1 addition & 1 deletion tests/unit_tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def assertCors(
): # pylint: disable=invalid-name
self.assertIn('Access-Control-Allow-Origin', response.headers)
self.assertIsNotNone(
re.match(origin_pattern, response.headers['Access-Control-Allow-Origin']),
re.fullmatch(origin_pattern, response.headers['Access-Control-Allow-Origin']),
msg=f"Access-Control-Allow-Origin={response.headers['Access-Control-Allow-Origin']}"
f" doesn't match {origin_pattern}"
)
Expand Down
21 changes: 21 additions & 0 deletions tests/unit_tests/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,25 @@ def test_create_shortlink_non_allowed_hostname(self):
}
)

def test_create_shortlink_non_allowed_hostname_containing_admin_address(self):
response = self.app.post(
url_for('create_shortlink'),
json={"url": "https://map.geo.admin.ch.non-allowed.hostname.ch/test"},
headers={"Origin": "map.geo.admin.ch"}
)
self.assertEqual(response.status_code, 400)
self.assertCors(response, ['POST', 'OPTIONS'])
self.assertIn('application/json', response.content_type)
self.assertEqual(
response.json,
{
'success': False,
'error': {
'code': 400, 'message': 'URL given as a parameter is not allowed.'
}
}
)

def test_create_shortlink_url_too_long(self):
url = self.invalid_urls_list[0]
response = self.app.post(
Expand Down Expand Up @@ -276,6 +295,7 @@ def test_fetch_full_url_from_shortlink_url_not_found(self):
@params(
None,
{'Origin': 'www.example'},
{'Origin': 'map.geo.admin.ch.non-allowed.hostname.ch'},
{'Origin': ''},
{
'Origin': 'www.example', 'Sec-Fetch-Site': 'cross-site'
Expand All @@ -286,6 +306,7 @@ def test_fetch_full_url_from_shortlink_url_not_found(self):
{
'Origin': 'www.example', 'Sec-Fetch-Site': 'same-origin'
},
{'Referer': 'map.geo.admin.ch.non-allowed.hostname.ch'},
{'Referer': 'http://www.example'},
{'Referer': ''},
)
Expand Down

0 comments on commit e45f03a

Please sign in to comment.