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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

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

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b8b9f2
Expire and renew the access token when using OAuth 2.0
GraemeMeyerGT Aug 8, 2024
4ad1a42
Fix typo
GraemeMeyerGT Aug 28, 2024
1b386b9
Getters, setters, validators and default value for oauthTokenRenewalO…
GraemeMeyerGT Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions okta/oauth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import time
from urllib.parse import urlencode, quote
from okta.jwt import JWT
from okta.http_client import HTTPClient
Expand Down Expand Up @@ -37,6 +38,13 @@ async def get_access_token(self):
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want this to be a tough ask, but could this be also present in the config when creating the OktaClient? Have the renewal time also be configurable and default to minutes

Copy link
Author

@GraemeMeyerGT GraemeMeyerGT Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to take on some feedback.

Just to clarify: You want the renewal time - the number of minutes before the token expires that the token is automatically renewed - to be configurable when creating the Okta Client. Are you happy with default of 5 minutes?

So configuring the client might look something like this:

  configuration = {
      'orgUrl': f'https://{current_app.config["OKTA_BASE_URL"]}',
      'authorizationMode': 'PrivateKey',
      'clientId': current_app.config['OKTA_API_CLIENT_ID'],
      'scopes': current_app.config['OKTA_API_SCOPES'],
      'privateKey': current_app.config['OKTA_API_PRIVATE_KEY'],
      'kid': current_app.config['OKTA_API_PRIVATE_KEY_ID'],
      'oauthTokenRenewalOffset': 10 # Example of changing the default of 5 minutes to 10 minutes before expiration
  }
  current_app.okta_client = OktaClient(configuration)

And the token renewal code would look something like:

# Check if access token has expired or will expire soon
current_time = int(time.time())
if self._access_token and hasattr(self, '_access_token_expiry_time'):
    renewal_offset = self._config.get('oauthTokenRenewalOffset', 5) * 60  # Convert minutes to seconds
    if current_time + renewal_offset >= self._access_token_expiry_time:
        self.clear_access_token()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The 5 minute default is okay IMO

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @haggrip, I've made these changes, and I've integrated the changes more completely into the application's convention/structure - initialising the configuration variable with the Client, setting the default value with the ConfigSetter and applying the default value with _apply_default_values etc.

Hopefully this is what you had in mind. In my manual testing, it works well.

self.clear_access_token()

# Return token if already generated
if self._access_token:
return (self._access_token, None)
Expand Down Expand Up @@ -83,6 +91,9 @@ async def get_access_token(self):

# Otherwise set token and return it
self._access_token = parsed_response["access_token"]

# Set token expiry time
self._access_token_expiry_time = int(time.time()) + parsed_response["expires_in"]
return (self._access_token, None)

def clear_access_token(self):
Expand All @@ -92,3 +103,4 @@ def clear_access_token(self):
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