Improve "early adopter check" error messages

see openEOPlatform/architecture-docs#105

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Access to authenticated endpoints requires the "early adopter" role now (through EGI Check-in `eduPersonEntitlement`) (EP-3969)
 - Work with aggregator-specific OIDC provider settings (e.g. dedicated default clients) (EP-4046, [#7](https://github.com/Open-EO/openeo-aggregator/issues/7))
 - Disable `egi-dev` OIDC provider (EP-4046, [#7](https://github.com/Open-EO/openeo-aggregator/issues/7))
+- Improve "early adopter check" error messages ([openEOPlatform/architecture-docs#105](https://github.com/openEOPlatform/architecture-docs/issues/105))
 
 
 ### Fixed

setup.py

@@ -22,7 +22,7 @@
     install_requires=[
         "requests",
         "openeo>=0.7.1a1.*",
-        "openeo_driver>=0.14.2a1.*",
+        "openeo_driver>=0.14.3a1.*",
         "flask~=2.0",
         "gunicorn~=20.0",
     ],

src/openeo_aggregator/backend.py

@@ -14,7 +14,7 @@
 from openeo_aggregator.connection import MultiBackendConnection, BackendConnection, streaming_flask_response
 from openeo_aggregator.egi import is_early_adopter
 from openeo_aggregator.errors import BackendLookupFailureException
-from openeo_aggregator.utils import TtlCache, MultiDictGetter
+from openeo_aggregator.utils import TtlCache, MultiDictGetter, subdict
 from openeo_driver.ProcessGraphDeserializer import SimpleProcessing
 from openeo_driver.backend import OpenEoBackendImplementation, AbstractCollectionCatalog, LoadParameters, Processing, \
     OidcProvider, BatchJobs, BatchJobMetadata
@@ -491,7 +491,7 @@ def __init__(self, backends: MultiBackendConnection, config: AggregatorConfig):
             user_defined_processes=None,
         )
         self._cache = TtlCache(default_ttl=CACHE_TTL_DEFAULT)
-        self._auth_entitlement_check = config.auth_entitlement_check
+        self._auth_entitlement_check: Union[bool, dict] = config.auth_entitlement_check
         self._configured_oidc_providers: List[OidcProvider] = config.configured_oidc_providers
 
     def oidc_providers(self) -> List[OidcProvider]:
@@ -528,16 +528,28 @@ def merge(formats: dict, to_add: dict):
 
     def user_access_validation(self, user: User, request: flask.Request) -> User:
         if self._auth_entitlement_check:
-            if user.internal_auth_data["authentication_method"] != "OIDC":
-                raise PermissionsInsufficientException("OIDC auth required")
+            base_error_message = "Not a valid openEO Platform user"
+            int_data = user.internal_auth_data
+            issuer_whitelist = self._auth_entitlement_check.get("oidc_issuer_whitelist", {"https://aai.egi.eu/oidc"})
+            if not (
+                    int_data["authentication_method"] == "OIDC"
+                    and int_data["oidc_issuer"].rstrip("/").lower() in issuer_whitelist
+            ):
+                message = f"{base_error_message}: OIDC authentication with EGI Check-in is required."
+                debug_info = subdict(int_data, keys=["authentication_method", "oidc_issuer"])
+                _log.warning(f"{message} debug_info:{debug_info} whitelist:{issuer_whitelist}")
+                raise PermissionsInsufficientException(message)
             try:
                 eduperson_entitlements = user.info["oidc_userinfo"]["eduperson_entitlement"]
             except KeyError as e:
-                _log.warning(f"No eduperson_entitlement data ({e!r})")
-                raise PermissionsInsufficientException("No eduperson_entitlement data")
+                message = f"{base_error_message}: missing entitlement data."
+                # Note: just log userinfo keys to avoid leaking sensitive user data.
+                _log.warning(f"{message} {e!r} {user.info.keys()} {user.info.get('oidc_userinfo', {}).keys()}")
+                raise PermissionsInsufficientException(message)
             if not any(is_early_adopter(e) for e in eduperson_entitlements):
-                _log.warning(f"User {user.user_id} has no early adopter role: {eduperson_entitlements}")
-                raise PermissionsInsufficientException("No early adopter role")
+                message = f"{base_error_message}: no early adopter role."
+                _log.warning(f"{message} user:{user.user_id} entitlements:{eduperson_entitlements}")
+                raise PermissionsInsufficientException(message)
 
             # TODO: list multiple roles/levels? Better "status" signaling?
             user.info["roles"] = ["EarlyAdopter"]

src/openeo_aggregator/config.py

@@ -34,7 +34,7 @@ class AggregatorConfig(dict):
 
     # TODO: add validation/normalization to make sure we have a real list of OidcProvider objects?
     configured_oidc_providers: List[OidcProvider] = dict_item(default=[])
-    auth_entitlement_check = dict_item(default=True)
+    auth_entitlement_check: Union[bool, dict] = dict_item()
 
     @classmethod
     def from_json(cls, data: str):
@@ -67,7 +67,7 @@ def from_json_file(cls, path: Union[str, Path]):
         "eodc": "https://openeo.eodc.eu/v1.0/",
         # "eodcdev": "https://openeo-dev.eodc.eu/v1.0/",
     },
-    auth_entitlement_check=True,
+    auth_entitlement_check={"oidc_issuer_whitelist": {"https://aai.egi.eu/oidc"}},
     configured_oidc_providers=[
         OidcProvider(
             id="egi",

src/openeo_aggregator/utils.py

@@ -104,3 +104,11 @@ def first(self, key, default=None):
     def select(self, key: str) -> 'MultiDictGetter':
         """Create new getter, one step deeper in the dictionary hierarchy."""
         return MultiDictGetter(d for d in self.get(key=key) if isinstance(d, dict))
+
+
+def subdict(d: dict, *args, keys: list = None, default=None) -> dict:
+    """Extract dict with only selected keys from given dict"""
+    keys = set(keys or []) | set(args)
+    # TODO: way to not provide default and raise KeyError on missing keys
+    # TODO: move to openeo-python-driver?
+    return {k: d.get(k, default) for k in keys}

tests/conftest.py

@@ -81,7 +81,7 @@ def api100(flask_app: flask.Flask) -> ApiTester:
 
 @pytest.fixture
 def api100_with_entitlement_check(config: AggregatorConfig) -> ApiTester:
-    config.auth_entitlement_check = True
+    config.auth_entitlement_check = {"oidc_issuer_whitelist": {"https://egi.test", "https://egi.test/oidc"}}
     return get_api100(get_flask_app(config))
 
 

tests/test_utils.py

@@ -1,6 +1,6 @@
 import pytest
 
-from openeo_aggregator.utils import TtlCache, CacheMissException, MultiDictGetter
+from openeo_aggregator.utils import TtlCache, CacheMissException, MultiDictGetter, subdict
 
 
 class FakeClock:
@@ -129,3 +129,15 @@ def test_select(self):
         assert list(getter.select("b").select("ba").get("x")) == []
         assert list(getter.select("b").select("bb").get("bba")) == [5]
         assert list(getter.select("x").select("y").select("z").get("z")) == []
+
+
+def test_subdict():
+    d = {"foo": "bar", "meh": 3}
+    assert subdict(d) == {}
+    assert subdict(d, "foo") == {"foo": "bar"}
+    assert subdict(d, "foo", "meh") == {"foo": "bar", "meh": 3}
+    assert subdict(d, "foo", "bar") == {"foo": "bar", "bar": None}
+    assert subdict(d, keys=["foo"]) == {"foo": "bar"}
+    assert subdict(d, keys=["foo", "meh"]) == {"foo": "bar", "meh": 3}
+    assert subdict(d, keys=["foo", "bar"]) == {"foo": "bar", "bar": None}
+    assert subdict(d, "foo", keys=["meh", "bar"]) == {"foo": "bar", "meh": 3, "bar": None}

tests/test_views.py

@@ -101,7 +101,10 @@ class TestAuthEntitlementCheck:
     def test_basic_auth(self, api100_with_entitlement_check):
         api100_with_entitlement_check.set_auth_bearer_token(token=TEST_USER_BEARER_TOKEN)
         res = api100_with_entitlement_check.get("/me")
-        res.assert_error(403, "PermissionsInsufficient", message="OIDC auth required")
+        res.assert_error(
+            403, "PermissionsInsufficient",
+            message="Not a valid openEO Platform user: OIDC authentication with EGI Check-in is required."
+        )
 
     def test_oidc_no_entitlement_data(self, api100_with_entitlement_check, requests_mock):
         def get_userinfo(request: requests.Request, context):
@@ -115,7 +118,10 @@ def get_userinfo(request: requests.Request, context):
         api100_with_entitlement_check.set_auth_bearer_token(token="oidc/egi/funiculifunicula")
 
         res = api100_with_entitlement_check.get("/me")
-        res.assert_error(403, "PermissionsInsufficient", message="No eduperson_entitlement data")
+        res.assert_error(
+            403, "PermissionsInsufficient",
+            message="Not a valid openEO Platform user: missing entitlement data."
+        )
 
     def test_oidc_no_early_adopter(self, api100_with_entitlement_check, requests_mock):
         def get_userinfo(request: requests.Request, context):
@@ -135,7 +141,10 @@ def get_userinfo(request: requests.Request, context):
         api100_with_entitlement_check.set_auth_bearer_token(token="oidc/egi/funiculifunicula")
 
         res = api100_with_entitlement_check.get("/me")
-        res.assert_error(403, "PermissionsInsufficient", message="No early adopter role")
+        res.assert_error(
+            403, "PermissionsInsufficient",
+            message="Not a valid openEO Platform user: no early adopter role."
+        )
 
     def test_oidc_early_adopter(self, api100_with_entitlement_check, requests_mock):
         def get_userinfo(request: requests.Request, context):