Skip to content

Conversation

@dkaser
Copy link
Collaborator

@dkaser dkaser commented Nov 10, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Fixed the reauthenticate button so it reliably triggers the intended reauthentication action and removed the now-unused legacy behavior.
  • Documentation
    • Shortened and clarified the reauthenticate button description to a concise "Force a Tailscale reauthentication."

@github-actions github-actions bot added the feat label Nov 10, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 10, 2025

Walkthrough

The reauthenticate button's client-side handler was changed from expireTailscaleKeyNow() to tailscaleUp(). The English locale text for "reauthenticate" was shortened to a single concise sentence.

Changes

Cohort / File(s) Summary
Reauthenticate button handler
src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php
Replaced onclick handler on the reauthenticate button: expireTailscaleKeyNow()tailscaleUp(); removed the now-unused expireTailscaleKeyNow() function and its AJAX call.
Locale string update
src/usr/local/emhttp/plugins/tailscale/locales/en_US.json
Shortened the reauthenticate message to a concise single-line description: "Force a Tailscale reauthentication."

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)
Loading

Note: diagram shows new control flow invoking tailscaleUp() instead of the removed expireTailscaleKeyNow().

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Focus review on:
    • Settings.php to confirm expireTailscaleKeyNow() removal did not leave references.
    • en_US.json to validate the updated string key and punctuation.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'seamless reauthentication' directly describes the main change: replacing expireTailscaleKeyNow() with tailscaleUp() to enable seamless reauthentication without manual key expiration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/seamless-reauth

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6bd5e6d and 5632545.

📒 Files selected for processing (1)
  • src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Derek Kaser <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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 use tailscaleUp(). 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

📥 Commits

Reviewing files that changed from the base of the PR and between f865b17 and 6bd5e6d.

📒 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>
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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.php

Length 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 -5

Length 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.php

Length 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 f

Length 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 -50

Length 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).

Copy link
Collaborator Author

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.

Copy link

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.

@dkaser dkaser merged commit 3fd803e into trunk Nov 11, 2025
6 checks passed
@dkaser dkaser deleted the feat/seamless-reauth branch November 11, 2025 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants