-
Notifications
You must be signed in to change notification settings - Fork 31
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
[WIP] Passwordless email and local dev server #857
Open
lazerwalker
wants to merge
26
commits into
main
Choose a base branch
from
local-server
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
This was referenced Sep 20, 2024
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.
This does two main things:
/sendMagicEmail
endpoint takes an email address, generates a URL that has query params containing a unique userId for that email address and an expiring auth token based on that userId, and sends that URL to that email address. The client stores such query params in localStorage and sends them along as headers with network requests. The server now validates that token instead of the previously-used Firebase token. When running as a local dev server, it doesn't send emails but logs URLs to the console. Seedocs/authentication.md
for more information.server/server.ts
, which is an Express server that serves the same routes as our Azure Functions server, as well as WebSockets (including a reimplementation of Azure PubSub's group management functionality). This implementation is explicitly not production-ready, and is currently only intended to be used for local dev, although this may change.npm run dev
now runs both client and server instances, with separate individualnpm run dev:client
andnpm run dev:server
commands also existing. See README for setup info.I tried to keep refactors minimal to minimize the footprint of these changes, but a few incidental refactors to note
npm run dev
performs an initial copying of all images into thedist
folder, meaning that you won't have broken images in dev instances any more. This does not auto-update, so if you add new images you'll need to re-run the dev server.App.tsx
is much simpler now, including cleaning up the muddledAuthenticate
reducer action./cognitiveServicesKey
endpoint, which existed to set up live transcription from Twilio video and was not being called from the current clientThese changes require Node 18 on the server, where (as of 9/19/2024) we are currently using Node 14 in production (which is EOL). Before we deploy this, we should migrate our existing live production server to use Node 18 (an Azure Portal change, does not require a code change/deploy) and confirm this does not cause any issues. This should be uncontroversial (famous last words).
Work that needs to be done:
npm run dev
to watch the image folder and recopy as necessary (should just require changing how we're usingrsync
)I think it is unlikely we want to deploy this for 2024 (unless we have a good answer to "how are we going to sufficiently test a new authentication flow"), but that can be a discussion topic once this is more ready-to-go.