Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,8 @@ import (
"time"
)

// Built-in OAuth client credentials for the CLI app.
const (
oauthClientID = "khMWSVDVSq78oyKA3KtxmYRv"
installID = "hey-cli"
)
// Built-in OAuth client ID for the CLI app.
const oauthClientID = "khMWSVDVSq78oyKA3KtxmYRv"

type callbackWaiter func(context.Context, string, string, net.Listener, LoginOptions) (string, error)
type listenerFactory func(context.Context, string, string) (net.Listener, error)
Expand Down Expand Up @@ -177,6 +174,11 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) error {
defer func() { _ = listener.Close() }()
redirectURI := "http://" + listener.Addr().String() + "/callback"

installID, err := m.store.InstallID()
if err != nil {
return fmt.Errorf("install id: %w", err)
}

state := generateState()
codeVerifier := generateCodeVerifier()
codeChallenge := generateCodeChallenge(codeVerifier)
Expand Down Expand Up @@ -295,6 +297,11 @@ func (m *Manager) refreshLocked(ctx context.Context, creds *Credentials) error {
tokenEndpoint = m.baseURL + "/oauth/tokens"
}

installID, err := m.store.installID()
if err != nil {
return fmt.Errorf("install id: %w", err)
}

token, err := refreshOAuthToken(ctx, m.httpClient, tokenEndpoint, creds.RefreshToken, oauthClientID, installID)
if err != nil {
return fmt.Errorf("token refresh failed: %w", err)
Expand Down
11 changes: 11 additions & 0 deletions internal/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,17 @@ func TestNormalizeBaseURL(t *testing.T) {

func TestLoginOAuthFlow(t *testing.T) {
redirectURIs := make(chan string, 1)
var installID string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/oauth/tokens" {
t.Errorf("path = %q, want /oauth/tokens", r.URL.Path)
}
if err := r.ParseForm(); err != nil {
t.Fatalf("ParseForm: %v", err)
}
if got := r.Form.Get("install_id"); got != installID {
t.Errorf("install_id = %q, want this install's %q", got, installID)
}
if got := r.Form.Get("code"); got != "callback-code" {
t.Errorf("code = %q, want callback-code", got)
}
Expand All @@ -123,6 +127,10 @@ func TestLoginOAuthFlow(t *testing.T) {
}
return listen(ctx, network, address)
}
var err error
if installID, err = mgr.GetStore().InstallID(); err != nil {
t.Fatalf("InstallID: %v", err)
}
mgr.callbackWait = func(_ context.Context, state, authURL string, listener net.Listener, opts LoginOptions) (string, error) {
if state == "" {
t.Error("state is empty")
Expand Down Expand Up @@ -710,6 +718,9 @@ func TestConcurrentManagersRefreshOnce(t *testing.T) {
if err := r.ParseForm(); err != nil {
t.Fatalf("ParseForm: %v", err)
}
if got := r.Form.Get("install_id"); got == "" || got == "hey-cli" {
t.Errorf("install_id = %q, want a per-install identifier", got)
}
// Rotation: the refresh token is spent by the first refresh that presents it.
if r.Form.Get("refresh_token") != "first-refresh" {
w.WriteHeader(http.StatusUnauthorized)
Expand Down
111 changes: 111 additions & 0 deletions internal/auth/install_id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package auth

import (
"crypto/rand"
"fmt"
"os"
"path/filepath"
"regexp"
"strings"
)

// InstallID identifies this install to HEY as a device, minting the identifier on first
// use. It lives beside the credentials rather than in them: a device outlasts a logout,
// and HEY alerts on a sign-in from a device it hasn't seen.
func (s *Store) InstallID() (string, error) {
unlock, err := s.lock()
if err != nil {
return "", err
}
defer unlock()

return s.installID()
}

// installID is the unlocked variant, for a caller already holding the store lock.
func (s *Store) installID() (string, error) {
path := s.installIDPath()

data, err := os.ReadFile(path) //nolint:gosec // G304: path built from the store's own config directory
if err == nil {
// Only a well-formed identifier is a usable identity. A truncated or
// garbage file — an earlier write interrupted by a crash or a full
// disk, say — must not be adopted and sent to HEY on every login and
// refresh, so fall through and mint a fresh one over it.
if id := strings.TrimSpace(string(data)); isInstallID(id) {
return id, nil
}
} else if !os.IsNotExist(err) {
return "", err
}

id := newInstallID()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve legacy installation IDs during credential migration

For users upgrading from the old password-grant CLI who supplied its supported --install-id option, migrateOldCredentials migrates the tokens but OldConfig does not read the existing install_id, and ScrubLegacyCredentials then deletes it. This path consequently mints a different UUID here before the next refresh; unlike the documented placeholder-to-real transition, that changes an already-custom device identity and can make a refresh-token lineage bound to the old ID fail. Migrate a valid legacy installation ID into this file before scrubbing the old config.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not doing this — leaving it for a human call. The premise is partly right: install_id is in legacyCredentialKeys, so the old config.json can carry one. But OldConfig deliberately omits install_id together with client_id/client_secret — the redesign does not carry the old password-grant client identity forward at all. Re-adopting only the legacy install_id while still dropping the client it was minted under would be incoherent: a password-grant refresh token is not refreshable by the new OAuth client regardless of install_id, so install_id is not the load-bearing factor in that lineage. A fresh device identity (and HEYs new-device signal) on the first OAuth login after upgrade is the intended behavior of this change, not a regression to paper over. It also reaches into migration code (OldConfig, migrateOldCredentials) outside this PRs diff. Flagging for a human decision rather than acting on it.

if err := os.MkdirAll(s.fallbackDir, 0700); err != nil {
return "", err
}
if err := writeFileAtomic(path, []byte(id+"\n"), 0600); err != nil {
return "", err
}
return id, nil
}

// installIDPattern is the canonical version-4 UUID shape newInstallID mints and
// the mobile apps send.
var installIDPattern = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`)

func isInstallID(id string) bool {
return installIDPattern.MatchString(id)
}

// writeFileAtomic writes data to a temporary mode-perm file in the destination
// directory and renames it into place. A crash or full disk mid-write then
// leaves the previous file (or none) rather than a truncated one that the next
// run would mistake for a valid identifier.
func writeFileAtomic(path string, data []byte, perm os.FileMode) error {
tmp, err := os.CreateTemp(filepath.Dir(path), ".install_id-*")
if err != nil {
return err
}
tmpName := tmp.Name()
defer func() {
if tmpName != "" {
_ = os.Remove(tmpName)
}
}()

if err := tmp.Chmod(perm); err != nil {
_ = tmp.Close()
return err
}
if _, err := tmp.Write(data); err != nil {
_ = tmp.Close()
return err
}
if err := tmp.Sync(); err != nil {
_ = tmp.Close()
return err
}
if err := tmp.Close(); err != nil {
return err
}
if err := os.Rename(tmpName, path); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make install-ID replacement atomic on Windows

On Windows, which this repository ships, Go explicitly does not guarantee that os.Rename is atomic even within one directory. If the process or machine stops while replacing a malformed install_id, the destination can disappear; the next refresh then mints another UUID, which no longer matches the server-bound refresh-token lineage and prevents authentication until login is repeated. This is fresh evidence beyond the earlier comment: the new hardening now relies on os.Rename for its crash-safety guarantee, so the replacement needs a Windows-specific atomic implementation.

Useful? React with 👍 / 👎.

return err
}
tmpName = "" // renamed into place; nothing to clean up
return nil
}

func (s *Store) installIDPath() string {
return filepath.Join(s.fallbackDir, "install_id")
}

// newInstallID is a random version-4 UUID, the shape the mobile apps send.
func newInstallID() string {
b := make([]byte, 16)
if _, err := rand.Read(b); err != nil {
panic("crypto/rand failed: " + err.Error())
}
b[6] = (b[6] & 0x0f) | 0x40
b[8] = (b[8] & 0x3f) | 0x80
return fmt.Sprintf("%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:16])
}
101 changes: 101 additions & 0 deletions internal/auth/install_id_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package auth

import (
"os"
"path/filepath"
"regexp"
"testing"
)

var uuidV4 = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`)

func TestInstallIDIsMintedOnceAndPersists(t *testing.T) {
t.Setenv("HEY_NO_KEYRING", "1")
configDir := t.TempDir()
store := NewStore(configDir)

first, err := store.InstallID()
if err != nil {
t.Fatalf("InstallID: %v", err)
}
if !uuidV4.MatchString(first) {
t.Fatalf("install id = %q, want a v4 UUID", first)
}

second, err := store.InstallID()
if err != nil {
t.Fatalf("InstallID: %v", err)
}
if second != first {
t.Errorf("install id changed between calls: %q then %q", first, second)
}

if other, _ := NewStore(configDir).InstallID(); other != first {
t.Errorf("install id = %q from a second store on the same directory, want %q", other, first)
}

info, err := os.Stat(filepath.Join(configDir, "install_id"))
if err != nil {
t.Fatalf("Stat: %v", err)
}
if perm := info.Mode().Perm(); perm != 0600 {
t.Errorf("install_id mode = %o, want 0600", perm)
}
}

func TestInstallIDSurvivesLogout(t *testing.T) {
t.Setenv("HEY_NO_KEYRING", "1")
configDir := t.TempDir()
mgr := NewManager("https://app.hey.com", nil, configDir)

id, err := mgr.GetStore().InstallID()
if err != nil {
t.Fatalf("InstallID: %v", err)
}
if err := mgr.LoginWithToken("token"); err != nil {
t.Fatalf("LoginWithToken: %v", err)
}
if err := mgr.Logout(); err != nil {
t.Fatalf("Logout: %v", err)
}

if after, _ := mgr.GetStore().InstallID(); after != id {
t.Errorf("install id = %q after logout, want %q", after, id)
}
}

func TestInstallIDsDifferPerInstall(t *testing.T) {
t.Setenv("HEY_NO_KEYRING", "1")
a, _ := NewStore(t.TempDir()).InstallID()
b, _ := NewStore(t.TempDir()).InstallID()
if a == b {
t.Errorf("two installs share install id %q", a)
}
}

func TestInstallIDReplacesAMalformedFile(t *testing.T) {
t.Setenv("HEY_NO_KEYRING", "1")
configDir := t.TempDir()
path := filepath.Join(configDir, "install_id")

// A truncated or garbage file — e.g. a write interrupted by a crash or a
// full disk, or the old constant "hey-cli" identifier — is not a usable
// identity and must be reminted, never sent to HEY as-is.
if err := os.WriteFile(path, []byte("hey-cli"), 0600); err != nil {
t.Fatalf("seed: %v", err)
}

id, err := NewStore(configDir).InstallID()
if err != nil {
t.Fatalf("InstallID: %v", err)
}
if !uuidV4.MatchString(id) {
t.Fatalf("install id = %q, want a v4 UUID", id)
}

// The mint is durable: the replacement is written back, so a second store
// reads the same value rather than reminting again.
if again, _ := NewStore(configDir).InstallID(); again != id {
t.Errorf("install id = %q on reload, want the reminted %q", again, id)
}
}
20 changes: 19 additions & 1 deletion internal/cmd/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,16 @@ func newAuthStatusCommand() *cobra.Command {
"authenticated": false,
}

// The install identifier is install-scoped: it survives logout and
// is sent on every OAuth login and refresh. Surface it in every
// status path — env token and logged-out included, both of which
// return before the signed-in path — so the JSON and styled output
// stay consistent and can diagnose HEY's new-device alerts.
installID, _ := authMgr.GetStore().InstallID()
if installID != "" {
status["install_id"] = installID
}

if os.Getenv("HEY_TOKEN") != "" {
status["authenticated"] = true
status["method"] = "env_var"
Expand All @@ -154,6 +164,9 @@ func newAuthStatusCommand() *cobra.Command {
w := cmd.OutOrStdout()
fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL)
fmt.Fprintf(w, "Mail: %s (%s)\n", cfg.AccountID, cfg.SourceOf("account_id"))
if installID != "" {
fmt.Fprintf(w, "Install: %s\n", installID)
}
fmt.Fprintln(w, "Status: Logged in (via HEY_TOKEN env var)")
return nil
}
Expand All @@ -167,6 +180,9 @@ func newAuthStatusCommand() *cobra.Command {
w := cmd.OutOrStdout()
fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL)
fmt.Fprintf(w, "Mail: %s (%s)\n", cfg.AccountID, cfg.SourceOf("account_id"))
if installID != "" {
fmt.Fprintf(w, "Install: %s\n", installID)
}
fmt.Fprintln(w, "Status: Not logged in")
return nil
}
Expand Down Expand Up @@ -196,7 +212,6 @@ func newAuthStatusCommand() *cobra.Command {
if creds.RefreshToken != "" {
status["refresh_available"] = true
}

if writer.IsStyled() {
w := cmd.OutOrStdout()
fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL)
Expand All @@ -216,6 +231,9 @@ func newAuthStatusCommand() *cobra.Command {
fmt.Fprintf(w, "Cookie: %s...%s\n", cookie[:8], cookie[len(cookie)-4:])
}
}
if installID != "" {
fmt.Fprintf(w, "Install: %s\n", installID)
}

if creds.ExpiresAt > 0 {
expiry := time.Unix(creds.ExpiresAt, 0)
Expand Down
Loading
Loading