do not allow preauth keys to be deleted if assigned to node (#2396)

* do not allow preauth keys to be deleted if assigned to node Signed-off-by: Kristoffer Dalby <[email protected]> * update changelog Signed-off-by: Kristoffer Dalby <[email protected]> --------- Signed-off-by: Kristoffer Dalby <[email protected]>
juanfont · Feb 1, 2025 · 9bd1438 · 9bd1438
1 parent d57a55c
commit 9bd1438
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,10 @@
   [#2350](https://github.com/juanfont/headscale/pull/2350)
 - Print Tailscale version instead of capability versions for outdated nodes
   [#2391](https://github.com/juanfont/headscale/pull/2391)
+- Pre auth keys belonging to a user are no longer deleted with the user
+  [#2396](https://github.com/juanfont/headscale/pull/2396)
+- Pre auth keys that are used by a node can no longer be deleted
+  [#2396](https://github.com/juanfont/headscale/pull/2396)
 
 ## 0.24.2 (2025-01-30)
 

diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go
@@ -582,6 +582,24 @@ COMMIT;
 				},
 				Rollback: func(db *gorm.DB) error { return nil },
 			},
+			// Add back constraint so you cannot delete preauth keys that
+			// is still used by a node.
+			{
+				ID: "202501311657",
+				Migrate: func(tx *gorm.DB) error {
+					err := tx.AutoMigrate(&types.PreAuthKey{})
+					if err != nil {
+						return err
+					}
+					err = tx.AutoMigrate(&types.Node{})
+					if err != nil {
+						return err
+					}
+
+					return nil
+				},
+				Rollback: func(db *gorm.DB) error { return nil },
+			},
 		},
 	)
 

diff --git a/hscontrol/db/preauth_keys_test.go b/hscontrol/db/preauth_keys_test.go
@@ -2,10 +2,13 @@ package db
 
 import (
 	"sort"
+	"testing"
 	"time"
 
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/check.v1"
 	"tailscale.com/types/ptr"
 )
@@ -175,3 +178,25 @@ func (*Suite) TestPreAuthKeyACLTags(c *check.C) {
 	sort.Sort(sort.StringSlice(gotTags))
 	c.Assert(gotTags, check.DeepEquals, tags)
 }
+
+func TestCannotDeleteAssignedPreAuthKey(t *testing.T) {
+	db, err := newSQLiteTestDB()
+	require.NoError(t, err)
+	user, err := db.CreateUser(types.User{Name: "test8"})
+	assert.NoError(t, err)
+
+	key, err := db.CreatePreAuthKey(types.UserID(user.ID), false, false, nil, []string{"tag:good"})
+	assert.NoError(t, err)
+
+	node := types.Node{
+		ID:             0,
+		Hostname:       "testest",
+		UserID:         user.ID,
+		RegisterMethod: util.RegisterMethodAuthKey,
+		AuthKeyID:      ptr.To(key.ID),
+	}
+	db.DB.Save(&node)
+
+	err = db.DB.Delete(key).Error
+	require.ErrorContains(t, err, "constraint failed: FOREIGN KEY constraint failed")
+}
diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go
@@ -77,9 +77,12 @@ type Node struct {
 
 	ForcedTags []string `gorm:"serializer:json"`
 
-	// TODO(kradalby): This seems like irrelevant information?
-	AuthKeyID *uint64     `sql:"DEFAULT:NULL"`
-	AuthKey   *PreAuthKey `gorm:"constraint:OnDelete:SET NULL;"`
+	// When a node has been created with a PreAuthKey, we need to
+	// prevent the preauthkey from being deleted before the node.
+	// The preauthkey can define "tags" of the node so we need it
+	// around.
+	AuthKeyID *uint64 `sql:"DEFAULT:NULL"`
+	AuthKey   *PreAuthKey
 
 	LastSeen *time.Time
 	Expiry   *time.Time

diff --git a/hscontrol/types/preauth_key.go b/hscontrol/types/preauth_key.go
@@ -14,7 +14,7 @@ type PreAuthKey struct {
 	ID        uint64 `gorm:"primary_key"`
 	Key       string
 	UserID    uint
-	User      User `gorm:"constraint:OnDelete:CASCADE;"`
+	User      User `gorm:"constraint:OnDelete:SET NULL;"`
 	Reusable  bool
 	Ephemeral bool     `gorm:"default:false"`
 	Used      bool     `gorm:"default:false"`