mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 04:34:07 +09:00
state, oidc: persist user changes and refresh nodes and clients
Write cleared user columns, discard OIDC email the provider no longer vouches for, refresh NodeStore users, and notify clients from the callback. Fixes #3471
This commit is contained in:
1 parent
faa2b7c89d
commit
cea0ef340d
8 files changed
+522
-16
No files matched your search
+5
-4
@@ -127,12 +127,13 @@ are configured, a user needs to pass all of them.
|
||||
|
||||
### Control email verification
|
||||
|
||||
Headscale uses the `email` claim from the identity provider to synchronize the email address to its user profile. By
|
||||
default, a user's email address is only synchronized when the identity provider reports the email address as verified
|
||||
via the `email_verified: true` claim.
|
||||
Headscale synchronizes the `email` claim from the identity provider to its user profile on each OIDC login. By default,
|
||||
the profile uses the email only when the latest login reports `email_verified: true`. An absent, unverified, or
|
||||
malformed email clears the previously stored address.
|
||||
|
||||
Unverified emails may be allowed in case an identity provider does not send the `email_verified` claim or email
|
||||
verification is not required. In that case, a user's email address is always synchronized to the user profile.
|
||||
verification is not required. With `email_verified_required: false`, the latest login's email is stored without
|
||||
requiring verification. An absent or malformed email still clears the previously stored address.
|
||||
|
||||
```yaml hl_lines="5"
|
||||
oidc:
|
||||
|
||||
+12
-8
@@ -19,6 +19,7 @@ import (
|
||||
"github.com/juanfont/headscale/hscontrol/db"
|
||||
"github.com/juanfont/headscale/hscontrol/templates"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/rs/zerolog/log"
|
||||
"golang.org/x/oauth2"
|
||||
@@ -377,7 +378,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
|
||||
return
|
||||
}
|
||||
|
||||
user, err := a.createOrUpdateUserFromClaim(&claims)
|
||||
user, userChange, err := a.createOrUpdateUserFromClaim(&claims)
|
||||
if err != nil {
|
||||
httpUserError(writer, NewHTTPError(
|
||||
http.StatusInternalServerError,
|
||||
@@ -388,6 +389,8 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
|
||||
return
|
||||
}
|
||||
|
||||
a.h.Change(userChange)
|
||||
|
||||
// If this is a registration flow, render the confirmation
|
||||
// interstitial instead of finalising the registration immediately.
|
||||
// Without an explicit user click, a single GET to
|
||||
@@ -713,16 +716,17 @@ func (a *AuthProviderOIDC) getAuthInfoFromState(state string) *AuthInfo {
|
||||
|
||||
func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
|
||||
claims *types.OIDCClaims,
|
||||
) (*types.User, error) {
|
||||
) (*types.User, change.Change, error) {
|
||||
var (
|
||||
user *types.User
|
||||
err error
|
||||
newUser bool
|
||||
c change.Change
|
||||
)
|
||||
|
||||
user, err = a.h.state.GetUserByOIDCIdentifier(claims.Identifier())
|
||||
if err != nil && !errors.Is(err, db.ErrUserNotFound) {
|
||||
return nil, fmt.Errorf("creating or updating user: %w", err)
|
||||
return nil, change.Change{}, fmt.Errorf("creating or updating user: %w", err)
|
||||
}
|
||||
|
||||
// if the user is still not found, create a new empty user.
|
||||
@@ -736,21 +740,21 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
|
||||
user.FromClaim(claims, a.cfg.EmailVerifiedRequired)
|
||||
|
||||
if newUser {
|
||||
user, _, err = a.h.state.CreateUser(*user)
|
||||
user, c, err = a.h.state.CreateUser(*user)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("creating user: %w", err)
|
||||
return nil, change.Change{}, fmt.Errorf("creating user: %w", err)
|
||||
}
|
||||
} else {
|
||||
_, _, err = a.h.state.UpdateUser(types.UserID(user.ID), func(u *types.User) error {
|
||||
_, c, err = a.h.state.UpdateUser(types.UserID(user.ID), func(u *types.User) error {
|
||||
*u = *user
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("updating user: %w", err)
|
||||
return nil, change.Change{}, fmt.Errorf("updating user: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
return user, nil
|
||||
return user, c, nil
|
||||
}
|
||||
|
||||
// registerConfirmCSRFCookie is the cookie name used to bind the
|
||||
|
||||
+79
-1
@@ -926,7 +926,7 @@ func TestOIDCPendingCallbackPreservesSSHVerdict(t *testing.T) {
|
||||
require.True(t, ok)
|
||||
|
||||
principal := mockoidc.DefaultUser()
|
||||
user, err := a.createOrUpdateUserFromClaim(&types.OIDCClaims{
|
||||
user, _, err := a.createOrUpdateUserFromClaim(&types.OIDCClaims{
|
||||
Iss: b.idp.Issuer(),
|
||||
Sub: principal.Subject,
|
||||
Username: principal.PreferredUsername,
|
||||
@@ -1470,6 +1470,84 @@ func TestOIDCRefusesAlteredSubject(t *testing.T) {
|
||||
assert.Equal(t, *before, users[0], "a refused subject must not update an existing row")
|
||||
}
|
||||
|
||||
func TestOIDCLoginClearsUnverifiedEmail(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
provider := &AuthProviderOIDC{
|
||||
h: app,
|
||||
cfg: &types.OIDCConfig{
|
||||
EmailVerifiedRequired: true,
|
||||
},
|
||||
}
|
||||
claims := &types.OIDCClaims{
|
||||
Iss: "https://idp.example.com", Sub: "alice", Username: "alice",
|
||||
Email: "alice@example.com", EmailVerified: true,
|
||||
}
|
||||
first, _, err := provider.createOrUpdateUserFromClaim(claims)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "alice@example.com", first.Email)
|
||||
|
||||
claims.EmailVerified = false
|
||||
second, _, err := provider.createOrUpdateUserFromClaim(claims)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, first.ID, second.ID)
|
||||
assert.Empty(t, second.Email)
|
||||
|
||||
stored, err := app.state.GetUserByID(types.UserID(first.ID))
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, stored.Email)
|
||||
}
|
||||
|
||||
func TestOIDCCallbackDispatchesUserIdentityChange(t *testing.T) {
|
||||
b := newOIDCBrowser(t)
|
||||
provider, ok := b.app.authProvider.(*AuthProviderOIDC)
|
||||
require.True(t, ok)
|
||||
|
||||
provider.cfg.EmailVerifiedRequired = true
|
||||
user, _, err := provider.createOrUpdateUserFromClaim(&types.OIDCClaims{
|
||||
Iss: b.idp.Issuer(), Sub: "identity-change", Username: "alice",
|
||||
Email: "alice@old.example", EmailVerified: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
node := b.app.state.CreateRegisteredNodeForTest(user, "identity-node")
|
||||
node.User = user.Clone()
|
||||
b.app.state.PutNodeInStoreForTest(*node)
|
||||
require.NoError(t, b.app.state.UpdatePolicyManagerNodesForTest())
|
||||
_, err = b.app.state.SetPolicy([]byte(`{
|
||||
"acls": [{"action": "accept", "src": ["alice@old.example"], "dst": ["*:*"]}]
|
||||
}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
responses := make(chan *tailcfg.MapResponse, 8)
|
||||
require.NoError(t, b.app.mapBatcher.AddNode(node.ID, responses, tailcfg.CurrentCapabilityVersion, nil))
|
||||
t.Cleanup(func() { b.app.mapBatcher.RemoveNode(node.ID, responses) })
|
||||
|
||||
initial := <-responses
|
||||
require.NotEmpty(t, initial.PacketFilters["base"])
|
||||
|
||||
b.idp.QueueUser(&mockoidc.MockUser{
|
||||
Subject: "identity-change", PreferredUsername: "alice",
|
||||
Email: "alice@new.example", EmailVerified: true,
|
||||
})
|
||||
_, registerURL := b.pendingNode(t)
|
||||
status, _, _ := b.get(t, registerURL)
|
||||
require.Equal(t, http.StatusOK, status)
|
||||
|
||||
stored, err := b.app.state.GetUserByID(types.UserID(user.ID))
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "alice@new.example", stored.Email)
|
||||
|
||||
assert.Eventually(t, func() bool {
|
||||
select {
|
||||
case response := <-responses:
|
||||
filters, present := response.PacketFilters["base"]
|
||||
return present && len(filters) == 0
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}, 3*time.Second, 20*time.Millisecond, "the connected node must receive the updated filter before registration confirmation")
|
||||
}
|
||||
|
||||
func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -1239,3 +1239,34 @@ func peerByHostname(nm *netmap.NetworkMap, hostname string) (tailcfg.NodeView, b
|
||||
|
||||
return tailcfg.NodeView{}, false
|
||||
}
|
||||
|
||||
func TestUserProfileFollowsRename(t *testing.T) {
|
||||
srv := servertest.NewServer(t)
|
||||
owner := srv.CreateUser(t, "profile-original")
|
||||
profileID := owner.TailscaleUserProfile().ID
|
||||
peer := srv.CreateUser(t, "profile-peer")
|
||||
ownerClient := servertest.NewClient(t, srv, "owner", servertest.WithUser(owner))
|
||||
peerClient := servertest.NewClient(t, srv, "peer", servertest.WithUser(peer))
|
||||
ownerClient.WaitForPeers(t, 1, 5*time.Second)
|
||||
peerClient.WaitForPeers(t, 1, 5*time.Second)
|
||||
|
||||
_, c, err := srv.State().RenameUser(types.UserID(owner.ID), "profile-renamed")
|
||||
require.NoError(t, err)
|
||||
|
||||
srv.App.Change(c)
|
||||
|
||||
for _, client := range []*servertest.TestClient{ownerClient, peerClient} {
|
||||
client.WaitForCondition(t, "renamed user profile", 5*time.Second,
|
||||
func(nm *netmap.NetworkMap) bool {
|
||||
profile, ok := nm.UserProfiles[profileID]
|
||||
return ok && profile.LoginName() == "profile-renamed"
|
||||
})
|
||||
}
|
||||
|
||||
late := servertest.NewClient(t, srv, "late", servertest.WithUser(peer))
|
||||
late.WaitForCondition(t, "renamed user profile on join", 5*time.Second,
|
||||
func(nm *netmap.NetworkMap) bool {
|
||||
profile, ok := nm.UserProfiles[profileID]
|
||||
return ok && profile.LoginName() == "profile-renamed"
|
||||
})
|
||||
}
|
||||
@@ -2475,3 +2475,303 @@ func TestReauthReplacesReportedTags(t *testing.T) {
|
||||
assert.Equal(t, want, row.Hostinfo.RequestTags)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateUserClearsColumns(t *testing.T) {
|
||||
_, s, nodeID := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
node, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
userID := types.UserID(node.UserID().Get())
|
||||
_, _, err := s.UpdateUser(userID, func(user *types.User) error {
|
||||
user.DisplayName = "Display"
|
||||
user.Email = "persist@example.com"
|
||||
user.ProfilePicURL = "https://example.com/picture"
|
||||
|
||||
return nil
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, _, err = s.RenameUser(userID, "renamed")
|
||||
require.NoError(t, err)
|
||||
user, err := s.GetUserByID(userID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "Display", user.DisplayName)
|
||||
assert.Equal(t, "persist@example.com", user.Email)
|
||||
assert.Equal(t, "https://example.com/picture", user.ProfilePicURL)
|
||||
_, _, err = s.UpdateUser(userID, func(user *types.User) error {
|
||||
user.DisplayName = ""
|
||||
user.Email = ""
|
||||
user.ProfilePicURL = ""
|
||||
|
||||
return nil
|
||||
})
|
||||
require.NoError(t, err)
|
||||
user, err = s.GetUserByID(userID)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, user.DisplayName)
|
||||
assert.Empty(t, user.Email)
|
||||
assert.Empty(t, user.ProfilePicURL)
|
||||
assert.Equal(t, "renamed", user.Name)
|
||||
}
|
||||
|
||||
func TestUpdateUserRefreshesNodeCopies(t *testing.T) {
|
||||
_, s, nodeID := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
original, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
userID := types.UserID(original.UserID().Get())
|
||||
user, err := s.GetUserByID(userID)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, ok = s.nodeStore.UpdateNode(nodeID, func(n *types.Node) {
|
||||
n.AuthKey = &types.Credential{ID: 1, UserID: new(user.ID), User: user.Clone()}
|
||||
})
|
||||
require.True(t, ok)
|
||||
|
||||
tagged := s.nodeStore.PutNode(types.Node{
|
||||
ID: nodeID + 1, Hostname: "tagged", Tags: []string{"tag:server"},
|
||||
MachineKey: key.NewMachine().Public(), NodeKey: key.NewNode().Public(),
|
||||
AuthKey: &types.Credential{ID: 2, UserID: new(user.ID), User: user.Clone()},
|
||||
})
|
||||
other, _, err := s.CreateUser(types.User{Name: "other"})
|
||||
require.NoError(t, err)
|
||||
|
||||
otherNode := s.nodeStore.PutNode(types.Node{
|
||||
ID: nodeID + 2, Hostname: "other", UserID: new(other.ID), User: other.Clone(),
|
||||
MachineKey: key.NewMachine().Public(), NodeKey: key.NewNode().Public(),
|
||||
AuthKey: &types.Credential{ID: 3, UserID: new(other.ID), User: other.Clone()},
|
||||
})
|
||||
before, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
_, c, err := s.RenameUser(userID, "renamed-user")
|
||||
require.NoError(t, err)
|
||||
assert.True(t, c.IncludeSelf)
|
||||
assert.True(t, c.SendAllPeers)
|
||||
|
||||
updated, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "renamed-user", updated.User().Name())
|
||||
assert.Equal(t, "renamed-user", updated.AuthKey().User().Name())
|
||||
assert.Equal(t, "persist-user", before.User().Name())
|
||||
assert.Equal(t, "persist-user", before.AuthKey().User().Name())
|
||||
|
||||
updatedTagged, ok := s.GetNodeByID(tagged.ID())
|
||||
require.True(t, ok)
|
||||
assert.True(t, updatedTagged.IsTagged())
|
||||
assert.Equal(t, "renamed-user", updatedTagged.AuthKey().User().Name())
|
||||
assert.Equal(t, "persist-user", tagged.AuthKey().User().Name())
|
||||
|
||||
unchanged, ok := s.GetNodeByID(otherNode.ID())
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "other", unchanged.User().Name())
|
||||
assert.Equal(t, "other", unchanged.AuthKey().User().Name())
|
||||
|
||||
snapshot := s.nodeStore.data.Load()
|
||||
_, c, err = s.RenameUser(userID, "renamed-user")
|
||||
require.NoError(t, err)
|
||||
assert.True(t, c.IsEmpty())
|
||||
assert.Same(t, snapshot, s.nodeStore.data.Load(), "an unchanged user must not rebuild the snapshot")
|
||||
|
||||
_, c, err = s.UpdateUser(userID, func(user *types.User) error {
|
||||
user.Provider = "updated-provider"
|
||||
return nil
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, c.IsEmpty(), "a non-profile column does not need a full map update")
|
||||
|
||||
updated, ok = s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "updated-provider", updated.User().Provider())
|
||||
assert.Equal(t, "updated-provider", updated.AuthKey().User().Provider())
|
||||
}
|
||||
|
||||
type blockingSetUsersPolicyManager struct {
|
||||
policy.PolicyManager
|
||||
|
||||
blocked atomic.Bool
|
||||
reached chan struct{}
|
||||
resume <-chan struct{}
|
||||
last []types.User
|
||||
}
|
||||
|
||||
func (pm *blockingSetUsersPolicyManager) SetUsers(users []types.User) (bool, bool, error) {
|
||||
if !pm.blocked.Swap(true) {
|
||||
close(pm.reached)
|
||||
<-pm.resume
|
||||
}
|
||||
|
||||
pm.last = users
|
||||
|
||||
return pm.PolicyManager.SetUsers(users)
|
||||
}
|
||||
|
||||
func TestUpdateUserPublishesInOrder(t *testing.T) {
|
||||
_, s, nodeID := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
node, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
user := node.User().AsStruct()
|
||||
userID := types.UserID(user.ID)
|
||||
_, ok = s.nodeStore.UpdateNode(nodeID, func(n *types.Node) {
|
||||
n.AuthKey = &types.Credential{ID: 1, UserID: new(user.ID), User: user.Clone()}
|
||||
})
|
||||
require.True(t, ok)
|
||||
|
||||
resume := make(chan struct{})
|
||||
|
||||
releaseFirst := sync.OnceFunc(func() { close(resume) })
|
||||
defer releaseFirst()
|
||||
|
||||
pm := &blockingSetUsersPolicyManager{
|
||||
PolicyManager: s.polMan,
|
||||
reached: make(chan struct{}),
|
||||
resume: resume,
|
||||
}
|
||||
s.polMan = pm
|
||||
|
||||
update := func(name string) error {
|
||||
_, _, err := s.UpdateUser(userID, func(u *types.User) error {
|
||||
u.Name = name
|
||||
u.Email = name + "@example.com"
|
||||
u.DisplayName = name + " display"
|
||||
u.ProfilePicURL = "https://example.com/" + name
|
||||
|
||||
return nil
|
||||
})
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
firstDone := make(chan error, 1)
|
||||
go func() { firstDone <- update("first-user") }()
|
||||
|
||||
// The first update pauses after its commit, inside the policy user update.
|
||||
<-pm.reached
|
||||
|
||||
var secondErr error
|
||||
|
||||
secondDone := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
secondErr = update("latest-user")
|
||||
|
||||
close(secondDone)
|
||||
}()
|
||||
|
||||
// Give the second update time to overtake the paused first one.
|
||||
select {
|
||||
case <-secondDone:
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
|
||||
releaseFirst()
|
||||
require.NoError(t, <-firstDone)
|
||||
<-secondDone
|
||||
require.NoError(t, secondErr)
|
||||
|
||||
stored, err := s.GetUserByID(userID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "latest-user", stored.Name)
|
||||
|
||||
require.Len(t, pm.last, 1)
|
||||
assert.Equal(t, stored.Name, pm.last[0].Name, "policy users must follow the database")
|
||||
|
||||
updated, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, stored.Name, updated.User().Name())
|
||||
assert.Equal(t, stored.Name, updated.AuthKey().User().Name())
|
||||
assert.Equal(t, stored.TailscaleUserProfile(), updated.User().TailscaleUserProfile())
|
||||
assert.Equal(t, stored.TailscaleUserProfile(), updated.AuthKey().User().TailscaleUserProfile())
|
||||
}
|
||||
|
||||
func TestUserListPublishesInOrder(t *testing.T) {
|
||||
_, s, nodeID := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
node, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
resume := make(chan struct{})
|
||||
|
||||
releaseFirst := sync.OnceFunc(func() { close(resume) })
|
||||
defer releaseFirst()
|
||||
|
||||
pm := &blockingSetUsersPolicyManager{
|
||||
PolicyManager: s.polMan,
|
||||
reached: make(chan struct{}),
|
||||
resume: resume,
|
||||
}
|
||||
s.polMan = pm
|
||||
|
||||
createDone := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
_, _, err := s.CreateUser(types.User{Name: "created-user"})
|
||||
createDone <- err
|
||||
}()
|
||||
|
||||
// The creation pauses holding a user list read before the rename commits.
|
||||
<-pm.reached
|
||||
|
||||
var renameErr error
|
||||
|
||||
renameDone := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
_, _, renameErr = s.RenameUser(types.UserID(node.User().ID()), "renamed-user")
|
||||
|
||||
close(renameDone)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-renameDone:
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
|
||||
releaseFirst()
|
||||
require.NoError(t, <-createDone)
|
||||
<-renameDone
|
||||
require.NoError(t, renameErr)
|
||||
|
||||
names := make([]string, 0, len(pm.last))
|
||||
for _, user := range pm.last {
|
||||
names = append(names, user.Name)
|
||||
}
|
||||
|
||||
assert.ElementsMatch(t, []string{"renamed-user", "created-user"}, names,
|
||||
"policy users must follow the database")
|
||||
}
|
||||
|
||||
var errSetUsersTest = errors.New("set users failed")
|
||||
|
||||
type failingSetUsersPolicyManager struct {
|
||||
policy.PolicyManager
|
||||
}
|
||||
|
||||
func (failingSetUsersPolicyManager) SetUsers([]types.User) (bool, bool, error) {
|
||||
return false, false, errSetUsersTest
|
||||
}
|
||||
|
||||
func TestUpdateUserRefreshesNodeCopiesWhenPolicyUpdateFails(t *testing.T) {
|
||||
_, s, nodeID := persistTestSetup(t)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
node, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
|
||||
s.polMan = failingSetUsersPolicyManager{PolicyManager: s.polMan}
|
||||
|
||||
_, _, err := s.RenameUser(types.UserID(node.User().ID()), "renamed-user")
|
||||
require.ErrorIs(t, err, errSetUsersTest)
|
||||
|
||||
updated, ok := s.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, "renamed-user", updated.User().Name(),
|
||||
"the committed row and its cached copy must agree")
|
||||
}
|
||||
@@ -244,6 +244,13 @@ type State struct {
|
||||
// nothing.
|
||||
persistMu sync.Mutex
|
||||
|
||||
// userUpdateMu keeps committed user changes and cached copies in the same order.
|
||||
userUpdateMu sync.Mutex
|
||||
|
||||
// policyUsersMu orders user-list reads with SetUsers, so a list read
|
||||
// before a commit cannot replace one read after it.
|
||||
policyUsersMu sync.Mutex
|
||||
|
||||
// selfRefresh holds nodes whose own DNS config inputs changed (NextDNS
|
||||
// device metadata, given name), drained by [State.DrainSelfRefreshes].
|
||||
selfRefresh []types.NodeID
|
||||
@@ -583,18 +590,26 @@ func (s *State) CreateUser(user types.User) (*types.User, change.Change, error)
|
||||
// UpdateUser modifies an existing user using the provided update function within a transaction.
|
||||
// Returns the updated user, change set, and any error.
|
||||
func (s *State) UpdateUser(userID types.UserID, updateFn func(*types.User) error) (*types.User, change.Change, error) {
|
||||
s.userUpdateMu.Lock()
|
||||
defer s.userUpdateMu.Unlock()
|
||||
|
||||
var previous types.User
|
||||
|
||||
user, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.User, error) {
|
||||
user, err := hsdb.GetUserByID(tx, userID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
previous = *user
|
||||
|
||||
if err := updateFn(user); err != nil { //nolint:noinlineerr
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Use Updates() to only update modified fields, preserving unchanged values.
|
||||
err = tx.Updates(user).Error
|
||||
// Select every column so cleared fields are written. Save would also
|
||||
// insert the row again if a delete removed it meanwhile.
|
||||
err = tx.Select("*").Updates(user).Error //nolint:unqueryvet // UPDATE, not a read
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("updating user: %w", err)
|
||||
}
|
||||
@@ -605,13 +620,57 @@ func (s *State) UpdateUser(userID types.UserID, updateFn func(*types.User) error
|
||||
return nil, change.Change{}, err
|
||||
}
|
||||
|
||||
var profileChanged bool
|
||||
|
||||
previous.UpdatedAt = user.UpdatedAt
|
||||
if previous != *user {
|
||||
updates := make(map[types.NodeID]UpdateNodeFunc)
|
||||
|
||||
var ownerRefreshed bool
|
||||
|
||||
for _, node := range s.nodeStore.ListNodes().All() {
|
||||
owner := node.User()
|
||||
credential := node.AuthKey()
|
||||
owns := owner.Valid() && owner.ID() == user.ID
|
||||
createdKey := credential.Valid() && credential.User().Valid() &&
|
||||
credential.User().ID() == user.ID
|
||||
|
||||
if !owns && !createdKey {
|
||||
continue
|
||||
}
|
||||
|
||||
updates[node.ID()] = func(n *types.Node) {
|
||||
// Recheck ownership after waiting in the writer queue, and replace
|
||||
// shared records so earlier snapshots keep their user data.
|
||||
if n.User != nil && n.User.ID == user.ID {
|
||||
n.User = user.Clone()
|
||||
ownerRefreshed = ownerRefreshed || !n.IsTagged()
|
||||
}
|
||||
|
||||
if n.AuthKey != nil && n.AuthKey.User != nil && n.AuthKey.User.ID == user.ID {
|
||||
n.AuthKey = n.AuthKey.Clone()
|
||||
n.AuthKey.User = user.Clone()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
s.nodeStore.UpdateNodes(updates)
|
||||
|
||||
previousProfile := previous.TailscaleUserProfile()
|
||||
profile := user.TailscaleUserProfile()
|
||||
|
||||
profileChanged = ownerRefreshed && !previousProfile.Equal(&profile)
|
||||
}
|
||||
|
||||
// Check if policy manager needs updating
|
||||
c, err := s.updatePolicyManagerUsers()
|
||||
if err != nil {
|
||||
return user, change.Change{}, fmt.Errorf("updating policy manager after user update: %w", err)
|
||||
}
|
||||
|
||||
// TODO(kradalby): We might want to update nodestore with the user data
|
||||
if profileChanged {
|
||||
c = change.UserAdded()
|
||||
}
|
||||
|
||||
return user, c, nil
|
||||
}
|
||||
@@ -3820,6 +3879,9 @@ func reloginChanged(before, after types.NodeView, now time.Time) relogin {
|
||||
// manager, rebuilds peer adjacency when user identity changed, and returns
|
||||
// a PolicyChange when clients need a refresh.
|
||||
func (s *State) updatePolicyManagerUsers() (change.Change, error) {
|
||||
s.policyUsersMu.Lock()
|
||||
defer s.policyUsersMu.Unlock()
|
||||
|
||||
users, err := s.ListAllUsers()
|
||||
if err != nil {
|
||||
return change.Change{}, fmt.Errorf("listing users for policy update: %w", err)
|
||||
|
||||
@@ -440,6 +440,8 @@ func (u *User) FromClaim(claims *OIDCClaims, emailVerifiedRequired bool) {
|
||||
log.Debug().Caller().Err(err).Msgf("username %s is not valid", claims.Username)
|
||||
}
|
||||
|
||||
u.Email = ""
|
||||
|
||||
if claims.EmailVerified || !FlexibleBoolean(emailVerifiedRequired) {
|
||||
_, err = mail.ParseAddress(claims.Email)
|
||||
if err == nil {
|
||||
|
||||
@@ -591,3 +591,31 @@ func TestOIDCClaimsIdentifierKeepsSubject(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFromClaimReplacesEmail(t *testing.T) {
|
||||
for _, tt := range []struct {
|
||||
name string
|
||||
email string
|
||||
verified FlexibleBoolean
|
||||
required bool
|
||||
want string
|
||||
}{
|
||||
{"verified", "current@example.com", true, true, "current@example.com"},
|
||||
{"unverified", "current@example.com", false, true, ""},
|
||||
{"absent", "", false, true, ""},
|
||||
{"malformed", "not-an-address", true, true, ""},
|
||||
{"verification optional", "current@example.com", false, false, "current@example.com"},
|
||||
{"optional but absent", "", false, false, ""},
|
||||
{"optional but malformed", "not-an-address", false, false, ""},
|
||||
} {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
user := User{Name: "alice", Email: "previous@example.com"}
|
||||
user.FromClaim(&OIDCClaims{
|
||||
Iss: "https://idp.example.com", Sub: "alice",
|
||||
Email: tt.email, EmailVerified: tt.verified,
|
||||
}, tt.required)
|
||||
assert.Equal(t, tt.want, user.Email)
|
||||
assert.Equal(t, "alice", user.Name)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user