okta / okta-sdk-python

Apache License 2.0
240 stars 142 forks source link

Expire and renew the access token when using OAuth 2.0 #415

Open GraemeMeyerGT opened 3 months ago

GraemeMeyerGT commented 3 months ago

This PR fixes https://github.com/okta/okta-sdk-python/issues/363 (Access token expiry not handled when using OAuth 2.0) by adding logic to expire and renew the access token when using OAuth 2.0 to authenticate with the Okta API. This issue and the fix was also discussed in PR #402 (Feature: Autorenew OAuth tokens]), but I believe this fix is simpler and superior, as it does not require importing any new libraries (other than time) and is more in line with the existing codebase.

I have reviewed the CLA and believe that this falls under the remit of an Obvious Fix and doesn't not require singing the CLA. I would be happy to do so however if the Okta team feels it is necessary.

I have also followed the steps CONTRIBUTING guide when submitting this PR.

I don't have the ability to run the pytest suite on my work PC, but I've run this code manually by modifying the local okta-sdk-python and it works. I've also implemented a hotfix library that imports the Okta code, replaces the OAuth.py functions code with my code and runs it, and it works well.

bryanapellanes-okta commented 3 months ago

@GraemeMeyerGT thanks for your contribution! Please see my comment here: https://github.com/okta/okta-sdk-python/pull/402#issuecomment-2276114071. We hope to release a preview of a new version of the SDK in the coming weeks (a firm date is not yet determined). We would be grateful to have your input once it is ready and perhaps we can work together to address this concern in the next version of the SDK.

Further, we welcome additional comment from @haggrip on this PR to continue to engage our Python developers so we can work together to provide the best SDK experience for all. We'll prioritize internally to determine how best to move forward with this and other outstanding PRs. Thanks again!

GraemeMeyerGT commented 3 months ago

Understood, thanks Bryan. I look forward to trying out the new version soon!

For anyone else dealing with this issue right now, you can use the same work around as me. Essentially I am patching the loaded methods from the Okta library with my fixed versions.

First, create a new file called okta_fix.py like so:

okta_fix.py ```python import time from urllib.parse import urlencode, quote from okta.jwt import JWT from okta.http_client import HTTPClient from okta.oauth import OAuth async def patched_get_access_token(self): """ Retrieves or generates the OAuth access token for the Okta Client Returns: str, Exception: Tuple of the access token, error that was raised (if any) """ # Check if access token has expired or will expire in the next 5 minutes current_time = int(time.time()) if self._access_token and hasattr(self, '_access_token_expiry_time'): if current_time + 300 >= self._access_token_expiry_time: self.clear_access_token() # Return token if already generated if self._access_token: return (self._access_token, None) # Otherwise create new one # Get JWT and create parameters for new Oauth token jwt = self.get_JWT() parameters = { 'grant_type': 'client_credentials', 'scope': ' '.join(self._config["client"]["scopes"]), 'client_assertion_type': 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', 'client_assertion': jwt } encoded_parameters = urlencode(parameters, quote_via=quote) org_url = self._config["client"]["orgUrl"] url = f"{org_url}{OAuth.OAUTH_ENDPOINT}?" + \ encoded_parameters # Craft request oauth_req, err = await self._request_executor.create_request( "POST", url, None, { 'Accept': "application/json", 'Content-Type': 'application/x-www-form-urlencoded' }, oauth=True) # TODO Make max 1 retry # Shoot request if err: return (None, err) _, res_details, res_json, err = \ await self._request_executor.fire_request(oauth_req) # Return HTTP Client error if raised if err: return (None, err) # Check response body for error message parsed_response, err = HTTPClient.check_response_for_error( url, res_details, res_json) # Return specific error if found in response if err: return (None, err) # Otherwise set token and return it self._access_token = parsed_response["access_token"] # Set token expiry time as epoch time self._access_token_expiry_time = int(time.time()) + parsed_response["expires_in"] return (self._access_token, None) def patched_clear_access_token(self): """ Clear currently used OAuth access token, probably expired """ self._access_token = None self._request_executor._cache.delete("OKTA_ACCESS_TOKEN") self._request_executor._default_headers.pop("Authorization", None) self._access_token_expiry_time = None # Apply the patches OAuth.get_access_token = patched_get_access_token OAuth.clear_access_token = patched_clear_access_token ```

This file is pretty much just the two modified methods, and then at the end they are patched on-top-of the Okta SDK's methods:

# Apply the patches
OAuth.get_access_token = patched_get_access_token
OAuth.clear_access_token = patched_clear_access_token

Now I just import okta_fix.py before any use of the Okta library, and it all works well. E.g. I am writing a Flask app, so I import it right at the top of my app's __init__.py file:

import okta_fix
from flask import Flask
from config import Config
from .extensions import login_manager

def create_app(config_class=Config):
    app = Flask(__name__)
    app.config.from_object(config_class)
...
bryanapellanes-okta commented 3 months ago

@GraemeMeyerGT I'm really happy to see the collaboration here! May I request that you add a unit test that tests your change?

GraemeMeyerGT commented 3 months ago

Thanks @bryanapellanes-okta. Unfortunately I don't have any experience with Python tests. I've spent a couple of hours looking at it with an LLM, but TBH I'm not understanding most of what it's spitting out, or any of the existing tests. I'll give myself a crash course in Python testing over the next few days, but I'm not confident yet that I'll be able to come up to the level of this project any time soon.

If you have any pointers on the exact type of test(s) you'd like to see and which file you'd like to see them in, that would be helpful (as would a pointer to one or two of the existing tests that you think would be a good reference).