Roguelike-Celebration / azure-mud

MIT License
191 stars 33 forks source link

[WIP] Passwordless email and local dev server #857

Open lazerwalker opened 2 months ago

lazerwalker commented 2 months ago

This does two main things:

  1. Rips out Firebase and replaces it with a "magic email" implementation. The /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. See docs/authentication.md for more information.
  2. Provides an alternate server implementation, 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 individual npm run dev:client and npm 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

These 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:

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.

jcorbin commented 1 month ago

Okay, got this up and running with one minor CORS hiccup:

Anyhow, good stuff, I thought I was going to die on the initial level, and have to post a question here, but instead I managed to make it work :-)

lazerwalker commented 1 month ago

@jcorbin Thanks! This honestly isn't yet in a place where I expected anyone to check this out, least of all someone not familiar with the codebase — I'm glad it was workable!

To confirm: the only .env sample fix needed is to change https to http in the client env's server hostname?

what would you think about a dev:setup script that would do the various .env file copies for folk? I for one totally missed the toplevel .env file also, ending up with an Config.hostname = undefined // ala $SERVER_HOSTNAME, before even getting to the CORS problem after doing the copy

So, my understanding is setup consists of a git clone, two npm install calls, two cp .env.sample .env calls, and a Redis install which is trickier to automate since it differs by platform. For so few steps, I'm not sure an automated script isn't premature optimization (especially since it won't be able to handle e.g. installing redis), and I'm hesitant to automate away npm install calls when we're dealing with folks who may not be used to the JS ecosystem and may discover their node install isn't set up right or is the wrong version.

Until/unless this balloons in complexity, my instinct is updating the README to make sure the first "copy .env.sample to .env" step makes clear you're doing this in the project root is probably the right level of abstraction for now?

also maybe it's too early to simplify the README away from "if running local dev..." but I really want to ;)

To be clear, this new local server implementation does not signify (for now) a shift away from Azure as the only recommended deployment path. server.ts is extremely naive and not currently production-ready, and at least I personally don't plan to invest time in changing that in, say, the time between now and the next Roguelike (although PRs gladly accepted!).

If you're talking about local dev in terms of "this is how to set up a local client, and optionally a local server", at least until we know this flow works well I imagine there may still be folks who use a local client with the remote server, especially since e.g. populating Redis with room data is mildly annoying.

(Actually, that's a good question: can you successfully log in and navigate the space? Realizing the instructions don't tell you to populate room data in the admin panel, which I think is mandatory for the space to actually be usable?)

I seem to recall someway to inject your own custom handlers into parcel itself, rather than needing to run a separate node/express process... ...but even if that's possible, we're held back be the two separate package.json / node_modules arenas, which I think is a thing that switching to yarn might eventually solve?

Not quite sure what you mean by "your own custom handlers". Parcel has an API proxy option, but that wouldn't replace having to run our own local node process for the server, it would just replace tweaking .ENV in a way that I don't think is necessarily a value-add.

jcorbin commented 1 month ago

To confirm: the only .env sample fix needed is to change https to http...

I believe so; spent a long time getting to that One Simple Change, as was orientating on a new codebase, and getting back into web dev after many a year in hiatus. All of my next problems seem to be about an under-populated redis database, e.g. error messages about lacking an obelisk room.

For so few steps, I'm not sure an automated script isn't premature optimization... ...until/unless this balloons in complexity, my instinct is updating the README

That's fair, I suppose that am coming from more a background of "npm install and lol you just have to run a redis for everything" is an assumed starting position, and was more looking to file off the parts that were bespoke to this project vs other redis-using nod projects. But you're right that it's not really worth the squeeze at this point. Perhaps just some clearer wording in the readme about how there are two different .env files would help, since my pain point was all about missing the client/parcel server env file.

Another tactic would be to eliminate the need to even have a .env file for the local dev server mode of "deployment", and make it the default. The alternative is that you end up with undefined values if the user misses the file copy. Specifically the type of Config.SERVER_HOSTNAME should currently be string|undefined but typescript is being too lenient; I believe that opting in to noUncheckedIndexedAccess via tsconfig would force light on this issue, but given the current iceberg of tech debt we have to work through, now isn't the time to go turning up the type check strictness ;-)

To be clear, this new local server implementation does not signify (for now) a shift away from Azure as the only recommended deployment path. server.ts is extremely naive and not currently production-ready, and at least I personally don't plan to invest time in changing that in, say, the time between now and the next Roguelike (although PRs gladly accepted!).

Well I'm coming from a prior of "already have a VPS via linode, and would like to just spin up my own mini mud there" as a dev goal beyond just "help slay the bitrot boss". So I'm quite motivated to make server.ts be a viable path for my own personal deployment purposes in the medium term, let alone uplifting the local dev story.

populating Redis with room data is mildly annoying.

I'm glad you mentioned this, since that's literally the level that I've found myself on now after descending the "process turn up" staircase ;-)

Actually, that's a good question: can you successfully log in and navigate the space?

Sorta, like I can send a message (haven't confirmed if others can see it, as neglected to even add a 2nd user in different browser to play ping/pong syn/ack games with...). I can also click different rooms on the map, and it sometimes sorta works. Sometimes I seem to loose session/just get logged out.

Realizing the instructions don't tell you to populate room data in the admin panel, which I think is mandatory for the space to actually be usable?

Looking at redis-cli, there are a good many room keys and such populated just by starting the server, but I've not begun to poke too deeply at the data schema as yet.

I seem to recall someway to inject your own custom handlers into parcel itself...

Not quite sure what you mean by "your own custom handlers"

What I mean is that "hey parcel already is a node server process, probably even using express"; I thought there was a way to just give it a handler or 2 so that it'll host your backend for you, obviating any need for a "backend/frontend" process split; they're both just http servers, one's a fancy static file server ( yes with auto compilation/transform and HMR, but squint and it's just a value added static file server ), and the other is our application specific endpoints.

OTOH, for my own deployment goals I'll probably go the other way and instead tack static file serving onto server.ts so that it can vend out all the compiled assets, images, etc. Altho I suppose the more modern thing to do is to pair your node process up with something like Caddy, which is at least less of a mental lift than orchestrating nginx for something like a single node deployment.

Anyhow, for a final meta note here, what's the best way to communicate going forward? Tried reaching out on Discord since we're mutually in the Roguelikes server, but not sure how much you use that channel. Is github issue/pr novelization like this the best for now?

jcorbin commented 1 month ago

Okay, getting back into the swing of how even stacked PRs work here on github, so not sure I've done that old song-and-dance correctly, but here's a couple draft PRs with progress:

lazerwalker commented 4 weeks ago

@jcorbin I haven't seen any Discord outreach from you, but I'm also honestly not sure what you're talking about — I don't believe I'm in any roguelike-themed Discords other than the internal Roguelike Celebration organization Discord.

It would probably be helpful to get you into our MUD discussion channel there, if you send me a DM (username: lazerwalker) I can get you an invite.

jcorbin commented 4 weeks ago

Sent an email to hi@lazerwalker.com, since I already did DM the only "lazerwalker" account that I could find on discord back on the 21st; sorry if I'm making this overly complicated :-/