-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(core): API Keys #3815
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
base: minor
Are you sure you want to change the base?
feat(core): API Keys #3815
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@DanielBiegler I've implemented the complete workflow of api keys with service accounts (tests, entities, strategy, dashboard ui, etc.). Let me know if you would like to discuss. I can share the codebase for you to review. ![]() |
Further TODOs in comments, UI & Tests still missing but API Keys work! :)
92282b9
to
8ea0323
Compare
This works now you can have separate permissions on your API Keys!
…ys from requests Also fixes and improves the session token extraction
|
Description
Support for API-Keys so you can write scripts and third-party integrations that dont have to worry about sessions and token expirations.
Warning
This is a work in progress, stuff's missing. See the TODO comments in the code and here.
How does this work?
When creating a new key via the api-key-service it creates a 100 year long "session" with the hashed-api-key as its session token. So everything related to that, like session cache etc. works like before!
This PR adds an
api-key
token-method which users can enable in their vendure configs, for example:["cookie", "bearer", "api-key"]
This allows for this to be entirely opt-in and without(!) impacting any hot path because I preserve the current order of checking for tokens
So any regular instance that won't have this enabled after patching shouldnt notice any change.
Now that the auth guard can extract an api-key from a header, we can hash the api-key and simply look it up in the session cache / DB, just like you would check for a regular session token.
Since this builds upon the existing logic even the shop-api can have api keys as well. Advanced usecase could be a Plugin that introduces a new mutation
createCustomerApiKey
which sets the owner AND apiKeyUser to the customer-user now enabling the customer to share its key with third-party integrations, for example AI Agents that buy something for you. Technically possible!Initially I thought about introducing an authentication method but after thinking on that for longer I realized that auth-methods are there for authenticating whereas api-keys are basically for authorizing. We dont want to "login" with an api key. We just need to be authorized to do something.
Example Usage
Some open questions:
1. Where do we put the API-Key?
Solved: Added
apiKeyHeaderKey
Config option that lets users define a header, defaults tovendure-api-key
.Original question/reasoning
Two main approaches are either in the
Authorization
-Header or custom header likex-api-key
, the latter seems more flexible.Here is how Stripe & Mollie do it with their APIs:
For this proof of concept I went with the same approach that Stripe is doing, namely using "Basic"-Auth. Changing this to a custom header is trivial, we just have to come to a consenus. Also interesting, could using Basic auth run into problems of people hosting vendure behind basic auth themselves? Not sure but that would probably decide this question for us to pick the custom header.
2. What's a suitable default hash function for API Keys?
Solved: Use a separate lookup ID, similar to shopware6 approach detailed below in the research section, this allows us to reuse the default password hashing behavior, namely bcrypt.
Shops that need higher performance per request could replace the default hashing strategy with a more performant hashing method.
The default now of 24 hex-characters of lookup ID plus 64 hex-characters of the key itself coupled with bcrypt seem sufficient, judging by looking at other projects.
Original question
Since API Keys are "basically" passwords, they shouldnt be stored in plaintext, this comes with an interesting consideration:
For passwords it's recommended to use a computationally painful hash method to minimize bruteforce effectiveness, but for API Keys we do need to hash them on every request, see sesstion token extraction, which means they must be as fast as possible and "hopefully never" collide.
With the node:crypto package we get access to openssl digest algorithms so to make things easy we could just pick something well known like SHA1 which is "probably" (?) good enough maybe? Keeping in mind that the server owners control an entirely hidden cryptographically secure generation function, see ApiKeyGenerationStrategy and example implementation, meaning the attackers dont control any input here and with lots of entropy to make bruteforcing more unappealing.
Using bcrypt similar to passwords would require storing some fixed salt inside the API-Key (eww) or rather a lookup key, because using the default salt generation would make every request have a different hash with the same API-Key and without a lookup key we dont know which hash to compare to. All that complexity with the performance implications, is that worth/necessary since the instance owners control the generation function? Would enough entropy in the generation be good enough, I'm not experienced enough in cryptography.
Maybe the performance implication of looking that stuff up every api-key-request plus more expensive hashing is still alright since api-key-requests should be "rare" in comparison to admin/shop users. If the hashing is too computationally expensive, does this become a DDOS vector?
Also: Since attackers could see the source of the default generation function, they technically could construct a rainbow table if we dont combine the generation with a secret. Easy fix is using an HMAC with the already existing server side secret used for cookies.
3. Introduce a
ApiKeyExpiryStrategy
?Solved: I did introduce a
lastUsedAt
field which would allow users to clean up keys via scheduled tasks or db triggers themselves. Let's keep it simple for now and not introduce another strategy.Original question/reasoning
Right now in this proof of concept API-Keys are valid "forever" (technically 100 years) an expiry strategy could be cool. For example we can introduce some type of
lastUsed
field and the default expiry-strategy could delete all keys that havent been used in X weeks/months. Both medusajs and shopware6 include a last-used field.Probably (?) would need to be checked on a per-request basis then or some schedule (?)
4. Impersonation & Privilege escalation
Solved: By default, you can't create an API-Key for an existing User. An API-Key gets its permissions from an underlying new user which has no authentication method and is solely used for holding roles.
We can easily restrict the available roles by only allowing to set roles which the owner of the api key has. We should investigate if theres an other privilege escalation path here somewhere, but the logic should basically be the exact same for any admin that has the CreateAdministrator permission.
I did however include this functionality for plugin authors via a simple argument!
Original question/reasoning
When we allow someone to create a key for someone else, it automatically means they can impersonate that user. There might be legal/compliance/iso-certification issues with allowing that (help?), and of course privilege escalation issues, so for now I'm restricting admins to only be able to create keys for themselves just to prove the concept.
This could be annoying when someone wants to create an admin-robot-user with less permissions than themselves, they'd need to create that admin, log into that, create a key and log back to their original user.
Ideally permissions would be tied directly to the api key itself instead of an underlying user but that would need more changes. I'll have to look into that, maybe we can introduce a new type of user like David suggested, and tie that new user to the key. Then when persisting a new api key session we simply save that user-id. Ownership of this new robot-user could be nicely enforced because we can relate through the admin/customer to their api-key and further to the robot-user. Then channels and permission should take over like before (?)
This sounds ideal actually, because creating different admins that are not meant to be logged-into is "mehh" but I gotta look if theres something im missing here.
Research
Medusajs
Links: Data Model Source
Inspecting the postgres table you can see:
scrypt
from packagenode:crypto
, see sourcecrypto.randomBytes
crypto.randomBytes
Since you can't know which hash to compare to medusa fetches all valid keys into memory and hashes them concurrently and filters out the matching one in javascript, see source, (this is terribly naive)
You may only have one active api key for the admin api at a time, see source, which is honestly so backwards that it starts to become impressive
By default there are no roles/permissions in medusa so the api key just gets attached to the user who creates it, which automatically means the sole api key has access to everything and cant be scoped
Conclusion: Medusas approach is so entirely backwards it is actually an extremely impressive case on how wrong you can make software
Shopware6
[a-zA-Z0-9]
and then b64 encoded, see sourceLet's calculate how big the search space is in comparison to medusas keys.
But keep in mind that technically the space gets larger for shop ware because you not only need the secret, you also need the correct access key. Didn't include it in the calculation.
Integrations can provide roles, wheras user keys impersonate the user
They are defined as
PasswordField
, see source, which seem to get hashed, source, when serialized by a configurable hash algo, by default bcrypt I think (?)accessKey
which is not the secretAs I described earlier, salted algos like bcrypt would make it impossible to directly lookup the hash which you need to compare to, which is why shopware retrieves the hash via a second "access key" lookup ID, see source
Conclusion: Shopware seems well thought through and makes me lean towards also adding a lookup ID to be able to support hard algos like bcrypt, argon, etc. instead of only single iteration digests like sha-1. This has also a nice sideffect of allowing the users to identify their api keys in a list without leaking any character of their secret.
Should the lookup be included in the same header or split up into two?
Saleor
App Auth happens via bearer token and the tokens do not expire, see docs
These are auth for machine-to-machine comms where an integration is an "app", i.e. if you'd wanna allow your CRM/ERP/.. to make changes in vendure you'd create individual "apps" for them.
Similar to the earlier problem of needing more info than just the key itself so you know what to compare to, you need to provide more info to your request, see source:
Sounds to me the
appId
is basically what shopware uses as their "access key" but a bit less precise, it helps to identify which app holds this token but the fetching itself seems to work by reading all the keys that have the same last 4 characters, see source, and if any of those matchvalid
becomes trueApptokens have a max length of 128, see source
See the verification being done by
from django.contrib.auth.hashers import check_password
and for app tokens used hereSylius
Sylius seems to entirely rely on the underlying Symfony framework both for Users and Auth which I also looked up and Symfony works on the "passport" based system where it abstracts different auth types etc. and lets you attach "badges" that hold additional info
There is a short demo in the docs on how to add api token auth, namely a custom authenticator called
ApiKeyAuthenticator.php
its the very first example hereAnd they do mention API keys here and there
So it seems to me Sylius entirely relies on symfony which makes you implement your own custom authenticator for api keys, they themselves link to Symfony docs here. So after you create your custom authenticator, add it to your config and voila you can auth with api keys in your api. But the underlying logic you have to make yourself or use someone elses api-key-passport implementation
ToDos
Fixes #3412
Breaking changes
No functionality breakage but a migration will be needed due to two new Entities, namely
ApiKey
andApiKeyTranslation
Screenshots
TODO
Checklist
📌 Always:
👍 Most of the time: