mirror of
https://github.com/juanfont/headscale.git
synced 2026-04-23 00:58:43 +02:00
db: scope DestroyUser to only delete the target user's pre-auth keys
DestroyUser called ListPreAuthKeys(tx) which returns ALL pre-auth keys across all users, then deleted every one of them. This caused deleting any single user to wipe out pre-auth keys for every other user. Extract a ListPreAuthKeysByUser function (consistent with the existing ListNodesByUser pattern) and use it in DestroyUser to scope key deletion to the user being destroyed. Add unit test (table-driven in TestDestroyUserErrors) and integration test to prevent regression. Fixes #3154 Co-authored-by: Kristoffer Dalby <kristoffer@dalby.cc>
This commit is contained in:
@@ -90,6 +90,12 @@ internet is a security-sensitive choice. `autogroup:danger-all` can only be used
|
|||||||
- Fix exit node approval not triggering filter rule recalculation for peers [#2180](https://github.com/juanfont/headscale/pull/2180)
|
- Fix exit node approval not triggering filter rule recalculation for peers [#2180](https://github.com/juanfont/headscale/pull/2180)
|
||||||
- Policy validation error messages now include field context (e.g., `src=`, `dst=`) and are more descriptive [#2180](https://github.com/juanfont/headscale/pull/2180)
|
- Policy validation error messages now include field context (e.g., `src=`, `dst=`) and are more descriptive [#2180](https://github.com/juanfont/headscale/pull/2180)
|
||||||
|
|
||||||
|
## 0.28.1 (202x-xx-xx)
|
||||||
|
|
||||||
|
### Changes
|
||||||
|
|
||||||
|
- **User deletion**: Fix `DestroyUser` deleting all pre-auth keys in the database instead of only the target user's keys [#3155](https://github.com/juanfont/headscale/pull/3155)
|
||||||
|
|
||||||
## 0.28.0 (2026-02-04)
|
## 0.28.0 (2026-02-04)
|
||||||
|
|
||||||
**Minimum supported Tailscale client version: v1.74.0**
|
**Minimum supported Tailscale client version: v1.74.0**
|
||||||
|
|||||||
@@ -170,6 +170,18 @@ func ListPreAuthKeys(tx *gorm.DB) ([]types.PreAuthKey, error) {
|
|||||||
return keys, nil
|
return keys, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ListPreAuthKeysByUser returns all PreAuthKeys belonging to a specific user.
|
||||||
|
func ListPreAuthKeysByUser(tx *gorm.DB, uid types.UserID) ([]types.PreAuthKey, error) {
|
||||||
|
var keys []types.PreAuthKey
|
||||||
|
|
||||||
|
err := tx.Preload("User").Where("user_id = ?", uint(uid)).Find(&keys).Error
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
return keys, nil
|
||||||
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
ErrPreAuthKeyFailedToParse = errors.New("failed to parse auth-key")
|
ErrPreAuthKeyFailedToParse = errors.New("failed to parse auth-key")
|
||||||
ErrPreAuthKeyNotTaggedOrOwned = errors.New("auth-key must be either tagged or owned by user")
|
ErrPreAuthKeyNotTaggedOrOwned = errors.New("auth-key must be either tagged or owned by user")
|
||||||
|
|||||||
@@ -65,7 +65,7 @@ func DestroyUser(tx *gorm.DB, uid types.UserID) error {
|
|||||||
return ErrUserStillHasNodes
|
return ErrUserStillHasNodes
|
||||||
}
|
}
|
||||||
|
|
||||||
keys, err := ListPreAuthKeys(tx)
|
keys, err := ListPreAuthKeysByUser(tx, uid)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -160,6 +160,48 @@ func TestDestroyUserErrors(t *testing.T) {
|
|||||||
require.ErrorIs(t, err, ErrUserStillHasNodes)
|
require.ErrorIs(t, err, ErrUserStillHasNodes)
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
// Regression test for https://github.com/juanfont/headscale/issues/3154
|
||||||
|
// DestroyUser must only delete the target user's pre-auth keys,
|
||||||
|
// not all pre-auth keys in the database.
|
||||||
|
name: "success_only_deletes_own_preauthkeys",
|
||||||
|
test: func(t *testing.T, db *HSDatabase) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
userA := db.CreateUserForTest("usera")
|
||||||
|
userB := db.CreateUserForTest("userb")
|
||||||
|
|
||||||
|
// Create 2 keys for userA, 1 key for userB.
|
||||||
|
_, err := db.CreatePreAuthKey(userA.TypedID(), false, false, nil, nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
_, err = db.CreatePreAuthKey(userA.TypedID(), false, false, nil, nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
_, err = db.CreatePreAuthKey(userB.TypedID(), false, false, nil, nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Sanity check: 3 keys exist.
|
||||||
|
allKeys, err := db.ListPreAuthKeys()
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Len(t, allKeys, 3)
|
||||||
|
|
||||||
|
// Delete userB.
|
||||||
|
err = db.DestroyUser(types.UserID(userB.ID))
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Only userA's 2 keys should remain.
|
||||||
|
remaining, err := db.ListPreAuthKeys()
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Len(t, remaining, 2,
|
||||||
|
"expected 2 keys for userA, got %d — DestroyUser deleted keys from other users",
|
||||||
|
len(remaining))
|
||||||
|
|
||||||
|
for _, key := range remaining {
|
||||||
|
assert.NotNil(t, key.UserID)
|
||||||
|
assert.Equal(t, userA.ID, *key.UserID,
|
||||||
|
"remaining key should belong to userA")
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|||||||
Reference in New Issue
Block a user