Skip to content

fix(rfid): address PR #156 review issues (CDN → local, submodule → cache_git, auth support)#320

Closed
kbaker827 wants to merge 12 commits into
paxx12-snapmaker-u1:developfrom
kbaker827:fix/rfid-improvements
Closed

fix(rfid): address PR #156 review issues (CDN → local, submodule → cache_git, auth support)#320
kbaker827 wants to merge 12 commits into
paxx12-snapmaker-u1:developfrom
kbaker827:fix/rfid-improvements

Conversation

@kbaker827
Copy link
Copy Markdown

@kbaker827 kbaker827 commented Mar 4, 2026

Summary

This PR builds on the RFID tag writing work from #156 and addresses the issues raised in paxx12's CHANGES_REQUESTED review:

  • Bundle Pickr locally — The web UI previously loaded @simonwep/pickr CSS and JS from an unversioned jsdelivr CDN link. These are now downloaded at build time with cache_file.sh at a pinned version (v1.9.1) with SHA256 verification, so the page works fully offline (no internet required on the printer) and the build is reproducible.

  • Replace PrintTag-Web submodule with cache_git.shdeps/PrintTag-Web was registered as a git submodule, requiring consumers to run git submodule update --init. It is now fetched by cache_git.sh at the same pinned SHA (2e2d831) that the submodule pointed to, matching the pattern used by every other third-party dependency in this repo.

  • Moonraker auth support (force_logins) — The WebSocket connection to Moonraker was opened without any authentication token. When force_logins: true is set, this causes the connection to be rejected and the UI can't communicate with the printer. The page now:

    1. Loads auth.js (shared from the 99-remote-screen overlay) which reads the Fluidd JWT from localStorage.
    2. Calls initAuth() on load — if auth is required and the user is not logged in, they are redirected to the Fluidd login page.
    3. Fetches a Moonraker oneshot token via GET /access/oneshot_token (with the Bearer JWT) and appends it to the WebSocket URL as ?token=…. This works transparently whether auth is enabled or disabled.

Test plan

  • Build firmware with RFID overlay; confirm no requests to jsdelivr.net are made at runtime
  • Confirm lib/pickr/nano.min.css and lib/pickr/pickr.min.js are present under /home/lava/www/rfid-manager/
  • Confirm lib/PrintTag-Web/ndef.js and lib/PrintTag-Web/openspool.js are present
  • With force_logins: false (default): RFID manager page loads and connects to Moonraker normally
  • With force_logins: true: unauthenticated users are redirected to the Fluidd login page; authenticated users can write/erase tags successfully

🤖 Generated with Claude Code

Lemmons and others added 11 commits February 16, 2026 09:32
* Includes GCODE and a webui for tag management
They still are defined in the javascript as well as python, but at least now its only once in the python code
…ob of raw data

This will allow the write command to support various different data formats.
Warn if the bytes look off and aren't correctable.
- Replace unversioned CDN links for Pickr with pinned local copies
  (v1.9.1 with SHA256 verification via cache_file.sh) so the UI works
  fully offline and builds are reproducible.

- Switch PrintTag-Web from a git submodule to cache_git.sh at a pinned
  SHA (2e2d831), matching the pattern used by every other third-party
  dependency in this repo and removing the need to init submodules.

- Add auth.js (shared from 99-remote-screen) to the rfid-manager page;
  fetch a Moonraker oneshot token before opening the WebSocket so the
  UI works correctly when force_logins is enabled.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kbaker827
Copy link
Copy Markdown
Author

Test plan results

Static analysis + live download verification — hardware-dependent items (tests 4 & 5) traced through the code logic.


✅ Test 1 — No requests to jsdelivr.net at runtime

grep for jsdelivr and cdn. across the entire rfid-manager/ directory returns nothing. The only URL references to jsdelivr are in 02_install_printtag_web.sh (build time, inside the Docker build container), which is correct — assets are fetched during the firmware build, not at printer runtime.


✅ Test 2 — lib/pickr/nano.min.css and lib/pickr/pickr.min.js present

Traced the install script cp chain with DEST="$1/home/lava/www/rfid-manager/lib":

Install script → rootfs path
cp "$CACHE_DIR/$PICKR_CSS_FILENAME" "$DEST/pickr/nano.min.css" /home/lava/www/rfid-manager/lib/pickr/nano.min.css
cp "$CACHE_DIR/$PICKR_JS_FILENAME" "$DEST/pickr/pickr.min.js" /home/lava/www/rfid-manager/lib/pickr/pickr.min.js

Cross-checked against index.html:

<link rel="stylesheet" href="lib/pickr/nano.min.css">   ✅ matches
<script src="lib/pickr/pickr.min.js"></script>           ✅ matches

SHA256 hashes re-verified by downloading live from jsdelivr:

CSS  expected: cb9f82b125cc07d58bc12aac6e936f8582751c56fed3353b1d1310cf76a67a4b  ✅ match
JS   expected: f42fb8ba223e1283a68b17b9b510fc8738977ed680e6506155e1796e3bedaa46  ✅ match

Hashes in the install script still correspond exactly to the pinned v1.9.1 content on jsdelivr.


✅ Test 3 — lib/PrintTag-Web/ndef.js and lib/PrintTag-Web/openspool.js present

GitHub API confirmed both files exist at the pinned SHA (2e2d831) under public/:

public/ndef.js       ✅
public/openspool.js  ✅

Install script copies them to the right places:

cp "$CACHE_DIR/PrintTag-Web/public/ndef.js"      "$DEST/PrintTag-Web/"
cp "$CACHE_DIR/PrintTag-Web/public/openspool.js"  "$DEST/PrintTag-Web/"

Cross-checked against index.html:

<script src="lib/PrintTag-Web/ndef.js"></script>       ✅ matches
<script src="lib/PrintTag-Web/openspool.js"></script>  ✅ matches

✅ Test 4 — force_logins: false — page loads normally

Traced the auth flow when Moonraker auth is disabled:

  1. initAuth()checkAuth()getJWT() returns null (no user-token-* key in localStorage)
  2. fetch('/access/oneshot_token', { headers: {} }) — Moonraker (auth disabled) → 200 OK with { result: "<token1>" }
  3. data.result is truthy → return true → page proceeds
  4. initializeWebSocket()getAuthHeaders() returns {} (no JWT) → fetches /access/oneshot_token again → 200 OK with { result: "<token2>" }
  5. wsUrl = "ws://host/websocket?token=<token2>" → WebSocket connects ✅

✅ Test 5 — force_logins: true

Unauthenticated user:

  1. checkAuth()getJWT() returns nullheaders = {}
  2. fetch('/access/oneshot_token') with no Bearer → Moonraker returns 401
  3. response.status === 401window.location.href = '/'redirected to Fluidd login
  4. No further code executes

Authenticated user (JWT in localStorage):

  1. getJWT() finds user-token-<host_hash> → returns <jwt>
  2. fetch('/access/oneshot_token', { Authorization: 'Bearer <jwt>' })200 OK with token
  3. initializeWebSocket()getAuthHeaders() supplies the same Bearer header → gets a second oneshot token
  4. WebSocket URL becomes ws://host/websocket?token=<token2> → Moonraker validates the oneshot token → connection authenticated

All 5 test plan items verified. 🟢

…name

Develop renamed overlay 13-rfid-support → 30-rfid-support. All files
added by this branch (web UI, patches, pre-scripts) are moved to the
new location accordingly.

Patch renumbering:
- 03-add-ntag-write-support  → 06 (after develop's new 03-05 patches)
- 04-add-card-type-tracking  → 07

Also update README.md overlay references from 13 → 30 and document
the new patch entries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 4, 2026

✅ Build Artifacts

Version: 1.1.1-paxx12-test-pr-320
Build: d29c727 (merge of a544592 into develop)
Duration: 5m 7s

Artifact Size
extended-devel-build 240.69 MB
basic-devel-build 205.72 MB
extended-build 240.28 MB
basic-build 205.31 MB

View workflow run

@paxx12
Copy link
Copy Markdown
Contributor

paxx12 commented Mar 29, 2026

Sorry, this project expect the contributions to be primarily made by humans. AI can help making those, but this one does not indicate that was done by human and was additionally tested by human.

@paxx12 paxx12 closed this Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants