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.

Sign up for GitHub

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

Update OPDS2 token auth fulfillment to use opaque patron identifier (PP-416) #1374

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Update OPDS2 token auth fulfillment to use opaque patron identifier (PP-416) #1374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d89524
Update OPDS2 token auth to work with any authentication mechanism.
jonathangreen Sep 12, 2023
f8e76e6
Remove debug log.
jonathangreen Sep 12, 2023
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
26 changes: 13 additions & 13 deletions api/opds2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from api.circulation import CirculationFulfillmentPostProcessor, FulfillmentInfo
from api.circulation_exceptions import CannotFulfill
from core.lane import Facets
from core.model import ConfigurationSetting, ExternalIntegration
from core.model import ConfigurationSetting, DataSource, ExternalIntegration
from core.model.edition import Edition
from core.model.identifier import Identifier
from core.model.licensing import LicensePoolDeliveryMechanism
Expand Down Expand Up @@ -94,6 +94,10 @@ class TokenAuthenticationFulfillmentProcessor(CirculationFulfillmentPostProcesso
def __init__(self, collection) -> None:
pass

@classmethod
def logger(cls) -> logging.Logger:
return logging.getLogger(f"{cls.__module__}.{cls.__name__}")

def fulfill(
self,
patron: Patron,
Expand All @@ -120,7 +124,9 @@ def fulfill(
if not token_auth or token_auth.value is None:
return fulfillment

token = self.get_authentication_token(patron, token_auth.value)
token = self.get_authentication_token(
patron, licensepool.data_source, token_auth.value
)
if isinstance(token, ProblemDetail):
raise CannotFulfill()

Expand All @@ -130,24 +136,18 @@ def fulfill(

@classmethod
def get_authentication_token(
cls, patron: Patron, token_auth_url: str
cls, patron: Patron, datasource: DataSource, token_auth_url: str
) -> ProblemDetail | str:
"""Get the authentication token for a patron"""
log = logging.getLogger("OPDS2API")

patron_id = patron.username if patron.username else patron.external_identifier
if patron_id is None:
log.error(
f"Could not authenticate the patron({patron.authorization_identifier}): "
f"both username and external_identifier are None."
)
return INVALID_CREDENTIALS
log = cls.logger()

patron_id = patron.identifier_to_remote_service(datasource)
url = URITemplate(token_auth_url).expand(patron_id=patron_id)
response = HTTP.get_with_timeout(url)
if response.status_code != 200:
log.error(
f"Could not authenticate the patron({patron_id}): {str(response.content)}"
f"Could not authenticate the patron (authorization identifier: '{patron.authorization_identifier}' "
f"external identifier: '{patron_id}'): {str(response.content)}"
)
return INVALID_CREDENTIALS

Expand Down
15 changes: 9 additions & 6 deletions tests/api/test_opds2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ class TestTokenAuthenticationFulfillmentProcessor:
@patch("api.opds2.HTTP")
def test_fulfill(self, mock_http, db: DatabaseTransactionFixture):
patron = db.patron()
patron.username = "username"
collection: Collection = db.collection(
protocol=ExternalIntegration.OPDS2_IMPORT
)
Expand Down Expand Up @@ -207,10 +206,14 @@ def test_fulfill(self, mock_http, db: DatabaseTransactionFixture):
processor = TokenAuthenticationFulfillmentProcessor(collection)
ff_info = processor.fulfill(patron, "", work.license_pools[0], None, ff_info)

patron_id = patron.identifier_to_remote_service(
work.license_pools[0].data_source
)

assert mock_http.get_with_timeout.call_count == 1
assert (
mock_http.get_with_timeout.call_args[0][0]
== "http://example.org/token?userName=username"
== f"http://example.org/token?userName={patron_id}"
)

assert (
Expand Down Expand Up @@ -265,9 +268,9 @@ def test_get_authentication_token(self, mock_http, db: DatabaseTransactionFixtur
resp.raw = io.BytesIO(b"plaintext-auth-token")
mock_http.get_with_timeout.return_value = resp
patron = db.patron()
patron.username = "test"
datasource = DataSource.lookup(db.session, "test", autocreate=True)
token = TokenAuthenticationFulfillmentProcessor.get_authentication_token(
patron, "http://example.org/token"
patron, datasource, "http://example.org/token"
)

assert token == "plaintext-auth-token"
Expand All @@ -280,9 +283,9 @@ def test_get_authentication_token_errors(
resp = Response()
resp.status_code = 400
mock_http.get_with_timeout.return_value = resp

datasource = DataSource.lookup(db.session, "test", autocreate=True)
token = TokenAuthenticationFulfillmentProcessor.get_authentication_token(
db.patron(), "http://example.org/token"
db.patron(), datasource, "http://example.org/token"
)

assert token == INVALID_CREDENTIALS
Expand Down