Skip to content

OAuth2 Sessions - #785

Draft
antheng wants to merge 18 commits into
astropy:mainfrom
antheng:ensure_auth
Draft

antheng wants to merge 18 commits into
astropy:mainfrom
antheng:ensure_auth

Conversation

@antheng

@antheng antheng commented Sep 25, 2026

Copy link
Copy Markdown

Introduces the new module pyvo.auth.oauth2 intended to act as a requests-like session for dispatching network calls towards oauth2 protected resources, using requests-oauthlib for many operations.

The new session class pyvo.auth.oauth2.oauth2session.VOAuthSession intends to emulate whats already been established in pyvo.auth.authsession.AuthSession, except for the functions that use the /capabilities endpoint as authentication methods are determined from the authorization metadata rather than those published to the endpoint.

The session storage class pyvo.auth.oauth2.sessionstore.SessionStore which the new session class uses, intends to also emulate the structure in pyvo.auth.authurls, however instead of storing the security methods for a given URL, we instead store another sessions like object which will contain the client credentials required to authenticate to a oauth2 protected resource, or a new one will be created via discovery through the Python interpreter.

At the moment only urn:ietf:params:oauth:grant-type:device_code grant type is supported however the authentication process is structured to eventually add any grant types supported by oauthlib and can be introduced in future PRs.

On Protect Resource Request

On a given _request call to a OAuth2 protected resource:

  1. On request to a given URL, attempt to obtain the session assigned to that URL or request anonymously.
  2. If receiving a 401 and performing authentication is allowed by the request call (default), and the response header contains a WWW-Authenticate with a resource_metadata property, follow the resource metadata to obtain the list of authorization servers. See RFC 9728
  3. Using the first authorization server that responds with a valid set of metadata (at /.well-known/openid-configuration or /.well-known/oauth-authorization-server, discover the valid grant types, and scopes. If the pyvo.auth.oauth2.VOAuthSession object is not configured with their own requests grant types and scopes we will simply use those advertised in the authorization server metadata, otherwise we will end up using an intersection between those configured with the object and those advertised. See RFC 8414
  4. If no explicit client_id and client_secret is passed to the request call, attempt dynamic registration as per RFC 7591 if a registration_endpoint is advertised in the authorization server metadata to obtain a dynamically allocated pair of client credentials. Construct a new requests_oauthlib.OAuth2Session for the requested URL with the new or explicit client id and secret.
  5. Obtain new session token with the first grant type supported by the authorization server and those configured with the pyvo.auth.oauth2.VOAuthSession object.

Helper functions

These functions were primarily to be used as individual steps within authenticate_new_session_from_metadata, however can be used to individually as part of different processes if required:

  • get_dynamic_client_credentials: If an authorization server supports it and with the registration endpoint exists, attempt to obtain a set of dynamic registration credentials. See RFC 7591
  • find_as_metadata: Given a list of Authorization server base URLs, attempt to find their metadata. See RFC 8414
  • token_from_device_code: Brings the user through the device_code grant for a given OAuth2Session, device authorization and token endpoint, using a set of client credentials. (see RFC 8628). This was intended to be merged into requests-oauthlib at Add Support for Device Code Authorization requests/requests-oauthlib#574 however it has not yet been reviewed and thus this serves as a temporary function to use device code authorization until it or an equivalent function is merged into requests-oauthlib

Added dependencies:

  • requests_oauthlib: Creating and storing requests_oauthlib.OAuth2Sessions are the session objects being stored and created, and are the intended stored sessions. Attempts to use other Sesssion-like classes will need to support the same interfaces.
  • keyring: For storing client secrets for the client id's in each OAuth2Session. keyring.cryptfile is a dependency and provided as a default backend, however other backends can be configured

Comment thread setup.cfg Outdated
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.68613% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.80%. Comparing base (4180ae0) to head (48c6490).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
pyvo/auth/oauth2/oauth2session.py 87.62% 25 Missing ⚠️
pyvo/auth/oauth2/sessionstore.py 91.30% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #785      +/-   ##
==========================================
+ Coverage   80.60%   80.80%   +0.20%     
==========================================
  Files          90       93       +3     
  Lines       10366    10640     +274     
==========================================
+ Hits         8355     8598     +243     
- Misses       2011     2042      +31     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@msdemlei

msdemlei commented Sep 25, 2026 via email

Copy link
Copy Markdown
Contributor

@ManonMarchand

Copy link
Copy Markdown
Member

I have no knowledge on OAuth2. If we don't find a reviewer I could try to persuade Gregory to help.

+1: working examples would be nice so that we can try by ourselves

@antheng

antheng commented Sep 26, 2026

Copy link
Copy Markdown
Author

On Fri, Sep 25, 2026 at 12:19:47AM -0700, Manon Marchand wrote: @ManonMarchand commented on this pull request. I'd rather have these dependencies as a new optional bundle
Agreed. Meanwhile, this is a scary amount of code, while I still only have a very superficial understanding of OAuth2. So, I'd be grateful if someone else could review this. But then: Are there live (registered?) services that can be used with that? I'd really like to try this, and perhaps we can even add a bit of executable documentation (though I will give you working examples would likely require working credentials... ouch).

Currently I'm using this fork with at this stage private OAuth2 protected resource however I have added some usage instructions to the docs in the latest commit. If someone were to test this fork on their own service supporting registration then I would be keen to hear how well it works to refine it further.

@andamian

Copy link
Copy Markdown
Contributor

Thank you for the PR. I will have a closer look and try it with services next week. Until then, a few points to consider:

  • This must align with the recommendations of the upcoming AuthVO (https://github.com/ivoa-std/AuthVO). The OAuth2 part seems to be agreed upon on the working draft but was not reviewed by the community and I'm not aware of a the timeline.
  • If we want to have something prior to AuthVO becoming a REC, it should be a prototype feature
  • The tricky part is striking the right balance between usability/convenience and security.

@bsipocz

bsipocz commented Sep 29, 2026

Copy link
Copy Markdown
Member

There are a lot of merge commits of various flavours. Could you please rebase to smooth those out and make the branch history linear? Thanks!

@andamian

andamian commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Here are a few observations to start with. Unlike X509 certificates and cookie, tokens are inherently very tricky to deal because they can be sent to the wrong destination and potentially replayed. It is even harder to used them with a public and generic client like PyVO. I think the implementation needs to be as defensive as the services it interacts with permit. For example, if the RS advertises certain scopes, PyVO needs to use them. Following, it's an assessment of where this PR stands against the OAuth 2.0 Protected Resource Metadata security considerations (RFC 9728 Section §7).

Topic Status Level Finding Suggestion
§7.1 / §3 TLS for metadata   Must requests uses TLS verification by default, but the client never requires https:// for resource_metadata, AS discovery, registration, or token endpoints.
§7.2 Minimal scopes   Should When scope is unset, the client requests every scopes_supported value from the authorization server.
§7.3 / §3.3 resource identity validation   Must Fetched protected-resource metadata is used without checking that metadata.resource equals the requested resource URL (WWW-Authenticate path) or the well-known resource identifier.
§7.3 TLS cert checking   Must Plain requests.get/session.request rely on default certificate verification unless the caller disables it.
§7.4 Audience-restricted tokens   Should No resource / audience indicator (RFC 8707) is sent during device-code or token requests. Dynamic clients make the RS1→RS2 token-reuse attack more relevant.
§7.5 Publishing metadata   Info Server-side concern. Client correctly consumes WWW-Authenticate resource_metadata + oauth-protected-resource.
§7.6 Authorization server binding   Rec authorization_servers is honored, but there is no cross-check against AS protected_resources, no issuer allowlist, and first usable AS wins.
§7.7 SSRF precautions   Should Challenge and AS metadata URLs are fetched without host/IP policy. A malicious RS can point resource_metadata or authorization_servers at internal targets.
§7.8 Phishing UX   Rec Device flow prints verification_uri and user_code with no confirmation that the AS domain matches expectations.
§7.9 Signed metadata   Info signed_metadata is not implemented. Relying on TLS-only (unsigned) metadata is allowed by the RFC.
§7.10 Metadata caching   Rec No client-side metadata cache; every 401 re-fetches. Safe but ignores Cache-Control guidance and can amplify load/SSRF.

I also recommend adding support to RFC 9728 (protected_resources cross-check) if AS supports it.

The implementation should allow users to opt out of some of these restrictions and also warn them when services have low security support. Failures should be as explicit as possible to allow users/service operators to take appropriate actions.

Anthony Heng added 18 commits October 6, 2026 10:07
… to mocks.py, with its instantiation done in conftest.py
… code fetch during device code auth. auth parameter to the pyvo auth2session request method. Add TODO stub to eventually come back to test support for implicit grant type
…tead of returning None on discovery/registration failures. Reuse client credentials from the request `auth` parameter to skip dynamic registration when they are known, and add test for this behaviour.
…th2 extras in doc building and testing. VOAuthSession and SessionStore can now be imported directly from pyvo.auth.oauth2
… the docs. Not possible to fix unless we use intersphinx
@aragilar

aragilar commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@andamian This aims to add the OAuth 2 parts of the AuthVO, so if you see any misalignment between the two, please flag it!

@msdemlei @ManonMarchand The Data Central VO services will support this when the new version is released (as a test of the AuthVO). @antheng has created patches to vollt to also add support. I'll let people know when this is running on a public endpoint, so that people can play with it. This requires three parts (the authorization server, the resource server and the client (in this case pyvo)) all working, and setting up the authorization server and the resource server is not trivial.

Some other comments on the PR:

  • This also includes some changes that we're waiting on requests-oauthlib to merge (Add Support for Device Code Authorization requests/requests-oauthlib#574), once those are upstreamed then they can be dropped here.
  • The idea is this PR (along with future PRs to fully cover all the possible flows) will provide the basis for adding OAuth 2 support to astroquery, with this allowing the user to choose how to do the authentication (and what grant types they can use on their system), and astroquery having autodetection built-in.

@ManonMarchand

Copy link
Copy Markdown
Member

It seems unlikely that the PR would be merged in requests-oauthlib, as the repo has not seen any activity since two years. Do you know if there could be a more active project that could support it?

@bsipocz

bsipocz commented Oct 15, 2026

Copy link
Copy Markdown
Member

Given the commentary in #785 (comment), I would assume we cannot realistically pursue this to be included in the next release (in ~2 weeks time), but aiming for next May instead?

Could/should we instead flip it to be a draft PR while there are active upstream blockers for it?

@aragilar

Copy link
Copy Markdown
Contributor

@bsipocz That would make sense. I asked @antheng to make this PR so that the approach can be validated (to ensure the work is going along the right path), and it makes sense to discuss this at ADASS/IVOA (@antheng won't be there in person, but could potentially zoom in to any meetings).

@bsipocz

bsipocz commented Oct 16, 2026

Copy link
Copy Markdown
Member

OK, that sounds great! I tentatively add this to the next milestone then, no pressure if it won't get wrapped up, but it helps me to see what are blocking the releases.

@bsipocz bsipocz added this to the v1.11.0 milestone Oct 16, 2026
@antheng

antheng commented Oct 16, 2026 •

Copy link
Copy Markdown
Author

Given the number of changes that might need to be made and in turn reviewed again I am happy to make this a draft PR that can be later merged for the May release. This also gives us time to discuss further at IVOA

It seems unlikely that the PR would be merged in requests-oauthlib, as the repo has not seen any activity since two years. Do you know if there could be a more active project that could support it?

@ManonMarchand There are a few other oauth2 libraries we could use that seem to be in a more active state however they seem to lack the device code grant used currently. requests-oauthlib was mostly chosen due to familiarity and reputability. Otherwise authlib seems to be more active whose clients are meant to replace requests-oauthlib but completely lacks any client structures for supporting the device code grant and doesn't include oauthlib as a dependency to help form these requests: it'd need to be written from scratch with their other grant types as reference.

@antheng
antheng marked this pull request as draft October 16, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants