-
Notifications
You must be signed in to change notification settings - Fork 146
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
Feature: Autorenew OAuth tokens #402
base: master
Are you sure you want to change the base?
Conversation
@jccaldw1 this seems simple enough, but I am not too familiar with OAuth. Seems like we are generating a new token here, but shouldn't we be able to generate a new token from a refresh token? |
@haggrip would the Okta team give some consideration to prioritising this feature? It's a very curious gap in the SDK. Server to Server applications (those which are likely to use the SDK) are obviously likely to be long-lived, and OAuth is the recommended / best practice authentication mechanism as I understand it. The Org Authorization Server fixes the access token lifetime at one hour, and as far as I can tell, the SDK exposes neither the token expiry time, nor any mechanism for deleting/refreshing the token. I've been rummaging through the code and as far as I can tell the SDK actually doesn't even have an internal mechanism to use an OAuth refresh token. I'm not saying @jccaldw1's solution is the best solve for this - importing another library seems excessive - but this is a big gap and worth exploring solutions. Some simple solutions:
Would Okta be open to a PR for this, based on one of these approaches, if someone were to work on it? I could probably manage the quick hack, but full refresh behaviour is probably beyond my capability. There's a related issue here: #363 |
I hadn't realised before, but the import time
...
TRUNCATED
...
async def 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()
self._access_token_expiry_time = None
# Return token if already generated
if self._access_token:
return (self._access_token, None)
...
TRUNCATED
...
# 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) I've made this modification locally and seems to be working. If it holds up for a day or two I'll maybe YOLO it in as a PR. A cleaner move would be to clear the token expiry time ( |
I've submitted my own PR (#415) to fix this issue, which I think is similar. If anyone from Okta would be able to give me in idea whether this has a hope of being accepted, that would be much appreciated. |
@haggrip @GraemeMeyerGT I'd like to thank you both for your active engagement! Additionally, I'd like to apologize for the delayed response as we juggle priorities. I think it is worth noting that we are actively working on a new version of the SDK using new tooling and a later version of the OpenApiSpec. We will review internally to determine best course forward to address this and other outstanding needs. Again thanks for your engagement, and thanks for using Okta! |
Currently, when a Client is initialized with OAuth configuration, as below:
An expired OAuth JWT token can be used if the client lives for long enough. This PR adds an additional check to see if the stored OAuth token is expired - if it is, we go through the normal token generation process and save the resultant token in
self._access_token
in the OAuth object. This will ensure that OAuth access tokens are never expired.