diff --git a/lms/security.py b/lms/security.py index 6af72fb773..93dce390e3 100644 --- a/lms/security.py +++ b/lms/security.py @@ -104,13 +104,16 @@ def get_policy(request: Request): # noqa: PLR0911 return LMSGoogleSecurityPolicy() if path.startswith(("/api/dashboard")): - # For certain routes we only use the google policy in case it resulted - # non empty identity. - # This is useful for routes that can be used by admin pages users on top of - # other type of access - policy = LMSGoogleSecurityPolicy() - if policy.identity(request): - return policy + # For the dashboard API we prefer, in this order: + # - HeadersBearerTokenLTIUserPolicy() + # For requests authorized via an API token, this applies to LMS users + # + # - LMSGoogleSecurityPolicy() + # If the header token policy didn't succeed we try the google auth cookie, for staff users + policies = [HeadersBearerTokenLTIUserPolicy(), LMSGoogleSecurityPolicy()] + for policy in policies: + if policy.identity(request): # type: ignore + return policy if path in {"/lti_launches", "/content_item_selection"}: # Actual LTI backed authentication diff --git a/tests/unit/lms/security_test.py b/tests/unit/lms/security_test.py index 0bad0f4446..995be603bf 100644 --- a/tests/unit/lms/security_test.py +++ b/tests/unit/lms/security_test.py @@ -429,17 +429,32 @@ def test_forgets(self, policy, pyramid_request, get_policy): assert user_id == get_policy.return_value.forget.return_value @pytest.mark.parametrize( - "path", ["/api/dashboard/assignments/10", "/dashboard/orgs/ORGID"] + "header_value,google_value,expected_policy", + [ + (sentinel.ok, None, HeadersBearerTokenLTIUserPolicy), + (None, sentinel.ok, LMSGoogleSecurityPolicy), + (sentinel.ok, sentinel.ok, HeadersBearerTokenLTIUserPolicy), + (None, None, HeadersBearerTokenLTIUserPolicy), + ], ) - def test_get_policy_google_when_available( - self, pyramid_request, path, LMSGoogleSecurityPolicy + def test_api_dashboard_policy( + self, + pyramid_request, + header_value, + google_value, + expected_policy, + HeadersBearerTokenLTIUserPolicy, + LMSGoogleSecurityPolicy, ): - pyramid_request.path = path - LMSGoogleSecurityPolicy.return_value.identity.return_value = sentinel.identity + pyramid_request.path = "/api/dashboard/assignments/10" + HeadersBearerTokenLTIUserPolicy.return_value.identity.return_value = ( + header_value + ) + LMSGoogleSecurityPolicy.return_value.identity.return_value = google_value policy = SecurityPolicy.get_policy(pyramid_request) - assert policy == LMSGoogleSecurityPolicy.return_value + assert policy.__class__ == expected_policy @pytest.mark.parametrize( "path", @@ -533,6 +548,10 @@ def LTIUserSecurityPolicy(self, patch): def LMSGoogleSecurityPolicy(self, patch): return patch("lms.security.LMSGoogleSecurityPolicy") + @pytest.fixture(autouse=True) + def HeadersBearerTokenLTIUserPolicy(self, patch): + return patch("lms.security.HeadersBearerTokenLTIUserPolicy") + @pytest.fixture(autouse=True) def EmailPreferencesSecurityPolicy(self, patch): return patch("lms.security.EmailPreferencesSecurityPolicy")