-
Notifications
You must be signed in to change notification settings - Fork 1
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
Auth overhaul #79
Merged
Merged
Auth overhaul #79
Conversation
This file contains 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
shanedg
commented
Jan 9, 2023
shanedg
commented
Jan 9, 2023
shanedg
commented
Jan 9, 2023
discord/lib/handlers/create-authenticated-render-handler.test.js
Outdated
Show resolved
Hide resolved
shanedg
commented
Jan 9, 2023
shanedg
commented
Feb 14, 2023
shanedg
force-pushed
the
auth-overhaul
branch
2 times, most recently
from
February 20, 2023 10:27
5976f70
to
e904770
Compare
shanedg
commented
Feb 20, 2023
shanedg
commented
Feb 20, 2023
shanedg
commented
Feb 20, 2023
shanedg
force-pushed
the
auth-overhaul
branch
from
February 26, 2023 03:43
e904770
to
2ee2640
Compare
Establish explicit paths for intercepting the authorization code, rendering the login page, and rendering the authenticated application. Replace authenticated html template with @trshcmpctr/client build. Replace hard-coded localhost addresses with a required redirect_uri configuration option. Add a helper for creating redirect handlers. Rename getNewTokenWithDependencies to createAuthorizationCodeGrantHandler. createAuthorizationCodeGrantHandler now: - no longer renders any html - only responsible for authenticating from authorization code - no longer fetches user and guild data - no longer injects user data or new session flag Rename getRenderLoginWithData to createLoginRenderHandler. createLoginRenderHandler no longer checks for code query param because this is now exclusively handled by getHandleAuthentication. Rename getReuseSessionTokenWithDependencies to createAuthenticatedRenderHandler. createAuthenticatedRenderHandler now: - renders html from @trshcmpctr/client build - no longer fetches user and guild data - no longer injects user data or new session flag
Discord auth ensures we have the user's identity but we require them to be a member of a particular guild to be authorized to see any data or take any actions. Removes previous guild membership check based on separate user and guild API requests. Removes now-unused request batching and guild filtering utils. Removes now-unused 'identity' scope.
Replace cookie-session with express-session backed by filesystem store
Return guild membership handler from a higher order function to allow for more convenient testing. Catch possible rejections from fetching the discord api. Add tests.
Catch possible rejections from posting the discord token endpoint
Avoid test file before hook for only one test
Express does not catch *any* handler errors automatically
Better than calling and returning on the next line
Prefer to avoid reexporting modules via directory index files. It adds unnecessary noise to fuzzy file search and creates toil when renaming exported modules.
Prefer to avoid reexporting modules from directory index, adds noise to fuzzy file search and toil renaming exports
shanedg
commented
Jun 25, 2023
shanedg
force-pushed
the
auth-overhaul
branch
2 times, most recently
from
June 25, 2023 17:34
b7ee4fe
to
4cafd24
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.