From 21706d8d54272e51ba961c6d38fda1d832119799 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 10:50:48 +0000 Subject: [PATCH] oidc: send the change from the user update in the callback --- hscontrol/oidc.go | 20 +++++++++------ hscontrol/oidc_test.go | 57 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index e79b9e4d7..e31265899 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -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 diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go index 69a677c81..32024f66a 100644 --- a/hscontrol/oidc_test.go +++ b/hscontrol/oidc_test.go @@ -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, @@ -1482,12 +1482,12 @@ func TestOIDCLoginClearsUnverifiedEmail(t *testing.T) { Iss: "https://idp.example.com", Sub: "alice", Username: "alice", Email: "alice@example.com", EmailVerified: true, } - first, err := provider.createOrUpdateUserFromClaim(claims) + 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) + second, _, err := provider.createOrUpdateUserFromClaim(claims) require.NoError(t, err) assert.Equal(t, first.ID, second.ID) assert.Empty(t, second.Email) @@ -1497,6 +1497,57 @@ func TestOIDCLoginClearsUnverifiedEmail(t *testing.T) { 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()