Leaderboard api - #561
Open
btfcookies wants to merge 2 commits into
Open
Conversation
Ranks every user by the number of questions they have heard. The data already existed behind /api/admin/leaderboard; this exposes it on the public, CORS-enabled API and widens it beyond raw play counts. database/account-info/leaderboard.js now also reports numCorrect, powers, tens, negs, tossupPoints, bonusPoints, points, pptu, ppb, accuracy, and averageCorrectCelerity. The existing fields are kept, so /api/admin/leaderboard and client/admin/leaderboard are unaffected. Also fixes three problems in that aggregation: - slice(0, limit) returned [] when limit was omitted, since Array.prototype.slice treats a null end index as 0. Omitting the limit now returns every row. - Users deleted since their buzzes were recorded produced rows with no username. They are dropped. - mergeTwoSortedArrays compares usernames with < and >, but the inputs were ordered by MongoDB's byte-wise sort. The two disagree outside ASCII, which split a non-ASCII user into two rows. Both inputs are now sorted in JS with the same comparison the merge uses, and the unindexed $sort stages are gone. The route catches rejections from the aggregation and returns 503. Express 4 does not forward a rejected promise to the error handler, so without this a database error takes down the process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This endpoint returns every user's username alongside their play stats. Rather than deciding on the maintainers' behalf that this is fine to publish openly, require a ?key= that matches LEADERBOARD_KEY in the environment. Requests with a missing or wrong key get 401. If LEADERBOARD_KEY is unset, every request is rejected rather than the endpoint quietly falling back to public — a deploy that forgets to set it fails closed, not open. The comparison hashes both sides with SHA-256 before calling timingSafeEqual, since that function throws on unequal-length buffers and comparing raw lengths would leak the true key length to a timing attack. Cache-Control changes from public to private: a shared or CDN cache must not be allowed to serve this response to a caller that supplied no key of its own. Docs updated to describe the key requirement and the new 401. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
|
i think i understand the shape of this PR. some objections:
|
Contributor
Author
|
Thanks for the review, I understand the need for user privacy. I will make a separate pr for the bug fixes. As for the api, would you be willing to instead setup an outbound webhook to POST the data directly to my server? It would still be secure since you could sign the HTTP request contents with your private key and I will verify the signature on my server using your public key. Additionally, I can change the functionality of the leaderboard so that data will only be displayed for users who make an account. |
Contributor
Author
|
oh yeah and as for the SSIs i will try to implement that in a seperate pr (if i have time before school starts) |
Member
|
the SSI was super fast and brought me much joy so i went ahead and implemented it |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
slice(0, limit)returned [] when limit was omitted, deleted users left rows that didn't have usernames, mergeTwoSortedArrays didn't match MongoDB's sort outside of ASCII, which split not ASCII usernames into 2 rows.