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

OAuth 2.0 Implicit Authentication querystring parameters missing #2506

Open
mpdunlop opened this issue Aug 9, 2024 · 4 comments · May be fixed by #2510
Open

OAuth 2.0 Implicit Authentication querystring parameters missing #2506

mpdunlop opened this issue Aug 9, 2024 · 4 comments · May be fixed by #2510

Comments

@mpdunlop
Copy link

mpdunlop commented Aug 9, 2024

Summary

My app primarily supports the OAuth 2.0 Authorization Code with PKCE Flow but I am also implementing OAuth 2.0 Implicit flow in order to provide compatability with OPDS Clients. The OPDS Authentication document returned when accessing my Bookshelf API is:

{
    "id": "https://localhost:7063/auth.json",
    "title": "Bookshelf",
    "authentication": [
        {
            "type": "http://opds-spec.org/auth/oauth/implicit",
            "links": [
                {
                    "href": "https://localhost:7136/connect/authorize",
                    "rel": "authenticate"
                }
            ]
        }
    ]
}

And the response header has the Content-Type application/opds-authentication+json.

This is very similar to the example provided in the Authentication for OPDS 1.0, minus the help links.

When accessing an OPDS feed source that requires authentication, Thorium Reader receives this OPDS authentication document. Stepping through source with the debugger, I can see that Thorium Reader correctly detects the Implicit OAuth flow as supported. It then opens the browser but does not append the required query string values client_id or response_type. I expect OPDS Clients to use the required OPDS OAuth Client Id http://opds-spec.org/auth/client when authenticating and the Implicit Flow should request a token.

When I embed these values into the URL in the opds authentication document, Thorium Reader opens the link successfully. I don't think this is the correct behavior:

  • The example in the documentation does not have these query string parameters
  • Including these query string parameters in the URL will force all OPDS Clients to use the same client_id, even though the specification states that "Specific OPDS clients may also be registered by each Catalog, in order to be further identified". Hard coding the client_id in the Authentication Document URL will prevent this.

Versions Tested

I have tested this using current stable release v3.0.0 and current alpha release v3.1.0-alpha.1.

Actual Behaviour

Thorium opens the URL provided in the authentication document exactly as provided and does not append the required query string parameters: response_type or client_id.

Expected Behaviour

Thorium opens a browser window to the link provided in the authentication document, adding the parameters response_type and client_id at a minimum, but ideally with a generated parameter state to prevent CSRF attacks on the login page.

Example of expected URL to be opened: https://localhost:7136/connect/authorize?client_id=http%3A%2F%2Fopds-spec.org%2Fauth%2Fclient&response_type=token&state=41ysjf5ts5vw1lt90sbgk4e0g7nd89iw

@mpdunlop
Copy link
Author

I'm happy to create a PR for this if someone can confirm that this behavior needs updating 🙂

@panaC panaC linked a pull request Aug 16, 2024 that will close this issue
@panaC
Copy link
Member

panaC commented Aug 16, 2024

Hello Matthew, thanks for raising an issue.

I'm worked on it and I publish a fix with a PR, feel free to complete or create a new PR if needed.

As i understand OAuth2 implicit grant need some fixes :

@mpdunlop
Copy link
Author

Thanks for your speedy work on this, it's much appreciated!

I have made some updates to your PR that I think address some of my comments: #2511. I look forward to hearing your thoughts on them.

@danielweck
Copy link
Member

see opds-community/drafts#82

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 a pull request may close this issue.

3 participants