chenchenandClaude Opus 4.7
5081289b65
User feedback: "都已经已撤销了为什么还有记录" — once a user clicks
revoke they expect the row gone from the list, not lingering with
a "已撤销" badge. The old behaviour treated the page as a security
audit log, which conflicts with its primary use as an active-device
management surface.
GetUserDeviceBoundTokens now filters `revoked_at = 0`. The row stays
in DB (soft-delete) so:
- audit trail (RevokedAt / RevokedReason / DeviceLastSeenIp /
DeviceFingerprint) remains inspectable by admins
- re-pair from the same physical device still self-heals the row
via the reactivate branch in PairDevice — covered by existing
TestPairDevice_RepairAfterRevokeReactivates
If a user wants security audit history in the UI, that should be a
separate "Security activity" page; not the device-management list.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
484 lines
15 KiB
Go
484 lines
15 KiB
Go
package controller
|
|
|
|
import (
|
|
"crypto/ed25519"
|
|
"crypto/rand"
|
|
"crypto/sha256"
|
|
"encoding/base64"
|
|
"encoding/hex"
|
|
"net/http"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
"unicode/utf8"
|
|
|
|
"github.com/gin-gonic/gin"
|
|
|
|
"github.com/heicode/manager/common"
|
|
"github.com/heicode/manager/model"
|
|
)
|
|
|
|
// TestPairDevice covers the re-pair self-heal path that the V2 device-
|
|
// binding flow depends on. Three behaviours under test:
|
|
//
|
|
// 1. First-time pair on a fresh user: inserts a row, returns 200 with
|
|
// `reused:false`.
|
|
// 2. Re-pair from the same physical device (same device_id + same
|
|
// pubkey) while the original row is still active: 200 with
|
|
// `reused:true` and no row mutation.
|
|
// 3. Re-pair after the user revoked the device in the Web UI: 200 with
|
|
// `reused:true, reactivated:true`. The row's revoked_at / status
|
|
// MUST be cleared, otherwise the client receives 200 from pair but
|
|
// every subsequent V2 request fails with ErrDeviceRevoked.
|
|
//
|
|
// Pubkey-mismatch (409) and revoke-with-cap-saturation (403) are covered
|
|
// as separate sub-tests.
|
|
|
|
func newTestPubkey(t *testing.T) string {
|
|
t.Helper()
|
|
pub, _, err := ed25519.GenerateKey(rand.Reader)
|
|
if err != nil {
|
|
t.Fatalf("ed25519 generate: %v", err)
|
|
}
|
|
return base64.StdEncoding.EncodeToString(pub)
|
|
}
|
|
|
|
func newTestFingerprint() string {
|
|
// `isValidPairRequest` requires exactly 64 hex chars (sha256).
|
|
sum := sha256.Sum256([]byte("test-machine"))
|
|
return hex.EncodeToString(sum[:])
|
|
}
|
|
|
|
func pairDeviceBody(deviceId, pubkey string) map[string]any {
|
|
return map[string]any{
|
|
"device_id": deviceId,
|
|
"public_key": pubkey,
|
|
"fingerprint": newTestFingerprint(),
|
|
"device_name": "Test MacBook",
|
|
"platform": "darwin",
|
|
"app_version": "0.3.3",
|
|
}
|
|
}
|
|
|
|
func TestPairDevice_FirstPairInserts(t *testing.T) {
|
|
setupTokenControllerTestDB(t)
|
|
|
|
pubkey := newTestPubkey(t)
|
|
deviceId := "dev-test-first"
|
|
const userID = 42
|
|
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx)
|
|
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String())
|
|
}
|
|
|
|
var got tokenAPIResponse
|
|
if err := common.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
|
t.Fatalf("unmarshal response: %v", err)
|
|
}
|
|
if !got.Success {
|
|
t.Fatalf("success=false: %s", got.Message)
|
|
}
|
|
|
|
// Row sanity-check: device_id + pubkey persisted, not revoked.
|
|
var stored model.Token
|
|
if err := model.DB.Where("user_id = ? AND device_id = ?", userID, deviceId).
|
|
First(&stored).Error; err != nil {
|
|
t.Fatalf("token not found in DB: %v", err)
|
|
}
|
|
if stored.RevokedAt != 0 {
|
|
t.Fatalf("freshly-paired row should not be revoked, got revoked_at=%d", stored.RevokedAt)
|
|
}
|
|
if stored.DevicePubkey == nil || *stored.DevicePubkey != pubkey {
|
|
t.Fatalf("device_pubkey mismatch")
|
|
}
|
|
}
|
|
|
|
func TestPairDevice_RepairActiveReturnsReused(t *testing.T) {
|
|
setupTokenControllerTestDB(t)
|
|
|
|
pubkey := newTestPubkey(t)
|
|
deviceId := "dev-test-repair"
|
|
const userID = 43
|
|
|
|
// First pair.
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("first pair: %d %s", rec.Code, rec.Body.String())
|
|
}
|
|
|
|
// Second pair, same inputs.
|
|
ctx2, rec2 := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx2)
|
|
|
|
if rec2.Code != http.StatusOK {
|
|
t.Fatalf("re-pair expected 200, got %d %s", rec2.Code, rec2.Body.String())
|
|
}
|
|
|
|
var got struct {
|
|
Success bool `json:"success"`
|
|
Data struct {
|
|
Reused bool `json:"reused"`
|
|
Reactivated bool `json:"reactivated"`
|
|
} `json:"data"`
|
|
}
|
|
if err := common.Unmarshal(rec2.Body.Bytes(), &got); err != nil {
|
|
t.Fatalf("unmarshal: %v", err)
|
|
}
|
|
if !got.Data.Reused {
|
|
t.Fatalf("expected reused=true on identical re-pair")
|
|
}
|
|
if got.Data.Reactivated {
|
|
t.Fatalf("expected reactivated=false on non-revoked re-pair")
|
|
}
|
|
}
|
|
|
|
func TestPairDevice_RepairAfterRevokeReactivates(t *testing.T) {
|
|
// THIS is the regression the controller fix targets. Before the
|
|
// fix, the dup-check branch returned 200 reused:true without
|
|
// touching revoked_at — leaving the client believing pair
|
|
// succeeded while every V2 request still 401'd.
|
|
setupTokenControllerTestDB(t)
|
|
|
|
pubkey := newTestPubkey(t)
|
|
deviceId := "dev-test-revoked-repair"
|
|
const userID = 44
|
|
|
|
// First pair.
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("first pair: %d", rec.Code)
|
|
}
|
|
|
|
// Look up + revoke the row directly.
|
|
var stored model.Token
|
|
if err := model.DB.Where("user_id = ? AND device_id = ?", userID, deviceId).
|
|
First(&stored).Error; err != nil {
|
|
t.Fatalf("locate row: %v", err)
|
|
}
|
|
if err := model.RevokeDevice(stored.Id, "user_initiated"); err != nil {
|
|
t.Fatalf("revoke: %v", err)
|
|
}
|
|
|
|
// Verify the revoke landed.
|
|
var after model.Token
|
|
if err := model.DB.First(&after, stored.Id).Error; err != nil {
|
|
t.Fatalf("re-lookup: %v", err)
|
|
}
|
|
if after.RevokedAt == 0 {
|
|
t.Fatalf("revoke didn't set revoked_at")
|
|
}
|
|
|
|
// Same device coming back online with the same identity.
|
|
ctx2, rec2 := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx2)
|
|
|
|
if rec2.Code != http.StatusOK {
|
|
t.Fatalf("re-pair after revoke expected 200, got %d %s",
|
|
rec2.Code, rec2.Body.String())
|
|
}
|
|
|
|
var got struct {
|
|
Success bool `json:"success"`
|
|
Data struct {
|
|
Reused bool `json:"reused"`
|
|
Reactivated bool `json:"reactivated"`
|
|
} `json:"data"`
|
|
}
|
|
if err := common.Unmarshal(rec2.Body.Bytes(), &got); err != nil {
|
|
t.Fatalf("unmarshal: %v", err)
|
|
}
|
|
if !got.Data.Reused || !got.Data.Reactivated {
|
|
t.Fatalf("expected reused=true reactivated=true, got %+v", got.Data)
|
|
}
|
|
|
|
// Row state: revoked_at cleared, status back to enabled.
|
|
var healed model.Token
|
|
if err := model.DB.First(&healed, stored.Id).Error; err != nil {
|
|
t.Fatalf("post-reactivate lookup: %v", err)
|
|
}
|
|
if healed.RevokedAt != 0 {
|
|
t.Fatalf("reactivate should clear revoked_at, got %d", healed.RevokedAt)
|
|
}
|
|
if healed.RevokedReason != "" {
|
|
t.Fatalf("reactivate should clear revoked_reason, got %q", healed.RevokedReason)
|
|
}
|
|
if healed.Status != common.TokenStatusEnabled {
|
|
t.Fatalf("reactivate should set status=enabled, got %d", healed.Status)
|
|
}
|
|
}
|
|
|
|
func TestPairDevice_PubkeyMismatchConflicts(t *testing.T) {
|
|
setupTokenControllerTestDB(t)
|
|
|
|
const userID = 45
|
|
deviceId := "dev-test-pubkey-mismatch"
|
|
|
|
// First pair with pubkey A.
|
|
keyA := newTestPubkey(t)
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, keyA), userID)
|
|
PairDevice(ctx)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("first pair: %d", rec.Code)
|
|
}
|
|
|
|
// Re-pair with a fresh pubkey B → 409.
|
|
keyB := newTestPubkey(t)
|
|
ctx2, rec2 := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, keyB), userID)
|
|
PairDevice(ctx2)
|
|
|
|
if rec2.Code != http.StatusConflict {
|
|
t.Fatalf("expected 409, got %d %s", rec2.Code, rec2.Body.String())
|
|
}
|
|
}
|
|
|
|
// pairExistingDevice is a helper for rename tests — it does a clean pair
|
|
// and returns the inserted row's id so the rename test can address it
|
|
// directly without parsing the response body.
|
|
func pairExistingDevice(t *testing.T, userID int, deviceId string) int {
|
|
t.Helper()
|
|
pubkey := newTestPubkey(t)
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPost,
|
|
"/api/devices/pair", pairDeviceBody(deviceId, pubkey), userID)
|
|
PairDevice(ctx)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("seed pair failed: %d %s", rec.Code, rec.Body.String())
|
|
}
|
|
var stored model.Token
|
|
if err := model.DB.Where("user_id = ? AND device_id = ?", userID, deviceId).
|
|
First(&stored).Error; err != nil {
|
|
t.Fatalf("seed lookup: %v", err)
|
|
}
|
|
return stored.Id
|
|
}
|
|
|
|
// callRename invokes RenameUserDevice against an in-memory request and
|
|
// sets the :id URL param Gin would normally populate from the route.
|
|
// Returns the recorder so the test can assert on the HTTP status.
|
|
func callRename(t *testing.T, userID, tokenID int, body map[string]any) int {
|
|
t.Helper()
|
|
target := "/api/devices/" + strconv.Itoa(tokenID)
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodPatch, target, body, userID)
|
|
ctx.Params = gin.Params{{Key: "id", Value: strconv.Itoa(tokenID)}}
|
|
RenameUserDevice(ctx)
|
|
return rec.Code
|
|
}
|
|
|
|
func TestRenameUserDevice_RejectsEmptyName(t *testing.T) {
|
|
// Trim-to-empty must 400 instead of wiping device_name to ''. Before
|
|
// the fix, " " (whitespace) silently set device_name = '' and the
|
|
// row appeared as "Unnamed device" in the Devices list.
|
|
setupTokenControllerTestDB(t)
|
|
const userID = 51
|
|
tokenID := pairExistingDevice(t, userID, "dev-rename-empty")
|
|
|
|
code := callRename(t, userID, tokenID, map[string]any{
|
|
"device_name": " ",
|
|
})
|
|
if code != http.StatusBadRequest {
|
|
t.Fatalf("expected 400 on whitespace-only rename, got %d", code)
|
|
}
|
|
|
|
// Row should be unchanged (still "Test MacBook" from seed pair).
|
|
var after model.Token
|
|
if err := model.DB.First(&after, tokenID).Error; err != nil {
|
|
t.Fatalf("post-rename lookup: %v", err)
|
|
}
|
|
if after.DeviceName == "" {
|
|
t.Fatalf("device_name was wiped to empty despite rejection")
|
|
}
|
|
}
|
|
|
|
func TestRenameUserDevice_TruncatesByRune(t *testing.T) {
|
|
// "陈" is 3 bytes / 1 rune in UTF-8. A name of 100 "陈"s is 300 bytes /
|
|
// 100 runes; truncating by byte at 64 would land mid-codepoint and
|
|
// produce invalid UTF-8. Rune-aware truncation cleanly cuts at 64.
|
|
setupTokenControllerTestDB(t)
|
|
const userID = 52
|
|
tokenID := pairExistingDevice(t, userID, "dev-rename-runes")
|
|
|
|
longName := strings.Repeat("陈", 100)
|
|
code := callRename(t, userID, tokenID, map[string]any{
|
|
"device_name": longName,
|
|
})
|
|
if code != http.StatusOK {
|
|
t.Fatalf("expected 200 on long-name rename, got %d", code)
|
|
}
|
|
|
|
var after model.Token
|
|
if err := model.DB.First(&after, tokenID).Error; err != nil {
|
|
t.Fatalf("lookup: %v", err)
|
|
}
|
|
if !utf8.ValidString(after.DeviceName) {
|
|
t.Fatalf("device_name not valid UTF-8 after rename: %q", after.DeviceName)
|
|
}
|
|
if utf8.RuneCountInString(after.DeviceName) != 64 {
|
|
t.Fatalf("expected exactly 64 runes after truncate, got %d",
|
|
utf8.RuneCountInString(after.DeviceName))
|
|
}
|
|
}
|
|
|
|
func TestListUserDevices_NeverUsedSortsByBoundAt(t *testing.T) {
|
|
// Sort rule (H): a device that hasn't been used since pair
|
|
// (last_used_at=0) MUST fall back to bound_at — otherwise the
|
|
// device the user just paired sinks to the bottom of the Devices
|
|
// list behind an older but actively-used machine, which looks
|
|
// like "my pair didn't work" to the user.
|
|
//
|
|
// Scenario:
|
|
// - device A: paired 1000 ms ago, never used (last_used=0)
|
|
// - device B: paired 5000 ms ago, used 500ms ago (last_used=500)
|
|
// - device C: paired 3000 ms ago, used 200ms ago (last_used=200)
|
|
// Old order (last_used DESC, id DESC): B, C, A
|
|
// New order (COALESCE-equivalent): A(1000), B(500), C(200)
|
|
setupTokenControllerTestDB(t)
|
|
const userID = 60
|
|
|
|
mkDev := func(deviceId string, boundAt, lastUsed int64) {
|
|
t.Helper()
|
|
dev := deviceId
|
|
key := "pk-" + deviceId
|
|
tok := model.Token{
|
|
UserId: userID,
|
|
Name: "device:" + deviceId,
|
|
Key: "tk-" + deviceId,
|
|
Status: common.TokenStatusEnabled,
|
|
ExpiredTime: -1,
|
|
UnlimitedQuota: true,
|
|
HideFromUserUI: true,
|
|
DeviceId: &dev,
|
|
DevicePubkey: &key,
|
|
DeviceBoundAt: boundAt,
|
|
DeviceLastUsedAt: lastUsed,
|
|
}
|
|
if err := model.DB.Create(&tok).Error; err != nil {
|
|
t.Fatalf("seed %s: %v", deviceId, err)
|
|
}
|
|
}
|
|
mkDev("A-never-used", 1000, 0)
|
|
mkDev("B-old-used", 5000, 500)
|
|
mkDev("C-mid-used", 3000, 200)
|
|
|
|
got, err := model.GetUserDeviceBoundTokens(userID)
|
|
if err != nil {
|
|
t.Fatalf("list: %v", err)
|
|
}
|
|
if len(got) != 3 {
|
|
t.Fatalf("expected 3 rows, got %d", len(got))
|
|
}
|
|
wantOrder := []string{"A-never-used", "B-old-used", "C-mid-used"}
|
|
for i, want := range wantOrder {
|
|
if got[i].DeviceId == nil || *got[i].DeviceId != want {
|
|
t.Errorf("position %d: want device_id=%q, got %v", i, want, got[i].DeviceId)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestListUserDevices_HidesRevokedRows(t *testing.T) {
|
|
// User feedback: "已撤销了为什么还有记录" — once a user revokes a
|
|
// device they expect it gone from the Devices list. We keep the
|
|
// soft-deleted row in DB for audit + so re-pair can heal it
|
|
// (TestPairDevice_RepairAfterRevokeReactivates), but it must NOT
|
|
// appear in GetUserDeviceBoundTokens results.
|
|
setupTokenControllerTestDB(t)
|
|
const userID = 70
|
|
|
|
// Seed: 1 active + 1 revoked, both belonging to the same user.
|
|
activeId := "dev-active"
|
|
revokedId := "dev-revoked"
|
|
for _, did := range []string{activeId, revokedId} {
|
|
d := did
|
|
k := "pk-" + did
|
|
if err := model.DB.Create(&model.Token{
|
|
UserId: userID,
|
|
Name: "device:" + did,
|
|
Key: "tk-" + did,
|
|
Status: common.TokenStatusEnabled,
|
|
ExpiredTime: -1,
|
|
UnlimitedQuota: true,
|
|
HideFromUserUI: true,
|
|
DeviceId: &d,
|
|
DevicePubkey: &k,
|
|
DeviceBoundAt: 1000,
|
|
}).Error; err != nil {
|
|
t.Fatalf("seed %s: %v", did, err)
|
|
}
|
|
}
|
|
// Revoke the second row.
|
|
var revoked model.Token
|
|
if err := model.DB.Where("device_id = ?", revokedId).First(&revoked).Error; err != nil {
|
|
t.Fatalf("lookup revoked: %v", err)
|
|
}
|
|
if err := model.RevokeDevice(revoked.Id, "user_initiated"); err != nil {
|
|
t.Fatalf("revoke: %v", err)
|
|
}
|
|
|
|
got, err := model.GetUserDeviceBoundTokens(userID)
|
|
if err != nil {
|
|
t.Fatalf("list: %v", err)
|
|
}
|
|
if len(got) != 1 {
|
|
t.Fatalf("expected 1 active row (revoked filtered out), got %d", len(got))
|
|
}
|
|
if got[0].DeviceId == nil || *got[0].DeviceId != activeId {
|
|
t.Fatalf("wrong row surfaced: got %v want %s", got[0].DeviceId, activeId)
|
|
}
|
|
|
|
// Sanity: the revoked row is still in DB, just hidden from this query.
|
|
var stillThere model.Token
|
|
if err := model.DB.First(&stillThere, revoked.Id).Error; err != nil {
|
|
t.Fatalf("revoked row was hard-deleted (should be soft only): %v", err)
|
|
}
|
|
if stillThere.RevokedAt == 0 {
|
|
t.Fatalf("revoked_at unexpectedly cleared")
|
|
}
|
|
}
|
|
|
|
func TestRevokeUserDevice_LogsAudit(t *testing.T) {
|
|
// Revoke must leave an audit breadcrumb. We don't assert the exact
|
|
// log line format (it's stable but the tests are noisy enough already
|
|
// — a SysLog grep is enough for the human reviewer). Here we just
|
|
// confirm the row state transition + the audit-relevant fields
|
|
// (DeviceId / DeviceName) are still readable post-revoke so the
|
|
// SysLog write doesn't crash on nil deref.
|
|
setupTokenControllerTestDB(t)
|
|
const userID = 61
|
|
tokenID := pairExistingDevice(t, userID, "dev-revoke-audit")
|
|
|
|
target := "/api/devices/" + strconv.Itoa(tokenID)
|
|
ctx, rec := newAuthenticatedContext(t, http.MethodDelete, target, nil, userID)
|
|
ctx.Params = gin.Params{{Key: "id", Value: strconv.Itoa(tokenID)}}
|
|
RevokeUserDevice(ctx)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("expected 200, got %d %s", rec.Code, rec.Body.String())
|
|
}
|
|
|
|
var after model.Token
|
|
if err := model.DB.First(&after, tokenID).Error; err != nil {
|
|
t.Fatalf("post-revoke lookup: %v", err)
|
|
}
|
|
if after.RevokedAt == 0 {
|
|
t.Fatalf("revoked_at not set")
|
|
}
|
|
if after.Status != common.TokenStatusDisabled {
|
|
t.Fatalf("status not disabled, got %d", after.Status)
|
|
}
|
|
if after.RevokedReason != "user_initiated" {
|
|
t.Fatalf("reason not user_initiated, got %q", after.RevokedReason)
|
|
}
|
|
if after.DeviceId == nil || *after.DeviceId == "" {
|
|
t.Fatalf("device_id should remain readable post-revoke for audit")
|
|
}
|
|
}
|