okta / okta-sdk-python

Apache License 2.0
240 stars 142 forks source link

Feature: Autorenew OAuth tokens #402

Open jccaldw1 opened 7 months ago

jccaldw1 commented 7 months ago

Currently, when a Client is initialized with OAuth configuration, as below:

config = {
    'orgUrl': 'https://{yourOktaDomain}',
    'authorizationMode': 'PrivateKey',
    'clientId': '{yourClientId}',
    'scopes': ['okta.users.manage'],
    'privateKey': 'YOUR_PRIVATE_JWK', # this parameter should be type of str
    'kid': 'YOUR_PRIVATE_KEY_ID' # if a key ID needs to be provided, it can be provided here or part of the privateKey under "kid"
}

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.

haggrip commented 4 months ago

@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?

GraemeMeyerGT commented 3 months ago

@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: https://github.com/okta/okta-sdk-python/issues/363

GraemeMeyerGT commented 3 months ago

I hadn't realised before, but the parsed_response from Okta for OAuth tokens actually includes the expiry time (as parsed_response["expires_in"]), so TBH I think this can probably be solved as simply as modifying the get_access_token() method as follows:

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 (self._access_token_expiry_time = None) in the clear_access_token() method. If I submit the PR, I'll do it that way.

GraemeMeyerGT commented 3 months ago

I've submitted my own PR (https://github.com/okta/okta-sdk-python/pull/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.

bryanapellanes-okta commented 3 months ago

@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!