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

New Release v1.31.0 - #minor #503

Merged
merged 20 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b84ad16
PB-1281: Add feature flag for new authentication
msom Jan 23, 2025
1e42de4
Merge pull request #501 from geoadmin/feat-PB-1281-add-new-auth-featu…
msom Jan 23, 2025
7876b73
using GET range request instead of HEAD to check external asset reach…
boecklic Jan 23, 2025
7f44864
PB-1372: Check content length of external asset
benschs Jan 23, 2025
89c4804
Merge pull request #499 from geoadmin/feat_PB-1372_range_request_exte…
boecklic Jan 23, 2025
d90b71f
PB-1366: make new auth work for write requests.
adk-swisstopo Jan 23, 2025
4a1303d
Merge pull request #502 from geoadmin/fix-PB-1366-auth-write
adk-swisstopo Jan 27, 2025
2f1d51d
PB-1169: Spec search forecast fields
benschs Jan 22, 2025
ad90dab
PB-1169: Search forecast fields
benschs Jan 23, 2025
b29937c
PB-1169: Spec remove forecast params from Get search
benschs Jan 23, 2025
757c46d
PB-1169: Remove forecast filter from GET search
benschs Jan 23, 2025
d2492aa
Fix for review comments
benschs Jan 27, 2025
e5aa63f
PB-1169: Index item forecast fields
benschs Jan 27, 2025
18d698b
Refactor models - move file
benschs Jan 27, 2025
e606b08
Refactor models - split into files
benschs Jan 27, 2025
1e2d766
Merge pull request #500 from geoadmin/feat-pb-1169-search-forecast
benschs Jan 27, 2025
d3ef360
PB-1282: document the new authentication method.
adk-swisstopo Jan 20, 2025
23f4c82
Appease GitGuardian.
adk-swisstopo Jan 27, 2025
e2924a9
Remove references to old auth methods.
adk-swisstopo Jan 27, 2025
b900b2f
Merge pull request #504 from geoadmin/feat-PB-1282-authdoc
adk-swisstopo Jan 28, 2025
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
10 changes: 0 additions & 10 deletions app/config/settings_dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@
AWS_SETTINGS['managed']['ACCESS_KEY_ID'] = env("LEGACY_AWS_ACCESS_KEY_ID")
AWS_SETTINGS['managed']['SECRET_ACCESS_KEY'] = env("LEGACY_AWS_SECRET_ACCESS_KEY")

# API Gateway integration PB-1009
AUTHENTICATION_BACKENDS = [
"django.contrib.auth.backends.RemoteUserBackend",
# We keep ModelBackend as fallback until we have moved all users to Cognito.
"django.contrib.auth.backends.ModelBackend",
]
MIDDLEWARE += [
"django.contrib.auth.middleware.AuthenticationMiddleware",
"middleware.apigw.ApiGatewayMiddleware",
]
# By default sessions expire after two weeks.
# Sessions are only useful for user tracking in the admin UI. For security
# reason we should expire these sessions as soon as possible. Given the use
Expand Down
11 changes: 11 additions & 0 deletions app/config/settings_prod.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@
'stac_api.apps.StacApiConfig',
]

# API Authentication options
FEATURE_AUTH_ENABLE_APIGW = env('FEATURE_AUTH_ENABLE_APIGW', bool, default=False)

# Middlewares are executed in order, once for the incoming
# request top-down, once for the outgoing response bottom up
# Note: The prometheus middlewares should always be first and
Expand All @@ -92,13 +95,20 @@
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'middleware.api_gateway_middleware.ApiGatewayMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'middleware.cache_headers.CacheHeadersMiddleware',
'middleware.exception.ExceptionLoggingMiddleware',
'django_prometheus.middleware.PrometheusAfterMiddleware',
]

AUTHENTICATION_BACKENDS = [
"middleware.api_gateway_middleware.ApiGatewayUserBackend",
# We keep ModelBackend as fallback until we have moved all users to Cognito.
"django.contrib.auth.backends.ModelBackend",
]

ROOT_URLCONF = 'config.urls'
API_BASE = 'api'
STAC_BASE = f'{API_BASE}/stac'
Expand Down Expand Up @@ -295,6 +305,7 @@ def get_logging_config():
REST_FRAMEWORK = {
'DEFAULT_RENDERER_CLASSES': ['rest_framework.renderers.JSONRenderer'],
'DEFAULT_AUTHENTICATION_CLASSES': [
'middleware.api_gateway_authentication.ApiGatewayAuthentication',
'rest_framework.authentication.BasicAuthentication',
'rest_framework.authentication.TokenAuthentication',
'rest_framework.authentication.SessionAuthentication',
Expand Down
30 changes: 30 additions & 0 deletions app/middleware/api_gateway.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
REMOTE_USER_HEADER = "HTTP_GEOADMIN_USERNAME"


def validate_username_header(request):
"""Drop the Geoadmin-Username header if it's invalid.

This should be called before making any decision based on the value of the
Geoadmin-Username header.

API Gateway always sends the Geoadmin-Username header regardless of
whether it was able to authenticate the user. If it could not
authenticate the user, the value of the header as seen on the wire is a
single whitespace. An hexdump looks like this:

47 65 6f 61 64 6d 69 6e 5f 75 73 65 72 6e 61 6d 65 3a 20 0d 0a
Geoadmin-Username:...

This doesn't seem possible to reproduce with curl. It is possible to
reproduce with wget. It is unclear whether that technically counts as an
empty value or a whitespace. It is also possible that AWS change their
implementation later to send something slightly different. Regardless,
we already have a separate signal to tell us whether that value is
valid: Geoadmin-Authenticated. So we only consider Geoadmin-Username if
Geoadmin-Authenticated is set to "true".

Based on discussion in https://code.djangoproject.com/ticket/35971
"""
apigw_auth = request.META.get("HTTP_GEOADMIN_AUTHENTICATED", "false").lower() == "true"
if not apigw_auth and REMOTE_USER_HEADER in request.META:
del request.META[REMOTE_USER_HEADER]
26 changes: 26 additions & 0 deletions app/middleware/api_gateway_authentication.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from middleware import api_gateway

from django.conf import settings

from rest_framework.authentication import RemoteUserAuthentication


class ApiGatewayAuthentication(RemoteUserAuthentication):
header = api_gateway.REMOTE_USER_HEADER

def authenticate(self, request):
if not settings.FEATURE_AUTH_ENABLE_APIGW:
return None

api_gateway.validate_username_header(request)
return super().authenticate(request)

def authenticate_header(self, request):
# For this authentication method, users send a "Bearer" token via the
# Authorization header. API Gateway looks up that token in Cognito and
# sets the Geoadmin-Username and Geoadmin-Authenticated headers. In this
# module we only care about the Geoadmin-* headers. But when
# authentication fails with a 401 error we need to hint at the correct
# authentication method from the point of view of the user, which is the
# Authorization/Bearer scheme.
return 'Bearer'
32 changes: 32 additions & 0 deletions app/middleware/api_gateway_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from middleware import api_gateway

from django.conf import settings
from django.contrib.auth.backends import RemoteUserBackend
from django.contrib.auth.middleware import PersistentRemoteUserMiddleware


class ApiGatewayMiddleware(PersistentRemoteUserMiddleware):
"""Persist user authentication based on the API Gateway headers."""
header = api_gateway.REMOTE_USER_HEADER

def process_request(self, request):
if not settings.FEATURE_AUTH_ENABLE_APIGW:
return None

api_gateway.validate_username_header(request)
return super().process_request(request)


class ApiGatewayUserBackend(RemoteUserBackend):
""" This backend is to be used in conjunction with the ``ApiGatewayMiddleware`.

It is probably not needed to provide a custom remote user backend as our custom remote user
middleware will never call authenticate if the feature is not enabled. But better be safe than
sorry.
"""

def authenticate(self, request, remote_user):
if not settings.FEATURE_AUTH_ENABLE_APIGW:
return None

return super().authenticate(request, remote_user)
32 changes: 0 additions & 32 deletions app/middleware/apigw.py

This file was deleted.

25 changes: 23 additions & 2 deletions app/stac_api/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,21 @@ def _validate_href_configured_pattern(url, collection):

def _validate_href_reachability(url, collection):
unreachable_error = _('Provided URL is unreachable')
invalidcontent_error = _('Provided URL returns bad content')
try:
response = requests.head(url, timeout=settings.EXTERNAL_URL_REACHABLE_TIMEOUT)
# We change the way how we check reachability for MCH usecase from
# using HTTP HEAD request to HTTP GET with range
# Once we have other use cases that would require using HTTP HEAD request
# we would have to generalize this and make it configurable
# response = requests.head(url, timeout=settings.EXTERNAL_URL_REACHABLE_TIMEOUT)

# We just wanna check reachability and aren't really interested in the content
headers = {"Range": "bytes=0-2"}
response = requests.get(
url, headers=headers, timeout=settings.EXTERNAL_URL_REACHABLE_TIMEOUT
)

if response.status_code > 400:
if response.status_code >= 400:
logger.warning(
"Attempted external asset upload failed the reachability check",
extra={
Expand All @@ -663,6 +674,16 @@ def _validate_href_reachability(url, collection):
}
)
raise ValidationError(unreachable_error)
if response.headers.get("Content-Length") != "3":
logger.warning(
"Attempted external asset upload failed the content length check",
extra={
'url': url,
'collection': collection, # to have the means to know who this might have been
'response': response,
}
)
raise ValidationError(invalidcontent_error)
except requests.Timeout as exc:
logger.warning(
"Attempted external asset upload resulted in a timeout",
Expand Down
16 changes: 16 additions & 0 deletions app/tests/test_admin_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from io import BytesIO

from django.conf import settings
from django.test import override_settings
from django.urls import reverse

from stac_api.models import Asset
Expand Down Expand Up @@ -47,6 +48,17 @@ def test_login_failure(self):
self.assertEqual(response.status_code, 302)
self.assertEqual("/api/stac/admin/login/?next=/api/stac/admin/", response.url)

def test_login_header_disabled(self):
response = self.client.get(
"/api/stac/admin/",
headers={
"Geoadmin-Username": self.username, "Geoadmin-Authenticated": "true"
}
)
self.assertEqual(response.status_code, 302)
self.assertEqual("/api/stac/admin/login/?next=/api/stac/admin/", response.url)

@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
def test_login_header(self):
response = self.client.get(
"/api/stac/admin/",
Expand All @@ -56,11 +68,13 @@ def test_login_header(self):
)
self.assertEqual(response.status_code, 200, msg="Admin page login with header failed")

@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
def test_login_header_noheader(self):
response = self.client.get("/api/stac/admin/")
self.assertEqual(response.status_code, 302)
self.assertEqual("/api/stac/admin/login/?next=/api/stac/admin/", response.url)

@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
def test_login_header_wronguser(self):
response = self.client.get(
"/api/stac/admin/",
Expand All @@ -71,6 +85,7 @@ def test_login_header_wronguser(self):
self.assertEqual(response.status_code, 302)
self.assertEqual("/api/stac/admin/login/?next=/api/stac/admin/", response.url)

@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
def test_login_header_not_authenticated(self):
self.assertNotIn("sessionid", self.client.cookies)
response = self.client.get(
Expand All @@ -82,6 +97,7 @@ def test_login_header_not_authenticated(self):
self.assertEqual(response.status_code, 302)
self.assertEqual("/api/stac/admin/login/?next=/api/stac/admin/", response.url)

@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
def test_login_header_session(self):
self.assertNotIn("sessionid", self.client.cookies)

Expand Down
51 changes: 51 additions & 0 deletions app/tests/tests_09/test_geoadmin_header_auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import logging

from parameterized import parameterized

from django.contrib.auth import get_user_model
from django.test import Client
from django.test import override_settings

from tests.tests_09.base_test import STAC_BASE_V
from tests.tests_09.base_test import StacBaseTestCase
from tests.tests_09.data_factory import Factory

logger = logging.getLogger(__name__)


@override_settings(FEATURE_AUTH_ENABLE_APIGW=True)
class GeoadminHeadersAuthForPutEndpointTestCase(StacBaseTestCase):
valid_username = "another_test_user"

def setUp(self): # pylint: disable=invalid-name
self.client = Client(enforce_csrf_checks=True)
self.factory = Factory()
self.collection = self.factory.create_collection_sample()
get_user_model().objects.create_superuser(self.valid_username)

@parameterized.expand([
(valid_username, "true", 201),
(None, None, 401),
(valid_username, "false", 401),
("wronguser", "true", 403),
])
def test_collection_upsert_create_with_geoadmin_header_auth(
self, username_header, authenticated_header, expected_response_code
):
sample = self.factory.create_collection_sample(sample='collection-2')

headers = None
if username_header or authenticated_header:
headers = {
"Geoadmin-Username": username_header,
"Geoadmin-Authenticated": authenticated_header,
}
response = self.client.put(
path=f"/{STAC_BASE_V}/collections/{sample['name']}",
data=sample.get_json('put'),
content_type='application/json',
headers=headers,
)
self.assertStatusCode(expected_response_code, response)
if 200 <= expected_response_code < 300:
self.check_stac_collection(sample.json, response.json())
Loading
Loading