diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index c6b969f01..d37153908 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -941,6 +941,10 @@ func (a *AuthProviderOIDC) RegisterConfirmGetHandler( info.Hostname = regData.Hostname info.MachineKey = regData.MachineKey.ShortString() + if node, ok := a.h.state.LoginNode(regData.MachineKey, types.UserID(user.ID)); ok { + info.ExistingNode = node.GivenName() + } + if regData.Hostinfo != nil { info.OS = regData.Hostinfo.OS } @@ -1045,7 +1049,12 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( content = renderAuthSuccessTemplate(user) } else { - newNode, err := a.handleRegistration(user, authID, pending.NodeExpiry) + _, reauth := a.h.state.LoginNode( + authReq.RegistrationData().MachineKey, + types.UserID(user.ID), + ) + + node, err := a.handleRegistration(user, authID, pending.NodeExpiry) if err != nil { if errors.Is(err, db.ErrNodeNotFoundRegistrationCache) { httpUserError(writer, newHTTPUserError( @@ -1063,7 +1072,7 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( return } - content = renderRegistrationSuccessTemplate(user, newNode) + content = renderRegistrationSuccessTemplate(user, node, reauth) } // Clear the CSRF cookie now that the request is final. @@ -1084,8 +1093,8 @@ func (a *AuthProviderOIDC) handleRegistration( user *types.User, registrationID types.AuthID, expiry *time.Time, -) (bool, error) { - _, nodeChange, err := a.h.state.HandleNodeFromAuthPath( +) (types.NodeView, error) { + node, nodeChange, err := a.h.state.HandleNodeFromAuthPath( registrationID, types.UserID(user.ID), expiry, @@ -1095,19 +1104,20 @@ func (a *AuthProviderOIDC) handleRegistration( if err != nil { if errors.Is(err, types.ErrHostinfoTooLarge) { - return false, newHTTPUserError(http.StatusBadRequest, "node report too large", + return types.NodeView{}, newHTTPUserError(http.StatusBadRequest, "node report too large", "This device reports too much data. Ask your administrator to reduce its reported metadata, then sign in again.", err) } - return false, fmt.Errorf("registering node: %w", err) + return types.NodeView{}, fmt.Errorf("registering node: %w", err) } - return !nodeChange.IsEmpty(), nil + return node, nil } func renderRegistrationSuccessTemplate( user *types.User, - newNode bool, + _ types.NodeView, + reauth bool, ) *bytes.Buffer { result := templates.AuthSuccessResult{ Title: "Headscale - Node Reauthenticated", @@ -1116,7 +1126,7 @@ func renderRegistrationSuccessTemplate( User: user.Display(), Message: "You can now close this window.", } - if newNode { + if !reauth { result.Title = "Headscale - Node Registered" result.Heading = "Node registered" result.Verb = "Registered" diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go index 1ac9da47c..898bb2c48 100644 --- a/hscontrol/oidc_test.go +++ b/hscontrol/oidc_test.go @@ -1723,6 +1723,83 @@ func TestOIDCConfirmationRejectsWrongRequestKind(t *testing.T) { } } +func TestRegisterConfirmShowsExistingNode(t *testing.T) { + b := newOIDCBrowser(t) + machineKey := key.NewMachine().Public() + existingRow := regexp.MustCompile(`>Existing node]*>([^<]*)`) + + // A tagged node another credential made: a login on its machine adds a + // device instead of taking it. + taggedMachine := key.NewMachine().Public() + tagKey, err := b.app.state.CreatePreAuthKey(nil, true, false, nil, []string{"tag:server"}) + require.NoError(t, err) + _, _, err = b.app.state.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: tagKey.Key}, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "server"}, + }, taggedMachine) + require.NoError(t, err) + + for _, tt := range []struct { + name string + machineKey key.MachinePublic + existing string + heading string + nodeCount int + }{ + { + name: "first login", machineKey: machineKey, + existing: "None, this is a new device", heading: "Node registered", nodeCount: 2, + }, + { + name: "same machine", machineKey: machineKey, + existing: "laptop", heading: "Node reauthenticated", nodeCount: 2, + }, + { + name: "copied hostname", machineKey: key.NewMachine().Public(), + existing: "None, this is a new device", heading: "Node registered", nodeCount: 3, + }, + { + name: "tagged machine", machineKey: taggedMachine, + existing: "None, this is a new device", heading: "Node registered", nodeCount: 4, + }, + } { + authID := types.MustAuthID() + require.NoError(t, b.app.state.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: tt.machineKey, + NodeKey: key.NewNode().Public(), + Hostname: "laptop", + }))) + b.idp.QueueUser(&mockoidc.MockUser{ + Subject: "existing-device", PreferredUsername: "device-owner", + }) + + status, landed, body := b.get(t, b.publicURL+"/register/"+authID.String()) + require.Equal(t, http.StatusOK, status) + + row := existingRow.FindStringSubmatch(body) + if assert.Len(t, row, 2, "the page must distinguish an existing node from client-supplied details") { + assert.Equal(t, tt.existing, row[1], tt.name) + } + + csrf := csrfInputRe.FindStringSubmatch(body) + require.Len(t, csrf, 2) + + //nolint:noctx,bodyclose // test response closed below + confirmed, err := b.client.PostForm(landed.String(), url.Values{ + registerConfirmCSRFCookie: {csrf[1]}, + }) + require.NoError(t, err) + + confirmedBody, err := io.ReadAll(confirmed.Body) + confirmed.Body.Close() + require.NoError(t, err) + require.Equal(t, http.StatusOK, confirmed.StatusCode) + assert.Contains(t, string(confirmedBody), tt.heading, tt.name) + assert.Equal(t, tt.nodeCount, b.app.state.ListNodes().Len(), tt.name) + } +} + func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { t.Parallel() @@ -1745,7 +1822,7 @@ func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { }))) provider := &AuthProviderOIDC{h: app} - changed, err := provider.handleRegistration(user, authID, nil) + registered, err := provider.handleRegistration(user, authID, nil) require.ErrorIs(t, err, types.ErrHostinfoTooLarge) var httpErr HTTPError @@ -1754,7 +1831,7 @@ func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { assert.Contains(t, strings.ToLower(httpErr.UserMsg), "reduce") assert.Contains(t, httpErr.UserMsg, "metadata") assert.Contains(t, httpErr.UserMsg, "sign") - assert.False(t, changed) + assert.False(t, registered.Valid()) after, ok := app.state.GetNodeByID(node.ID) require.True(t, ok) diff --git a/hscontrol/templates/register_confirm.go b/hscontrol/templates/register_confirm.go index 8962cd239..d99474fdf 100644 --- a/hscontrol/templates/register_confirm.go +++ b/hscontrol/templates/register_confirm.go @@ -43,6 +43,9 @@ type RegisterConfirmInfo struct { // MachineKey is the short fingerprint of the registering machine // key. The full key is intentionally not shown. MachineKey string + + // ExistingNode is the stored node name, empty for a new device. + ExistingNode string } // RegisterConfirm renders an interstitial page that asks the @@ -57,6 +60,7 @@ func RegisterConfirm(info RegisterConfirmInfo) *elem.Element { {"Hostname", elem.Text(info.Hostname)}, {"OS", elem.Text(cmp.Or(info.OS, "(unknown)"))}, {"Machine key", Code(elem.Text(info.MachineKey))}, + {"Existing node", elem.Text(cmp.Or(info.ExistingNode, "None, this is a new device"))}, {"Registered to", elem.Text(info.User)}, }, )