bhj / KaraokeEternal

Open karaoke party system
https://www.karaoke-eternal.com
ISC License
460 stars 70 forks source link

Add QR Code to player #57

Open EffakT opened 1 year ago

EffakT commented 1 year ago

This adds a QR code to the bottom left of the player, using the document.baseURI as the URL for the QR code. Resolves #32

bhj commented 1 year ago

Hey, welcome and thanks for this!

The main reason this feature hasn't been implemented (yet) is that I'd like it to be a bit more flexible, which depends on there being per-room settings, which don't exist yet. Once they do, I think these ideally should be configurable per-room:

It's a lot, I know, but users will (rightly) be picky about things on the screen all the time. This PR is a great reference for the core functionality though. Thanks again!

EffakT commented 1 year ago

Sure, I'll look into adding settings. This was just a really "get a QR code on there" sort of thing for now. Instead of alternating the location each song, maybe at an interval? in-case the screen is on, with nothing in the queue.

I'll update the PR with a checklist of requirements.

EffakT commented 1 year ago

@bhj With regards to passing through the room password, What are your thoughts on using a nonce or short-term password for this? Would provide better security rather than passing the room password through to the client.

It could work in a way that after the QR code is used it regenerates a new password & QR code, leaving the previous password(s) available for a period of time (in-case multiple people scan the same code).

Basic steps would be:

  1. Generate a password & QR Code
  2. The client requests the page with the temporary password as a query string
  3. Client asks the server if password is correct (active or within the last few minutes)
  4. Server is notified the password is used, generates a new password and a new QR code.
  5. In the background, server can clean-up expired passwords to keep the database clean.

Let me know if you think this is too complicated, or if you have any other thoughts on how this should work.

bhj commented 1 year ago

@EffakT Very nice work! And wow, there are still more class-style components left than I thought 😮‍💨

Let's keep the QR password stuff simple for now (maybe just base64 it?). I guess the question is what this means for the QR payload size, and whether we need to enforce a max length for room passwords.

bhj commented 6 months ago

Just a note to say I haven't forgotten about this; been working on some long-standing things on the server side. Thanks again for the contribution and patience!