-
Notifications
You must be signed in to change notification settings - Fork 3
feat: seamless reauthentication #66
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
Conversation
Signed-off-by: Derek Kaser <[email protected]>
WalkthroughThe reauthenticate button's client-side handler was changed from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Settings.php
participant JS as tailscaleUp()
participant Server
User->>UI: Click "Reauthenticate" button
UI->>JS: onclick -> tailscaleUp()
JS->>Server: (AJAX) request to bring Tailscale up / reauthenticate
Server-->>JS: response (success / error)
JS-->>UI: update UI (disable/enable, messages)
Note: diagram shows new control flow invoking Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Derek Kaser <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php (1)
244-248: Remove unused function.The
expireTailscaleKeyNow()function is no longer called after switching the reauthenticate button to usetailscaleUp(). This dead code should be removed.Apply this diff to remove the unused function:
- async function expireTailscaleKeyNow() { - $('div.spinner.fixed').show('fast'); - var res = await $.post('/plugins/tailscale/include/data/Config.php',{action: 'expire-key'}); - location.reload(); - }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/locales/en_US.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-25T02:28:03.838Z
Learnt from: dkaser
Repo: unraid/unraid-tailscale PR: 51
File: src/usr/local/emhttp/plugins/tailscale/rc.tailscale:14-14
Timestamp: 2025-10-25T02:28:03.838Z
Learning: In Tailscale 1.90.2+, the `-hardware-attestation` and `-encrypt-state` flags are valid command-line options for tailscaled, even though they may not be fully documented in public knowledge base articles yet. The `-hardware-attestation` flag uses hardware-backed keys (TPM 2.0) to bind node identity, and `-encrypt-state` encrypts the state file on disk.
Applied to files:
src/usr/local/emhttp/plugins/tailscale/locales/en_US.json
🔇 Additional comments (1)
src/usr/local/emhttp/plugins/tailscale/locales/en_US.json (1)
63-63: LGTM! Clear and concise text.The updated reauthentication description is more straightforward and aligns well with the simplified flow.
| <dt><?= $tr->tr("settings.context.reauthenticate"); ?></dt> | ||
| <dd> | ||
| <span><input type="button" value="<?= $tr->tr('settings.reauthenticate'); ?>" onclick="expireTailscaleKeyNow()" <?= $tailscaleDisconnect; ?>></span> | ||
| <span><input type="button" value="<?= $tr->tr('settings.reauthenticate'); ?>" onclick="tailscaleUp()" <?= $tailscaleDisconnect; ?>></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that tailscaleUp() exists and provides the expected reauthentication behavior.
The button now calls tailscaleUp() instead of expireTailscaleKeyNow(), representing a significant behavioral change from explicit key expiration to seamless reauthentication. Ensure that tailscaleUp() is defined and implements the intended functionality.
Run the following script to locate the tailscaleUp() function definition:
🏁 Script executed:
#!/bin/bash
# Description: Find the tailscaleUp() function definition in the codebase
# Search for tailscaleUp function definition
rg -nP --type=js --type=php -C3 'function\s+tailscaleUp\s*\('Length of output: 836
🏁 Script executed:
#!/bin/bash
# Description: Verify Settings.php and Tailscale.php are in same context
# Check directory structure
echo "=== Directory structure ==="
find src/usr/local/emhttp/plugins/tailscale/include/Pages -type f -name "*.php" | head -20
# Check if Settings.php references or includes Tailscale.php
echo -e "\n=== Check Settings.php for references to Tailscale.php ==="
grep -n "Tailscale" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php
# Check what files are in the same directory and likely included together
echo -e "\n=== Check for common includes pattern ==="
rg -nP "include|require" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php -B2 -A2 | head -30
# Check if tailscaleUp function is called elsewhere in Settings.php
echo -e "\n=== Check for tailscaleUp calls in Settings.php ==="
grep -n "tailscaleUp" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.phpLength of output: 575
🏁 Script executed:
#!/bin/bash
# Description: Verify Settings.php and Tailscale.php are in same context
# Check directory structure
echo "=== Directory structure ==="
find src/usr/local/emhttp/plugins/tailscale/include/Pages -type f -name "*.php"
# Check if Settings.php has tailscaleUp calls
echo -e "\n=== Check for tailscaleUp calls in Settings.php ==="
grep -n "tailscaleUp" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php
# Check Tailscale.php to understand full context
echo -e "\n=== Check Tailscale.php header and script includes ==="
head -50 src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php
# Check how both files are used in the plugin structure
echo -e "\n=== Check main plugin file ==="
find src/usr/local/emhttp/plugins/tailscale -name "*.php" -type f | grep -E "(index|main|plugin)" | head -5Length of output: 2729
🏁 Script executed:
#!/bin/bash
# Description: Verify how Settings.php and Tailscale.php are loaded together
# Check for page inclusion logic in main plugin files
echo "=== Check for page loading logic ==="
find src/usr/local/emhttp/plugins/tailscale -name "*.php" -type f ! -path "*/Pages/*" -exec grep -l "Settings\|Tailscale" {} \;
# Look at the plugin entry point or index
echo -e "\n=== Check plugin structure ==="
ls -la src/usr/local/emhttp/plugins/tailscale/ | head -20
# Check if there's a main plugin PHP file that includes pages
echo -e "\n=== Search for page includes in main plugin files ==="
rg -n "Pages/Settings|Pages/Tailscale" src/usr/local/emhttp/plugins/tailscale/ --type=php -B2 -A2
# Check if Settings.php is loaded as part of the same page view
echo -e "\n=== Check Settings.php structure ==="
head -30 src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.phpLength of output: 3214
🏁 Script executed:
#!/bin/bash
# Description: Check page loading mechanism
# Check page.php to understand how pages are loaded
echo "=== Check include/page.php ==="
cat src/usr/local/emhttp/plugins/tailscale/include/page.php
# Check a .page file to understand the page descriptor format
echo -e "\n=== Check Tailscale-1-Settings.page ==="
cat src/usr/local/emhttp/plugins/tailscale/Tailscale-1-Settings.page
# Check Tailscale.page
echo -e "\n=== Check Tailscale.page ==="
cat src/usr/local/emhttp/plugins/tailscale/Tailscale.page
# Check if Settings.php has any script includes at the bottom
echo -e "\n=== Check Settings.php for script includes ==="
tail -50 src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php | grep -A5 "script"Length of output: 2742
🏁 Script executed:
#!/bin/bash
# Description: Verify if tailscaleUp is defined elsewhere or if pages share scripts
# Search for all tailscaleUp definitions
echo "=== Search for all tailscaleUp definitions ==="
rg -n "tailscaleUp" src/usr/local/emhttp/plugins/tailscale/ --type=php --type=js
# Check end of Settings.php for script includes or references to Tailscale.php
echo -e "\n=== Check full Settings.php for script src includes ==="
grep -n "script src" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php
# Check if Tailscale.php is included in Settings.php
echo -e "\n=== Check if Tailscale.php is included anywhere ==="
grep -r "Tailscale.php\|include.*Tailscale\|require.*Tailscale" src/usr/local/emhttp/plugins/tailscale/include/Pages/ --type=php
# Check Tailscale.php to see if it has a separate script file with these functions
echo -e "\n=== Search for external script files ==="
grep -n "script src" src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php
# Check if there's a shared JS file
echo -e "\n=== Check for JS files in the plugin ==="
find src/usr/local/emhttp/plugins/tailscale -name "*.js" -type fLength of output: 1572
🏁 Script executed:
#!/bin/bash
# Description: Check if tailscaleUp function exists in Settings.php or is embedded elsewhere
# Check entire Settings.php for any function definition
echo "=== Search for function definitions in Settings.php ==="
grep -n "function\|async" src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php
# Get the full Tailscale.php script section
echo -e "\n=== Get full Tailscale.php script section ==="
sed -n '/<script>/,/<\/script>/p' src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php | head -100
# Check if there are multiple <script> tags in Tailscale.php
echo -e "\n=== Count script tags in Tailscale.php ==="
grep -c "<script" src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php
# Check if Settings.php footer includes any script references
echo -e "\n=== Check Settings.php footer (last 100 lines) ==="
tail -100 src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php | head -50Length of output: 7269
Fix undefined tailscaleUp() function reference in onclick handler
The button at line 218 calls tailscaleUp(), but this function is defined only in Tailscale.php, which is loaded in a separate page context. Settings.php does not include or define tailscaleUp(), causing a runtime error when clicked.
Define tailscaleUp() within Settings.php, or ensure the function is available in the Settings page's JavaScript scope (e.g., by loading it from a shared script file).
🤖 Prompt for AI Agents
In src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php around line
218, the onclick handler references tailscaleUp() which is not defined in this
page and causes a runtime error; either add a local JavaScript definition of
tailscaleUp() in Settings.php that mirrors the behavior in Tailscale.php or
include the shared script that defines tailscaleUp() so it is available in the
Settings page (ensure the function is attached to the global/window scope or
exported/imported appropriately and avoid duplicate definitions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tailscaleUp() is defined in src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php, which is within the page's scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the page system loads files and shares scope
echo "=== Check all .page files in the plugin ==="
find src/usr/local/emhttp/plugins/tailscale -name "*.page" -type f -exec sh -c 'echo "File: $1"; cat "$1"; echo' _ {} \;
echo -e "\n=== Check if Settings is loaded within Tailscale page ==="
cat src/usr/local/emhttp/plugins/tailscale/include/page.php
echo -e "\n=== Check main Tailscale.php to see if it includes Settings.php ==="
grep -n "Settings.php\|include\|require" src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php | head -20
echo -e "\n=== Check directory listing ==="
ls -la src/usr/local/emhttp/plugins/tailscale/include/Pages/
echo -e "\n=== Verify tailscaleUp is indeed in Tailscale.php ==="
grep -n "function tailscaleUp" src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php
Length of output: 6818
@dkaser Thank you for the clarification! I see now that in Unraid's tabbed page system, the Settings page (loaded via Tailscale-1-Settings.page) shares the JavaScript scope with the main Tailscale page (loaded via Tailscale.page with Tabs="true"). This means the tailscaleUp() function defined in Tailscale.php is indeed available when the reauthenticate button in Settings.php is clicked.
I apologize for the confusion—I wasn't familiar with how Unraid's tabbed plugin pages share their JavaScript context. The change from expireTailscaleKeyNow() to tailscaleUp() should work correctly. 👍
✏️ Learnings added
Learnt from: dkaser
Repo: unraid/unraid-tailscale PR: 66
File: src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php:218-218
Timestamp: 2025-11-11T00:22:27.107Z
Learning: In the unraid-tailscale plugin, pages with `Tabs="true"` (like Tailscale.page) and their submenu pages (like Tailscale-1-Settings.page) share the same JavaScript scope. JavaScript functions defined in Tailscale.php are accessible in Settings.php because they load in the same browser context.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Summary by CodeRabbit