Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

OpenID KEY caching #73

Open
abonne01 opened this issue Mar 2, 2021 · 7 comments
Open

OpenID KEY caching #73

abonne01 opened this issue Mar 2, 2021 · 7 comments

Comments

@abonne01
Copy link

abonne01 commented Mar 2, 2021

Hello,

I noticed that at each call of library, severall calls are made to get auth key.
1 wellknown to get jwks_uri
2 jwks URI to get keys.

Could you please check if we could put in cache these keys?

thanks

@bryanapellanes-okta
Copy link

@abonne01,
Thanks for reaching out! Our Php developers will review and respond here with an answer.

Thanks for using Okta!

@bretterer
Copy link
Contributor

@abonne01 It is correct that we are not currently caching these calls. There were some technical issues with doing this, by locking a user into a caching mechanism. It is something we want to address in a future update with the library itself, however i am not sure when this will be slated for work.

In the mean time, if this is causing you an issue, you can provide your own Adaptor. The adaptor is where the call to get keys happens. You can take our current Adaptor, update to your needs with caching, adn then include it in your builder with ->setAdaptor(new \Okta\JwtVerifier\Adaptors\FirebasePhpJwt) like normal, changing the class name to your new one.

@abonne01
Copy link
Author

abonne01 commented Mar 4, 2021

Hi.

sure I can, but I would prefer keep library unmodified to follow potential update.

I will wait then. Thanks for anwser

@bretterer
Copy link
Contributor

The nice thing is we designed this package to allow for Adaptors to be used for customization. You can swap in your own adaptor and still get updates from the main codebase.

As mentioned, I am not sure when this will be slated for work to provide an adaptor with caching.

@RikkiMasters
Copy link

RikkiMasters commented Jul 30, 2021

@bretterer I've made a custom adapter in Laravel for key caching.

public function getKeys($jku)
{
    if (!$keys = Cache::get('oktaKeys')) {
        $keys = json_decode($this->request->setUrl($jku)->get()->getBody()->getContents());
        Cache::put('oktaKeys', $keys, now()->addMinutes(60)); // cache keys for 60 minutes
    }

    return self::parseKeySet($keys);
}

The problem now is the other call it makes to wellknown which is built into the JwtVerifier.php constructor. Is there any way of caching this without changing the package itself? The call adds 300 - 600ms to my api requests in dev.

@convenient
Copy link

convenient commented Sep 14, 2021

Why the requests are slow

These requests are slow because (i think) the SSL handshake is slow, for us around 550ms.

time_appconnect     
The time, in seconds, it took from the start until the SSL/SSH/etc connect/handshake to the remote host was completed.
curl -s -o /dev/null -XGET -H 'Accept: application/json'  -w '\ntime_total: %{time_total}s\ntime_appconnect: %{time_appconnect}s\ntime_connect: %{time_connect}s\n\n'  https://example.okta.com/oauth2/default/v1/keys

time_total: 0.701045s
time_appconnect: 0.526796s
time_connect: 0.171552s

Compare this to google

$ curl -s -o /dev/null -XGET  -w '\ntime_total: %{time_total}s\ntime_appconnect: %{time_appconnect}s\ntime_connect: %{time_connect}s\n\n'  https://www.google.com

time_total: 0.104479s
time_appconnect: 0.047376s
time_connect: 0.011333s

I would think Okta network people need to have a review and see what they can do to improve the connection time for requests outside their own network :/

Improving the performance of this library

So there are three slow requests that happen every time you try and verify a token

The first request is the one we actually need to do, the second two are triggered by this library when you instantiate it to verify a token.

When you instantiate the verifier it does this

$this->metaData = json_decode($request->setUrl($this->wellknown)->get()
->getBody());

When you get the keys to verify your token it does this

public function getKeys($jku)
{
$keys = json_decode($this->request->setUrl($jku)->get()->getBody()->getContents());
return self::parseKeySet($keys);
}

The nice thing is we designed this package to allow for Adaptors to be used for customization. You can swap in your own adaptor and still get updates from the main codebase.

sadly this does not cover both use cases as the adaptor only covers the call to get the keys

What I have done for our Magento 2 implementation is something like the following

// di.xml
    <type name="Okta\JwtVerifier\JwtVerifierBuilder">
        <arguments>
            <argument name="request" xsi:type="object">Namespace\Okta\Service\JwtVerifierRequestCached</argument>
        </arguments>
    </type>
    <type name="Okta\JwtVerifier\Adaptors\FirebasePhpJwt">
        <arguments>
            <argument name="request" xsi:type="object">Namespace\Okta\Service\JwtVerifierRequestCached</argument>
        </arguments>
    </type>

Then inside that new request class you can override the get method to cache both requests, you need to do some jiggery pokery to still return a request object so that the library can get the body, but so far this lib only makes use of the body of the response so I stubbed it into a Laminas\Diactoros\Response\TextResponse without issue

    // Namespace\Okta\Service\JwtVerifierRequestCached
    // class JwtVerifierRequestCached extends Okta\JwtVerifier\Request
    public function get(): ResponseInterface
    {
        $cacheKey = JwtVerifierCacheKey::getCacheKey($this->url->__toString(), $this->query);
        if (!$contents = $this->cache->load($cacheKey)) {
            $result = parent::get();
            if ($result->getStatusCode() !== 200) {
                return $result; //Only cache and handle 200 responses, bubble up any weird errors here
            }
            $contents = $result->getBody()->getContents();
            $this->cache->save($contents, $cacheKey, [\Magento\Framework\App\Config::CACHE_TAG], 3600);
        }

        // We need to return a response object so that the jwtverifier can call ->getBody->getContents()
        // The verifier module doesnt check status codes or use anything other than body
        /** @var Laminas\Diactoros\Response\TextResponseFactory */
        return $this->textResponseFactory->create(['text' => $contents]);
    }

This will cache for the frontend users for an hour a piece and lazy save it if its not there.

I've also added a cron process in the background which automatically grabs these endpoints and saves them to cache every few minutes, this is because the keys can cycle sometimes I think so by essentially polling and forcing these endpoints into cache I can make it so its like customers are always getting the most up to date version from the cache with the fallback to do the request and save themselves if necessary.

This isn't very future proof, if this library every makes use of any part of the request other than the body contents I'll need to revisit it and pass that along after the cache load. However the performance as it stands isn't acceptable for us so this workaround will have to suffice for now.

@convenient
Copy link

convenient commented Sep 16, 2021

@RikkiMasters I am curious if your "300-600ms" are also because of the ssl handshake? Just good to get confirmation from someone else?

You can run the curl commands I detailed above to get some indication.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants