diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 398e657ccb8..514c96a0fe6 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -19,11 +19,11 @@ jobs: - TestACLNamedHostsCanReach - TestACLDevice1CanAccessDevice2 - TestPolicyUpdateWhileRunningWithCLIInDatabase + - TestAuthNodeApproval - TestOIDCAuthenticationPingAll - TestOIDCExpireNodesBasedOnTokenExpiry - TestAuthWebFlowAuthenticationPingAll - TestAuthWebFlowLogoutAndRelogin - - TestAuthNodeApproval - TestUserCommand - TestPreAuthKeyCommand - TestPreAuthKeyCommandWithoutExpiry diff --git a/cmd/headscale/cli/nodes.go b/cmd/headscale/cli/nodes.go index 8585c6c0ae3..845cbf8050b 100644 --- a/cmd/headscale/cli/nodes.go +++ b/cmd/headscale/cli/nodes.go @@ -51,7 +51,7 @@ func init() { approveNodeCmd.Flags().Uint64P("identifier", "i", 0, "Node identifier (ID)") err = approveNodeCmd.MarkFlagRequired("identifier") if err != nil { - log.Fatalf(err.Error()) + log.Fatalf("%v", err) } nodeCmd.AddCommand(approveNodeCmd) @@ -226,6 +226,7 @@ var approveNodeCmd = &cobra.Command{ fmt.Sprintf("Error converting ID to integer: %s", err), output, ) + return } ctx, client, conn, cancel := newHeadscaleCLIWithConfig() @@ -244,6 +245,7 @@ var approveNodeCmd = &cobra.Command{ ), output, ) + return } SuccessOutput(response.GetNode(), "Node approved", output) diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 9e4e9ee1d27..d25fbd6139e 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -313,7 +313,7 @@ func (h *Headscale) handleAuthKey( node.AuthKeyID = ptr.To(pak.ID) } - if node.Approved == false { + if !node.Approved { node.Approved = nodeApproved } diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index 9d156674110..d61a3f5b486 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -29,7 +29,10 @@ func init() { schema.RegisterSerializer("text", TextSerialiser{}) } -var errDatabaseNotSupported = errors.New("database type not supported") +var ( + errDatabaseNotSupported = errors.New("database type not supported") + errNoNodeApprovedColumnInDatabase = errors.New("no node approved column in database") +) // KV is a key-value store in a psql table. For future use... // TODO(kradalby): Is this used for anything? @@ -489,26 +492,26 @@ func NewHeadscaleDatabase( }, { ID: "202410071005", - Migrate: func(tx *gorm.DB) error { - err = tx.AutoMigrate(&types.PreAuthKey{}) + Migrate: func(db *gorm.DB) error { + err = db.AutoMigrate(&types.PreAuthKey{}) if err != nil { return err } - err = tx.AutoMigrate(&types.Node{}) + err = db.AutoMigrate(&types.Node{}) if err != nil { return err } - if tx.Migrator().HasColumn(&types.Node{}, "approved") { + if db.Migrator().HasColumn(&types.Node{}, "approved") { nodes := types.Nodes{} - if err := tx.Find(&nodes).Error; err != nil { + if err := db.Find(&nodes).Error; err != nil { log.Error().Err(err).Msg("Error accessing db") } for item, node := range nodes { - if node.IsApproved() == false { - err = tx.Model(nodes[item]).Updates(types.Node{ + if !node.IsApproved() { + err = db.Model(nodes[item]).Updates(types.Node{ Approved: true, }).Error if err != nil { @@ -525,7 +528,7 @@ func NewHeadscaleDatabase( return nil } - return fmt.Errorf("no node approved column in DB") + return errNoNodeApprovedColumnInDatabase }, Rollback: func(db *gorm.DB) error { return nil }, }, diff --git a/hscontrol/db/ip.go b/hscontrol/db/ip.go index b1289ab89af..afe33bbf3b0 100644 --- a/hscontrol/db/ip.go +++ b/hscontrol/db/ip.go @@ -265,17 +265,17 @@ func isTailscaleReservedIP(ip netip.Addr) bool { // it will be added. // If a prefix type has been removed (IPv4 or IPv6), it // will remove the IPs in that family from the node. -func (hsdb *HSDatabase) BackfillNodeIPs(i *IPAllocator) ([]string, error) { +func (hsdb *HSDatabase) BackfillNodeIPs(ip *IPAllocator) ([]string, error) { var err error var ret []string - err = hsdb.Write(func(tx *gorm.DB) error { - if i == nil { + err = hsdb.Write(func(db *gorm.DB) error { + if ip == nil { return errors.New("backfilling IPs: ip allocator was nil") } log.Trace().Msgf("starting to backfill IPs") - nodes, err := ListNodes(tx) + nodes, err := ListNodes(db) if err != nil { return fmt.Errorf("listing nodes to backfill IPs: %w", err) } @@ -285,8 +285,8 @@ func (hsdb *HSDatabase) BackfillNodeIPs(i *IPAllocator) ([]string, error) { changed := false // IPv4 prefix is set, but node ip is missing, alloc - if i.prefix4 != nil && node.IPv4 == nil { - ret4, err := i.nextLocked(i.prev4, i.prefix4) + if ip.prefix4 != nil && node.IPv4 == nil { + ret4, err := ip.nextLocked(ip.prev4, ip.prefix4) if err != nil { return fmt.Errorf("failed to allocate ipv4 for node(%d): %w", node.ID, err) } @@ -297,8 +297,8 @@ func (hsdb *HSDatabase) BackfillNodeIPs(i *IPAllocator) ([]string, error) { } // IPv6 prefix is set, but node ip is missing, alloc - if i.prefix6 != nil && node.IPv6 == nil { - ret6, err := i.nextLocked(i.prev6, i.prefix6) + if ip.prefix6 != nil && node.IPv6 == nil { + ret6, err := ip.nextLocked(ip.prev6, ip.prefix6) if err != nil { return fmt.Errorf("failed to allocate ipv6 for node(%d): %w", node.ID, err) } @@ -309,21 +309,21 @@ func (hsdb *HSDatabase) BackfillNodeIPs(i *IPAllocator) ([]string, error) { } // IPv4 prefix is not set, but node has IP, remove - if i.prefix4 == nil && node.IPv4 != nil { + if ip.prefix4 == nil && node.IPv4 != nil { ret = append(ret, fmt.Sprintf("removing IPv4 %q from Node(%d) %q", node.IPv4.String(), node.ID, node.Hostname)) node.IPv4 = nil changed = true } // IPv6 prefix is not set, but node has IP, remove - if i.prefix6 == nil && node.IPv6 != nil { + if ip.prefix6 == nil && node.IPv6 != nil { ret = append(ret, fmt.Sprintf("removing IPv6 %q from Node(%d) %q", node.IPv6.String(), node.ID, node.Hostname)) node.IPv6 = nil changed = true } if changed { - err := tx.Save(node).Error + err := db.Save(node).Error if err != nil { return fmt.Errorf("saving node(%d) after adding IPs: %w", node.ID, err) } diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index 96b6c406817..f1bf1e73137 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -268,7 +268,7 @@ func (hsdb *HSDatabase) NodeSetApprove(nodeID types.NodeID, approved bool) error }) } -// NodeSetApprove takes a Node struct and a set approval option +// NodeSetApprove takes a Node struct and a set approval option. func NodeSetApprove(tx *gorm.DB, nodeID types.NodeID, approved bool, ) error { @@ -371,7 +371,7 @@ func (hsdb *HSDatabase) RegisterNodeFromAuthCallback( node.User = *user node.RegisterMethod = registrationMethod - if node.IsApproved() == false && manualApprovedNode == false { + if !node.IsApproved() && manualApprovedNode == false { node.Approved = true } diff --git a/hscontrol/db/node_test.go b/hscontrol/db/node_test.go index 112079c2d15..4446726a282 100644 --- a/hscontrol/db/node_test.go +++ b/hscontrol/db/node_test.go @@ -202,7 +202,7 @@ func (s *Suite) TestListPeersWithoutNonAuthorized(c *check.C) { } node := types.Node{ - ID: types.NodeID(index), + ID: types.NodeID(int64(index)), MachineKey: machineKey.Public(), NodeKey: nodeKey.Public(), Hostname: "testnode" + strconv.Itoa(index), diff --git a/hscontrol/types/common.go b/hscontrol/types/common.go index 32ad8a67dbb..05a0b7090bd 100644 --- a/hscontrol/types/common.go +++ b/hscontrol/types/common.go @@ -15,6 +15,11 @@ const ( DatabaseSqlite = "sqlite3" ) +const ( + SchemaHttp = "http" + SchemaHttps = "https" +) + var ErrCannotParsePrefix = errors.New("cannot parse prefix") type StateUpdateType int diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 94c6b589283..860e1596233 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -112,7 +112,7 @@ func (node Node) IsExpired() bool { // IsApproved returns whether the node is approved. func (node Node) IsApproved() bool { - return node.Approved == true + return node.Approved } // IsEphemeral returns if the node is registered as an Ephemeral node. diff --git a/integration/auth_approval_test.go b/integration/auth_approval_test.go index e4359eb4c06..c9fd98a9e58 100644 --- a/integration/auth_approval_test.go +++ b/integration/auth_approval_test.go @@ -4,7 +4,8 @@ import ( "context" "crypto/tls" "fmt" - v1 "github.com/juanfont/headscale/gen/go/headscale/v1" + "github.com/juanfont/headscale/gen/go/headscale/v1" + "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/integration/hsic" "github.com/samber/lo" "github.com/stretchr/testify/assert" @@ -56,7 +57,7 @@ func TestAuthNodeApproval(t *testing.T) { status, err := client.Status() assertNoErr(t, err) assert.Equal(t, "NeedsMachineAuth", status.BackendState) - assert.Len(t, status.Peers(), 0) + assert.Empty(t, status.Peers()) } headscale, err := scenario.Headscale() @@ -74,11 +75,11 @@ func TestAuthNodeApproval(t *testing.T) { }, &allNodes, ) - assert.Nil(t, err) + assert.NoError(t, err) for _, node := range allNodes { _, err = headscale.Execute([]string{ - "headscale", "nodes", "approve", "--identifier", fmt.Sprintf("%d", node.Id), + "headscale", "nodes", "approve", "--identifier", fmt.Sprintf("%d", node.GetId()), }) assertNoErr(t, err) } @@ -113,7 +114,7 @@ func TestAuthNodeApproval(t *testing.T) { err = scenario.WaitForTailscaleSync() assertNoErrSync(t, err) - //assertClientsState(t, allClients) + // assertClientsState(t, allClients) allAddrs := lo.Map(allIps, func(x netip.Addr, index int) string { return x.String() @@ -236,11 +237,11 @@ func (s *AuthApprovalScenario) runHeadscaleRegister(userStr string, loginURL *ur } log.Printf("loginURL: %s", loginURL) - loginURL.Host = fmt.Sprintf("%s:8080", headscale.GetIP()) - loginURL.Scheme = "http" + loginURL.Host = fmt.Sprintf("%s:%d", headscale.GetIP(), 8080) + loginURL.Scheme = types.SchemaHttp if len(headscale.GetCert()) > 0 { - loginURL.Scheme = "https" + loginURL.Scheme = types.SchemaHttps } insecureTransport := &http.Transport{ diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index 79fa2921726..6e57c61b0ba 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -211,7 +211,7 @@ func WithTuning(batchTimeout time.Duration, mapSessionChanSize int) Option { } } -// WithManualApproveNewNode allows devices to access the network only after manual approval +// WithManualApproveNewNode allows devices to access the network only after manual approval. func WithManualApproveNewNode() Option { return func(hsic *HeadscaleInContainer) { hsic.env["HEADSCALE_NODE_MANAGEMENT_MANUAL_APPROVE_NEW_NODE"] = "true"