Repository: juanfont/headscale

Issue:
[Bug] re-assigen node to new user THEN new user can not see this node
### Is this a support request?

- [x] This is not a support request

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current Behavior

I used the Pre-Auth Key to register node-A to user-A. Then, I assigned Node-A to user-B. but it didn't show in `tailscale status` (another node of User-B)

Then I delete Node-A, and used User-B's Pre-Auth Key to login. It shown in the `tailscale status` but I cannot ping node-A.(It works after restart the PC)

### Expected Behavior

none

### Steps To Reproduce

see above

### Environment

```markdown
- OS: Windows
- Headscale version: 0.25.1
- Tailscale version: 1.80.2
```

### Runtime environment

- [ ] Headscale is behind a (reverse) proxy
- [ ] Headscale runs in a container

### Anything else?

_No response_

Hints from issue comments:

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eb98bbd2..18878d8f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -97,6 +97,8 @@ working in v1 and not tested might be broken in v2 (and vice versa).
   [#2493](https://github.com/juanfont/headscale/pull/2493)
   - If a OIDC provider doesn't include the `email_verified` claim in its ID
     tokens, Headscale will attempt to get it from the UserInfo endpoint.
+- OIDC: Try to populate name, email and username from UserInfo
+  [#2545](https://github.com/juanfont/headscale/pull/2545)
 - Improve performance by only querying relevant nodes from the database for node
   updates [#2509](https://github.com/juanfont/headscale/pull/2509)
 - node FQDNs in the netmap will now contain a dot (".") at the end. This aligns
diff --git a/cmd/headscale/cli/nodes.go b/cmd/headscale/cli/nodes.go
index 9234cc49..baa89820 100644
--- a/cmd/headscale/cli/nodes.go
+++ b/cmd/headscale/cli/nodes.go
@@ -78,7 +78,7 @@ func init() {
 		log.Fatal(err.Error())
 	}
 
-	moveNodeCmd.Flags().StringP("user", "u", "", "New user")
+	moveNodeCmd.Flags().Uint64P("user", "u", 0, "New user")
 
 	moveNodeCmd.Flags().StringP("namespace", "n", "", "User")
 	moveNodeNamespaceFlag := moveNodeCmd.Flags().Lookup("namespace")
@@ -470,7 +470,7 @@ var moveNodeCmd = &cobra.Command{
 			return
 		}
 
-		user, err := cmd.Flags().GetString("user")
+		user, err := cmd.Flags().GetUint64("user")
 		if err != nil {
 			ErrorOutput(
 				err,
diff --git a/gen/go/headscale/v1/node.pb.go b/gen/go/headscale/v1/node.pb.go
index 1c4f2e3c..db2817fc 100644
--- a/gen/go/headscale/v1/node.pb.go
+++ b/gen/go/headscale/v1/node.pb.go
@@ -1001,7 +1001,7 @@ func (x *ListNodesResponse) GetNodes() []*Node {
 type MoveNodeRequest struct {
 	state         protoimpl.MessageState `protogen:"open.v1"`
 	NodeId        uint64                 `protobuf:"varint,1,opt,name=node_id,json=nodeId,proto3" json:"node_id,omitempty"`
-	User          string                 `protobuf:"bytes,2,opt,name=user,proto3" json:"user,omitempty"`
+	User          uint64                 `protobuf:"varint,2,opt,name=user,proto3" json:"user,omitempty"`
 	unknownFields protoimpl.UnknownFields
 	sizeCache     protoimpl.SizeCache
 }
@@ -1043,11 +1043,11 @@ func (x *MoveNodeRequest) GetNodeId() uint64 {
 	return 0
 }
 
-func (x *MoveNodeRequest) GetUser() string {
+func (x *MoveNodeRequest) GetUser() uint64 {
 	if x != nil {
 		return x.User
 	}
-	return ""
+	return 0
 }
 
 type MoveNodeResponse struct {
@@ -1365,7 +1365,7 @@ const file_headscale_v1_node_proto_rawDesc = "" +
 	"\x05nodes\x18\x01 \x03(\v2\x12.headscale.v1.NodeR\x05nodes\">\n" +
 	"\x0fMoveNodeRequest\x12\x17\n" +
 	"\anode_id\x18\x01 \x01(\x04R\x06nodeId\x12\x12\n" +
-	"\x04user\x18\x02 \x01(\tR\x04user\":\n" +
+	"\x04user\x18\x02 \x01(\x04R\x04user\":\n" +
 	"\x10MoveNodeResponse\x12&\n" +
 	"\x04node\x18\x01 \x01(\v2\x12.headscale.v1.NodeR\x04node\"j\n" +
 	"\x16DebugCreateNodeRequest\x12\x12\n" +
diff --git a/gen/openapiv2/headscale/v1/headscale.swagger.json b/gen/openapiv2/headscale/v1/headscale.swagger.json
index e75b7ee2..c55dc077 100644
--- a/gen/openapiv2/headscale/v1/headscale.swagger.json
+++ b/gen/openapiv2/headscale/v1/headscale.swagger.json
@@ -800,7 +800,8 @@
       "type": "object",
       "properties": {
         "user": {
-          "type": "string"
+          "type": "string",
+          "format": "uint64"
         }
       }
     },
diff --git a/hscontrol/grpcv1.go b/hscontrol/grpcv1.go
index d924a1fb..0e36cf63 100644
--- a/hscontrol/grpcv1.go
+++ b/hscontrol/grpcv1.go
@@ -547,22 +547,31 @@ func (api headscaleV1APIServer) MoveNode(
 	ctx context.Context,
 	request *v1.MoveNodeRequest,
 ) (*v1.MoveNodeResponse, error) {
-	// TODO(kradalby): This should be done in one tx.
-	node, err := api.h.db.GetNodeByID(types.NodeID(request.GetNodeId()))
-	if err != nil {
-		return nil, err
-	}
+	node, err := db.Write(api.h.db.DB, func(tx *gorm.DB) (*types.Node, error) {
+		node, err := db.GetNodeByID(tx, types.NodeID(request.GetNodeId()))
+		if err != nil {
+			return nil, err
+		}
 
-	user, err := api.h.db.GetUserByName(request.GetUser())
-	if err != nil {
-		return nil, err
-	}
+		err = db.AssignNodeToUser(tx, node, types.UserID(request.GetUser()))
+		if err != nil {
+			return nil, err
+		}
 
-	err = api.h.db.AssignNodeToUser(node, types.UserID(user.ID))
+		return node, nil
+	})
 	if err != nil {
 		return nil, err
 	}
 
+	ctx = types.NotifyCtx(ctx, "cli-movenode-self", node.Hostname)
+	api.h.nodeNotifier.NotifyByNodeID(
+		ctx,
+		types.UpdateSelf(node.ID),
+		node.ID)
+	ctx = types.NotifyCtx(ctx, "cli-movenode", node.Hostname)
+	api.h.nodeNotifier.NotifyWithIgnore(ctx, types.UpdatePeerChanged(node.ID), node.ID)
+
 	return &v1.MoveNodeResponse{Node: node.Proto()}, nil
 }
 
diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go
index 85566d0f..e566d64a 100644
--- a/hscontrol/oidc.go
+++ b/hscontrol/oidc.go
@@ -2,6 +2,7 @@ package hscontrol
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	_ "embed"
 	"errors"
@@ -280,14 +281,28 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 		return
 	}
 
-	// If EmailVerified is missing, we can try to get it from UserInfo
-	if !claims.EmailVerified {
-		var userinfo *oidc.UserInfo
-		userinfo, err = a.oidcProvider.UserInfo(req.Context(), oauth2.StaticTokenSource(oauth2Token))
-		if err != nil {
-			util.LogErr(err, "could not get userinfo; email cannot be verified")
+	var userinfo *oidc.UserInfo
+	userinfo, err = a.oidcProvider.UserInfo(req.Context(), oauth2.StaticTokenSource(oauth2Token))
+	if err != nil {
+		util.LogErr(err, "could not get userinfo; only checking claim")
+	}
+
+	// If the userinfo is available, we can check if the subject matches the
+	// claims, then use some of the userinfo fields to update the user.
+	// https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
+	if userinfo != nil && userinfo.Subject == claims.Sub {
+		claims.Email = cmp.Or(claims.Email, userinfo.Email)
+		claims.EmailVerified = cmp.Or(claims.EmailVerified, types.FlexibleBoolean(userinfo.EmailVerified))
+
+		// The userinfo has some extra fields that we can use to update the user but they are only
+		// available in the underlying claims struct.
+		// TODO(kradalby): there might be more interesting fields here that we have not found yet.
+		var userinfo2 types.OIDCUserInfo
+		if err := userinfo.Claims(&userinfo2); err == nil {
+			claims.Username = cmp.Or(claims.Username, userinfo2.PreferredUsername)
+			claims.Name = cmp.Or(claims.Name, userinfo2.Name)
+			claims.ProfilePictureURL = cmp.Or(claims.ProfilePictureURL, userinfo2.Picture)
 		}
-		claims.EmailVerified = types.FlexibleBoolean(userinfo.EmailVerified)
 	}
 
 	user, err := a.createOrUpdateUserFromClaim(&claims)
diff --git a/hscontrol/types/users.go b/hscontrol/types/users.go
index 96988a0a..471cb1e5 100644
--- a/hscontrol/types/users.go
+++ b/hscontrol/types/users.go
@@ -157,7 +157,7 @@ func (u *User) Proto() *v1.User {
 type FlexibleBoolean bool
 
 func (bit *FlexibleBoolean) UnmarshalJSON(data []byte) error {
-	var val interface{}
+	var val any
 	err := json.Unmarshal(data, &val)
 	if err != nil {
 		return fmt.Errorf("could not unmarshal data: %w", err)
@@ -203,6 +203,17 @@ func (c *OIDCClaims) Identifier() string {
 	return c.Iss + "/" + c.Sub
 }
 
+type OIDCUserInfo struct {
+	Sub               string          `json:"sub"`
+	Name              string          `json:"name"`
+	GivenName         string          `json:"given_name"`
+	FamilyName        string          `json:"family_name"`
+	PreferredUsername string          `json:"preferred_username"`
+	Email             string          `json:"email"`
+	EmailVerified     FlexibleBoolean `json:"email_verified,omitempty"`
+	Picture           string          `json:"picture"`
+}
+
 // FromClaim overrides a User from OIDC claims.
 // All fields will be updated, except for the ID.
 func (u *User) FromClaim(claims *OIDCClaims) {
diff --git a/proto/headscale/v1/node.proto b/proto/headscale/v1/node.proto
index 1b6021ce..89d2c347 100644
--- a/proto/headscale/v1/node.proto
+++ b/proto/headscale/v1/node.proto
@@ -99,7 +99,7 @@ message ListNodesResponse { repeated Node nodes = 1; }
 
 message MoveNodeRequest {
   uint64 node_id = 1;
-  string user = 2;
+  uint64 user = 2;
 }
 
 message MoveNodeResponse { Node node = 1; }



Test patch:
diff --git a/integration/cli_test.go b/integration/cli_test.go
index a474f5c2..435b7e55 100644
--- a/integration/cli_test.go
+++ b/integration/cli_test.go
@@ -1580,6 +1580,9 @@ func TestNodeMoveCommand(t *testing.T) {
 	// Randomly generated node key
 	regID := types.MustRegistrationID()
 
+	userMap, err := headscale.MapUsers()
+	assertNoErr(t, err)
+
 	_, err = headscale.Execute(
 		[]string{
 			"headscale",
@@ -1628,9 +1631,9 @@ func TestNodeMoveCommand(t *testing.T) {
 			"nodes",
 			"move",
 			"--identifier",
-			nodeID,
+			strconv.FormatUint(node.GetId(), 10),
 			"--user",
-			"new-user",
+			strconv.FormatUint(userMap["new-user"].GetId(), 10),
 			"--output",
 			"json",
 		},
@@ -1668,7 +1671,7 @@ func TestNodeMoveCommand(t *testing.T) {
 			"--identifier",
 			nodeID,
 			"--user",
-			"non-existing-user",
+			"999",
 			"--output",
 			"json",
 		},
@@ -1689,7 +1692,7 @@ func TestNodeMoveCommand(t *testing.T) {
 			"--identifier",
 			nodeID,
 			"--user",
-			"old-user",
+			strconv.FormatUint(userMap["old-user"].GetId(), 10),
 			"--output",
 			"json",
 		},
@@ -1708,7 +1711,7 @@ func TestNodeMoveCommand(t *testing.T) {
 			"--identifier",
 			nodeID,
 			"--user",
-			"old-user",
+			strconv.FormatUint(userMap["old-user"].GetId(), 10),
 			"--output",
 			"json",
 		},