Skip to content
Merged
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
24 changes: 20 additions & 4 deletions internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,8 +1496,12 @@ func (m *Manager) GetUserEmail() string {
// whoever the stored credentials belong to; writing it there would
// mislabel them. Skipping the store also keeps a token session off the
// keyring probe and the fallback warning it can raise.
//
// An empty email is an omission, not a value: an in-house (bc3) token's
// authorization document carries only the identity id, and a caller
// relaying that must not blank what a login stored.
func (m *Manager) SetUserEmail(email string) error {
if os.Getenv("BASECAMP_TOKEN") != "" {
if os.Getenv("BASECAMP_TOKEN") != "" || email == "" {
return nil
}

Expand All @@ -1510,15 +1514,27 @@ func (m *Manager) SetUserEmail(email string) error {
return m.store.Save(credKey, creds)
}

// SetUserIdentity stores the user ID and email for the current credential key.
// SetUserIdentity stores the user ID and email for the current credential
// key. As with SetUserEmail, an empty value leaves the stored field alone.
// Unlike it, BASECAMP_TOKEN does not suppress the write: a login stores
// its new credential and then records who it verified as, whatever the
// environment holds, so the caller decides whose identity this is.
func (m *Manager) SetUserIdentity(userID, email string) error {
if userID == "" && email == "" {
return nil
}

credKey := m.credentialKey()
creds, err := m.store.Load(credKey)
if err != nil {
return err
}
creds.UserID = userID
creds.UserEmail = email
if userID != "" {
creds.UserID = userID
}
if email != "" {
creds.UserEmail = email
}
return m.store.Save(credKey, creds)
}

Expand Down
43 changes: 43 additions & 0 deletions internal/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2167,3 +2167,46 @@ func TestRefresh_HalfConfiguredClientHintsTheProfile(t *testing.T) {
assert.Contains(t, cliErr.Message, "BASECAMP_OAUTH_CLIENT_SECRET is required")
assert.Equal(t, "Run: basecamp auth login -P work", cliErr.Hint)
}

// TestSetUserIdentity_EmptyValuesAreOmissions: an authorization document
// that names only an identity id must not blank the name a login stored.
func TestSetUserIdentity_EmptyValuesAreOmissions(t *testing.T) {
t.Setenv("BASECAMP_TOKEN", "")
m := &Manager{cfg: config.Default(), store: newTestStore(t, t.TempDir())}
key := m.credentialKey()
require.NoError(t, m.store.Save(key, &Credentials{
AccessToken: "tok", OAuthType: "bc5", UserID: "1", UserEmail: "kept@example.com",
}))

require.NoError(t, m.SetUserEmail(""))
require.NoError(t, m.SetUserIdentity("", ""))
creds, err := m.store.Load(key)
require.NoError(t, err)
assert.Equal(t, "1", creds.UserID)
assert.Equal(t, "kept@example.com", creds.UserEmail)

require.NoError(t, m.SetUserIdentity("2", ""))
creds, err = m.store.Load(key)
require.NoError(t, err)
assert.Equal(t, "2", creds.UserID)
assert.Equal(t, "kept@example.com", creds.UserEmail, "an omitted email leaves the stored one")

require.NoError(t, m.SetUserEmail("new@example.com"))
assert.Equal(t, "new@example.com", m.GetUserEmail())
}

// TestSetUserIdentity_WritesUnderEnvToken: a login records who its new
// credential verified as whatever BASECAMP_TOKEN holds; the environment
// token is the caller's concern (me skips the write), not this method's.
func TestSetUserIdentity_WritesUnderEnvToken(t *testing.T) {
t.Setenv("BASECAMP_TOKEN", "bc_at_env")
m := &Manager{cfg: config.Default(), store: newTestStore(t, t.TempDir())}
key := m.credentialKey()
require.NoError(t, m.store.Save(key, &Credentials{AccessToken: "tok", OAuthType: "bc5"}))

require.NoError(t, m.SetUserIdentity("2", "who@example.com"))
creds, err := m.store.Load(key)
require.NoError(t, err)
assert.Equal(t, "2", creds.UserID)
assert.Equal(t, "who@example.com", creds.UserEmail)
}
9 changes: 7 additions & 2 deletions internal/commands/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,9 @@ type loginIdentity struct {
}

// label renders the identity for a one-line terminal sink. Name and email
// are server-supplied, so they are reduced to single lines first.
// are server-supplied, so they are reduced to single lines first. An
// identity the server reported without either (an in-house bc3 token's)
// is named by its ids alone rather than as a blank with ids in brackets.
func (l *loginIdentity) label() string {
label := richtext.SanitizeSingleLine(l.Name)
if email := richtext.SanitizeSingleLine(l.Email); email != "" {
Expand All @@ -701,7 +703,10 @@ func (l *loginIdentity) label() string {
if l.PersonID != 0 {
parts = append(parts, fmt.Sprintf("person %d", l.PersonID))
}
if len(parts) > 0 {
switch {
case strings.TrimSpace(label) == "":
return strings.Join(parts, ", ")
case len(parts) > 0:
label += " (" + strings.Join(parts, ", ") + ")"
}
return strings.TrimSpace(label)
Expand Down
18 changes: 18 additions & 0 deletions internal/commands/auth_identity_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package commands

import (
"testing"

"github.com/stretchr/testify/assert"
)

// TestLoginIdentityLabelWithoutNameOrEmail: an authorization document that
// reports only an identity id (an in-house bc3 token's) is named by its
// ids, never as a blank name with the ids in brackets.
func TestLoginIdentityLabelWithoutNameOrEmail(t *testing.T) {
assert.Equal(t, "identity 28142355", (&loginIdentity{IdentityID: 28142355}).label())
assert.Equal(t, "identity 28142355, person 51177542", (&loginIdentity{IdentityID: 28142355, PersonID: 51177542}).label())
assert.Equal(t, "identity 28142355", (&loginIdentity{IdentityID: 28142355, Name: " \t"}).label())
assert.Equal(t, "Ada <ada@example.com> (identity 28142355)", (&loginIdentity{IdentityID: 28142355, Name: "Ada", Email: "ada@example.com"}).label())
assert.Equal(t, "<ada@example.com> (identity 28142355)", (&loginIdentity{IdentityID: 28142355, Email: "ada@example.com"}).label())
}
74 changes: 65 additions & 9 deletions internal/commands/people.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"net/mail"
"os"
"slices"
"sort"
"strconv"
Expand All @@ -25,6 +26,18 @@ import (
type MeOutput struct {
Identity basecamp.Identity `json:"identity"`
Accounts []AccountInfo `json:"accounts"`
// Person is the account-scoped person record, looked up when the
// authorization document names no one (an in-house bc3 token reports
// only the identity id) and an account is configured to ask.
Person *MePerson `json:"person,omitempty"`
}

// MePerson is the person the credential resolves to within the configured
// account.
type MePerson struct {
ID int64 `json:"id"`
Name string `json:"name"`
Email string `json:"email"`
}

// AccountInfo represents an account in the me command output
Expand Down Expand Up @@ -67,12 +80,44 @@ func runMe(cmd *cobra.Command, args []string) error {
return convertSDKError(err)
}

// Store user email for display purposes (non-fatal if fails).
_ = app.Auth.SetUserEmail(authInfo.Identity.EmailAddress)
currentAccountID := app.Config.AccountID

// Who the credential belongs to, for the summary and the stored label.
// The authorization document is the first word; an in-house (bc3)
// token's document carries only the identity id, and then the configured
// account's person record — the same lookup a login verifies against —
// fills in the name and email. That lookup is best-effort: a person the
// account cannot resolve leaves the identity as reported.
name := strings.TrimSpace(authInfo.Identity.FirstName + " " + authInfo.Identity.LastName)
email := authInfo.Identity.EmailAddress
var person *MePerson
if (name == "" || email == "") && app.RequireAccount() == nil && authorizesAccount(authInfo, currentAccountID) {
if p, err := app.Account().People().Me(cmd.Context()); err == nil {
person = &MePerson{ID: p.ID, Name: p.Name, Email: p.EmailAddress}
// The person record fills gaps only; a field the authorization
// document already named is kept as its own.
if name == "" {
name = p.Name
}
if email == "" {
email = p.EmailAddress
}
}
}
// Stored for display purposes (non-fatal if it fails). Empty values are
// omissions and leave what a login stored in place. Under BASECAMP_TOKEN
// nothing is written: what was learned names the environment token's
// user, not whoever the stored credential belongs to.
switch {
case os.Getenv("BASECAMP_TOKEN") != "":
case person != nil:
_ = app.Auth.SetUserIdentity(strconv.FormatInt(person.ID, 10), email)
default:
_ = app.Auth.SetUserEmail(email)
}

// Build account output (already filtered to bc3 by SDK)
var accounts []AccountInfo
currentAccountID := app.Config.AccountID
for _, acct := range authInfo.Accounts {
info := AccountInfo{
ID: acct.ID,
Expand All @@ -90,14 +135,10 @@ func runMe(cmd *cobra.Command, args []string) error {
result := MeOutput{
Identity: authInfo.Identity,
Accounts: accounts,
Person: person,
}

// Build summary
name := authInfo.Identity.FirstName
if authInfo.Identity.LastName != "" {
name += " " + authInfo.Identity.LastName
}
summary := fmt.Sprintf("%s <%s>", name, authInfo.Identity.EmailAddress)
summary := meLabel(authInfo.Identity.ID, name, email)
if len(accounts) > 0 {
summary += fmt.Sprintf(" - %d Basecamp account(s)", len(accounts))
}
Expand Down Expand Up @@ -130,6 +171,21 @@ func runMe(cmd *cobra.Command, args []string) error {
)
}

// meLabel names the authenticated user for the summary line: "Name <email>"
// with whichever parts are known, or the bare identity id when neither is.
func meLabel(identityID int64, name, email string) string {
switch {
case name != "" && email != "":
return name + " <" + email + ">"
case name != "":
return name
case email != "":
return email
default:
return fmt.Sprintf("identity %d", identityID)
}
}

// updateAccountsCache updates the completion cache with account data.
// Runs synchronously; errors are ignored (best-effort).
func updateAccountsCache(accounts []AccountInfo, cacheDir string) {
Expand Down
Loading