Closed htdvisser closed 2 years ago
More details in related issues: https://github.com/TheThingsIndustries/lorawan-stack/issues/1393 and https://github.com/TheThingsNetwork/lorawan-stack/issues/3304
Moving this to v3.16.0 so that we can first measure how much performance we can actually gain from this after the changes in https://github.com/TheThingsNetwork/lorawan-stack/pull/4732.
After doing some profiling of production deployments, it doesn't look like this will make much difference.
Access key + Client / API key + User lookups in the database cost < 5 ms, so this isn't really an issue. PBKDF2 hashing was already made less time-consuming for Access Keys and API keys by lowering the number of iterations in https://github.com/TheThingsNetwork/lorawan-stack/pull/3038 there. So I don't think there's that much to win here.
Summary:
Results of
(*IdentityServer).authInfo
should be cached.Why do we need this?
For performance reasons.
What is already there? What do you see now?
When
ttn-lw-stack
is started with all components attached, calls to the NS/AS/JS all use the modified IS rights hook that talks to the DB. As a result, a full SetEndDevice will do the entire rights check 4 times.The IS rights hook calls
(*IdentityServer).getRights
, which is quite a heavy call. It callsauthInfo
, which looks up the access token or API key and verifies it (with pbkdf2) andentityRights
which derives the membership tree. InentityRights
there is already some caching for the membership trees so that we don't have to hit the database all the time.What is missing? What do you want to see?
We should avoid doing
pbkdf2
hashing all the time. Ideally we should also avoid hitting the DB with API key (or Access Token and Client) GETs all the time.How do you propose to implement this?
authInfo
we can add a check with Redis before doingauth.Password(...).Validate(...)
. Then we've already made sure that the API key still exists.Can you do this yourself and submit a Pull Request?
Yes I can