search

node-oauth / node-oauth2-server

🚀 The successor to oauthjs/oauth2-server. 🔒 Complete, compliant, maintained and well tested OAuth2 Server for node.js. Includes native async await and PKCE.
https://www.npmjs.com/package/@node-oauth/oauth2-server
MIT License
286 stars 39 forks source link

perf: random tokens do not need to be hashed #313

Closed dhensby closed 1 month ago

dhensby commented 1 month ago

There are some wasted CPU cycles when generating the random tokens.

There's no need to generate extra random bytes only to hash them. A random input will lead to a random hash being generated, but the random input is enough in its own right and does not need to be hashed to make it any more or less secure. The amount of entropy is capped at 32 bytes when hashed, so we may as well just provide 32 random bytes.

As an added bonus, this also returns the method to a true non-blocking async function.

jankapunkt commented 1 month ago

@dhensby thanks for the improvement. Do you have experience with performance tests? I think they would be a great addition in the long run for methods like these.

dhensby commented 1 month ago

I haven't written or run performance tests before. I know there are even more theoretical improvements to be made to performance around random number generation (I know that the uuid lib has done some performance work around that by not hitting randomBytes every time), but it depend how focused on performance we are whether we want to implement those kinds of optimisations.