129 - Replace requests-oauthlib with requests-auth

[da1910]

This PR updates the OAuth library to use requests-auth rather than requests-oauthlib.

This has several benefits:
1 - Removes a good chunk of our code around handling callback urls and server setup etc
2 - Has native support for PKCE
3 - Has more robust and native support for refresh tokens

There is one missing feature that had to be implemented by us - providing a token. This is done by manually calling the refresh_token() method with the provided refresh token, then updating the token cache. We should contribute a fix upstream.

The test updates correspond to removing the ability to provide an access token. I was never certain we should support this in the first place, now we only support providing a refresh token. We had to mock most of the auth dance here, but I believe it's now to spec.

[Andy-Grigg]

Changes look good. I suspect we'll end up with a lot less code, which is a very good thing. Especially since it's the code that's been most problematic. Most comments are very minor tidying up things.

[Andy-Grigg]

I tested this locally and I initially hit a problem here: https://github.com/pyansys/openapi-common/pull/139/files#r806838935

Deleting this statement solved the issue, and it connected fine to the test server. Only tested the interactive auth though, haven't tested with a stored/provided token.

[PipKat]

Docstring stuff looks fine, so it looks good to me.

[codecov]

Codecov Report

Merging #139 (672df9c) into main (0ff9f8e) will increase coverage by 2.83%.
The diff coverage is 83.87%.

❗ Current head 672df9c differs from pull request most recent head eefc803. Consider uploading reports for the commit eefc803 to get more accurate results

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   91.11%   93.94%   +2.83%     
==========================================
  Files           8        8              
  Lines         833      793      -40     
==========================================
- Hits          759      745      -14     
+ Misses         74       48      -26     

Impacted Files	Coverage Δ
src/ansys/openapi/common/_util.py	99.40% <ø> (+1.05%)	⬆️
src/ansys/openapi/common/_oidc.py	86.17% <81.48%> (+11.68%)	⬆️
src/ansys/openapi/common/__init__.py	100.00% <100.00%> (+23.07%)	⬆️
src/ansys/openapi/common/_session.py	87.57% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ff9f8e...eefc803. Read the comment docs.

[Andy-Grigg]

This PR now looks good.

However, given how close we are to releasing 1.0.0, we're going to hold off on merging this PR. We will merge #138, and then assuming tests are successful, we will release with this fix. We will then merge this PR for a subsequent release.

[Andy-Grigg]

Marking as a draft so we don't merge this before we are ready.

[Andy-Grigg]

It looks like we need to persist the token in a cache on disk. The default behavior is to persist the token in memory, but adding the line:

OAuth2.token_cache = JsonTokenFileCache('cache.json')

creates a cache on disk contain the received token. This is then used to re-authenticate, and doesn't result in a browser window popping up.

Using 'with_token()` also works, I extracted the refresh token from the cache and passed it in, and again the session was authenticated without interacting with the web browser.

Keeping this stuff on disk is probably fine, but I'd think it should be possible to write an additional cache that uses the credential store since these tokens should probably be treated as sensitive. Relevant code here https://github.com/Colin-b/requests_auth/blob/develop/requests_auth/oauth2_tokens.py

[da1910]

It looks like we need to persist the token in a cache on disk. The default behavior is to persist the token in memory, but adding the line:

OAuth2.token_cache = JsonTokenFileCache('cache.json')

creates a cache on disk contain the received token. This is then used to re-authenticate, and doesn't result in a browser window popping up.

Using 'with_token()` also works, I extracted the refresh token from the cache and passed it in, and again the session was authenticated without interacting with the web browser.

Keeping this stuff on disk is probably fine, but I'd think it should be possible to write an additional cache that uses the credential store since these tokens should probably be treated as sensitive. Relevant code here https://github.com/Colin-b/requests_auth/blob/develop/requests_auth/oauth2_tokens.py

I don't think we want to be persisting tokens on disk, at least not unless we've explicitly asked the user for permission. Is there a lifetime on the in-memory token cache? I would expect it to persist the refresh token as long as the same python process continues.

I'll have an investigate and see if I can work out why it's evicting valid refresh tokens from the cache, as I recall refresh tokens are a new-ish feature, so there might well be a bug

[Andy-Grigg]

It looks like we need to persist the token in a cache on disk. The default behavior is to persist the token in memory, but adding the line:

OAuth2.token_cache = JsonTokenFileCache('cache.json')

creates a cache on disk contain the received token. This is then used to re-authenticate, and doesn't result in a browser window popping up.
Using 'with_token()` also works, I extracted the refresh token from the cache and passed it in, and again the session was authenticated without interacting with the web browser.
Keeping this stuff on disk is probably fine, but I'd think it should be possible to write an additional cache that uses the credential store since these tokens should probably be treated as sensitive. Relevant code here https://github.com/Colin-b/requests_auth/blob/develop/requests_auth/oauth2_tokens.py

I don't think we want to be persisting tokens on disk, at least not unless we've explicitly asked the user for permission. Is there a lifetime on the in-memory token cache? I would expect it to persist the refresh token as long as the same python process continues.

I'll have an investigate and see if I can work out why it's evicting valid refresh tokens from the cache, as I recall refresh tokens are a new-ish feature, so there might well be a bug

I assumed the test here was to validate that the refresh functionality worked across multiple script executions in separate processes. Since the in-memory cache is lost at the end of each process, filesystem persistence is the only option. If that's not the case, then I imagine it works as-is.

[da1910]

Refreshing looks OK. I think this is worth merging now, but I will raise a ticket about a certificate store issue. We will probably need to make a temporary store on disk somewhere.