Skip to content

Commit bda719d

Browse files
newbdez33kradalby
andcommitted
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>
1 parent 835db97 commit bda719d

4 files changed

Lines changed: 61 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ internet is a security-sensitive choice. `autogroup:danger-all` can only be used
8686
- Fix exit node approval not triggering filter rule recalculation for peers [#2180](https://github.com/juanfont/headscale/pull/2180)
8787
- Policy validation error messages now include field context (e.g., `src=`, `dst=`) and are more descriptive [#2180](https://github.com/juanfont/headscale/pull/2180)
8888

89+
## 0.28.1 (202x-xx-xx)
90+
91+
### Changes
92+
93+
- **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)
94+
8995
## 0.28.0 (2026-02-04)
9096

9197
**Minimum supported Tailscale client version: v1.74.0**

hscontrol/db/preauth_keys.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,18 @@ func ListPreAuthKeys(tx *gorm.DB) ([]types.PreAuthKey, error) {
170170
return keys, nil
171171
}
172172

173+
// ListPreAuthKeysByUser returns all PreAuthKeys belonging to a specific user.
174+
func ListPreAuthKeysByUser(tx *gorm.DB, uid types.UserID) ([]types.PreAuthKey, error) {
175+
var keys []types.PreAuthKey
176+
177+
err := tx.Preload("User").Where("user_id = ?", uint(uid)).Find(&keys).Error
178+
if err != nil {
179+
return nil, err
180+
}
181+
182+
return keys, nil
183+
}
184+
173185
var (
174186
ErrPreAuthKeyFailedToParse = errors.New("failed to parse auth-key")
175187
ErrPreAuthKeyNotTaggedOrOwned = errors.New("auth-key must be either tagged or owned by user")

hscontrol/db/users.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func DestroyUser(tx *gorm.DB, uid types.UserID) error {
6565
return ErrUserStillHasNodes
6666
}
6767

68-
keys, err := ListPreAuthKeys(tx)
68+
keys, err := ListPreAuthKeysByUser(tx, uid)
6969
if err != nil {
7070
return err
7171
}

hscontrol/db/users_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,48 @@ func TestDestroyUserErrors(t *testing.T) {
160160
require.ErrorIs(t, err, ErrUserStillHasNodes)
161161
},
162162
},
163+
{
164+
// Regression test for https://github.com/juanfont/headscale/issues/3154
165+
// DestroyUser must only delete the target user's pre-auth keys,
166+
// not all pre-auth keys in the database.
167+
name: "success_only_deletes_own_preauthkeys",
168+
test: func(t *testing.T, db *HSDatabase) {
169+
t.Helper()
170+
171+
userA := db.CreateUserForTest("usera")
172+
userB := db.CreateUserForTest("userb")
173+
174+
// Create 2 keys for userA, 1 key for userB.
175+
_, err := db.CreatePreAuthKey(userA.TypedID(), false, false, nil, nil)
176+
require.NoError(t, err)
177+
_, err = db.CreatePreAuthKey(userA.TypedID(), false, false, nil, nil)
178+
require.NoError(t, err)
179+
_, err = db.CreatePreAuthKey(userB.TypedID(), false, false, nil, nil)
180+
require.NoError(t, err)
181+
182+
// Sanity check: 3 keys exist.
183+
allKeys, err := db.ListPreAuthKeys()
184+
require.NoError(t, err)
185+
require.Len(t, allKeys, 3)
186+
187+
// Delete userB.
188+
err = db.DestroyUser(types.UserID(userB.ID))
189+
require.NoError(t, err)
190+
191+
// Only userA's 2 keys should remain.
192+
remaining, err := db.ListPreAuthKeys()
193+
require.NoError(t, err)
194+
assert.Len(t, remaining, 2,
195+
"expected 2 keys for userA, got %d — DestroyUser deleted keys from other users",
196+
len(remaining))
197+
198+
for _, key := range remaining {
199+
assert.NotNil(t, key.UserID)
200+
assert.Equal(t, userA.ID, *key.UserID,
201+
"remaining key should belong to userA")
202+
}
203+
},
204+
},
163205
}
164206

165207
for _, tt := range tests {

0 commit comments

Comments
 (0)