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

🐛 Fix OPDS path detection regression #350

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

aaronleopold
Copy link
Collaborator

@aaronleopold aaronleopold commented Jun 5, 2024

Relates to #346

The recent change to split the main OPDS router into sub-routers for each supported version caused some issues in the auth middleware. Specifically, the /opds prefix was missing which caused the conditional check which allows basic auth for OPDS-specific routes to fail.

Accessing the request parts uri in an extension seems to be scoped to the current router. This means when the auth middleware attempted to extract the uri, it was now relative to the new sub-router (which in this case, would be /v1.2/*). To correct this, I just used the OriginalUri extractor.

I also used this as an opportunity to fix some of the session handling logic in the OPDS auth flow

The recent change to split the main OPDS router into sub-routers for each supported version caused some issues in the auth middleware. Specifically, the `/opds` prefix was missing which caused the conditional check which allows basic auth for OPDS-specific routes. This is expected behavior with the changes, since accessing the request parts `uri` in an extension is scoped to the router, which after nesting multiple routers under the main opds one means the top-level is v1.2
@aaronleopold aaronleopold marked this pull request as ready for review June 5, 2024 00:38
@aaronleopold aaronleopold merged commit 52092c6 into develop Jun 5, 2024
3 checks passed
@aaronleopold aaronleopold deleted the al/fix-opds-regression branch June 5, 2024 01:24
@aaronleopold aaronleopold mentioned this pull request Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant