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
16 changes: 16 additions & 0 deletions internal/cmd/me/me.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,22 @@ func run(ctx context.Context, out, errOut io.Writer, idOnly, extended bool) erro
return fmt.Errorf("getting current user: %w", err)
}

// People's people/me only returns the account's own email when the token
// carries an email-bearing scope; gro requests userinfo.profile but not
// userinfo.email, so PrimaryEmail is blank on most tokens — while `config
// test` (Gmail profile) happily prints the address. `me` is the canonical
// identity surface, so fall back to the Gmail profile (gmail.modify is
// always granted) rather than rendering "-" where the email belongs.
// Best-effort: a fallback failure leaves the field blank instead of
// failing `me` — the People data is still worth showing. Adding the
// userinfo.email scope instead would force every existing user through a
// re-auth for data we can already read.
if profile.PrimaryEmail == "" {
if email, gerr := GmailEmailFactory(ctx); gerr == nil {
profile.PrimaryEmail = email
}
}

// Read token expiry AFTER the API call so any refresh by
// PersistentTokenSource is reflected.
var extras Extras
Expand Down
100 changes: 100 additions & 0 deletions internal/cmd/me/me_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ func withMockClient(t *testing.T, c PeopleClient) {
t.Cleanup(func() { ClientFactory = orig })
}

// withMockGmailEmail swaps GmailEmailFactory for the test and restores it on
// cleanup.
func withMockGmailEmail(t *testing.T, f func(ctx context.Context) (string, error)) {
t.Helper()
orig := GmailEmailFactory
GmailEmailFactory = f
t.Cleanup(func() { GmailEmailFactory = orig })
}

func TestRenderOneLinerHappyPath(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
Expand Down Expand Up @@ -177,6 +186,97 @@ func TestRunDefaultPipeOneLiner(t *testing.T) {
}
}

// TestRunFallsBackToGmailEmail is the regression test for the blank-email
// one-liner: gro requests userinfo.profile but not userinfo.email, so
// people/me returns no email and `gro me` printed "-" where `config test`
// printed the address. The Gmail-profile fallback must fill it in.
func TestRunFallsBackToGmailEmail(t *testing.T) {
// Not Parallel: mutates package-global ClientFactory + GmailEmailFactory.
withMockClient(t, &mockPeopleClient{
GetMeFunc: func(_ context.Context) (*people.Profile, error) {
return &people.Profile{
ResourceName: "people/c1",
DisplayName: "Ada",
PrimaryEmail: "", // what people/me returns without userinfo.email
}, nil
},
})
withMockGmailEmail(t, func(_ context.Context) (string, error) {
return "ada@example.com", nil
})
var out bytes.Buffer
if err := run(context.Background(), &out, &bytes.Buffer{}, false, false); err != nil {
t.Fatal(err)
}
want := "people/c1 | Ada | ada@example.com\n"
if got := out.String(); got != want {
t.Fatalf("got %q, want %q", got, want)
}
}

// TestRunFallbackFailureLeavesEmailBlank proves a Gmail hiccup degrades to
// the old "-" rendering instead of failing `me` — the People data is still
// worth showing.
func TestRunFallbackFailureLeavesEmailBlank(t *testing.T) {
// Not Parallel: mutates package-global ClientFactory + GmailEmailFactory.
withMockClient(t, &mockPeopleClient{
GetMeFunc: func(_ context.Context) (*people.Profile, error) {
return &people.Profile{ResourceName: "people/c1", DisplayName: "Ada"}, nil
},
})
withMockGmailEmail(t, func(_ context.Context) (string, error) {
return "", errors.New("gmail unavailable")
})
var out bytes.Buffer
if err := run(context.Background(), &out, &bytes.Buffer{}, false, false); err != nil {
t.Fatal(err)
}
want := "people/c1 | Ada | -\n"
if got := out.String(); got != want {
t.Fatalf("got %q, want %q", got, want)
}
}

// TestRunNoFallbackWhenEmailPresent proves the extra Gmail call is skipped
// when People already supplied the email.
func TestRunNoFallbackWhenEmailPresent(t *testing.T) {
// Not Parallel: mutates package-global ClientFactory + GmailEmailFactory.
withMockClient(t, &mockPeopleClient{
GetMeFunc: func(_ context.Context) (*people.Profile, error) {
return &people.Profile{ResourceName: "people/c1", DisplayName: "Ada", PrimaryEmail: "ada@example.com"}, nil
},
})
withMockGmailEmail(t, func(_ context.Context) (string, error) {
t.Error("GmailEmailFactory must not be called when People supplied the email")
return "", nil
})
var out bytes.Buffer
if err := run(context.Background(), &out, &bytes.Buffer{}, false, false); err != nil {
t.Fatal(err)
}
}

// TestRunIDOnlyFallsBackToGmailEmail covers the scripting surface: `gro me
// --id` printing "-" is worse than useless in a pipeline.
func TestRunIDOnlyFallsBackToGmailEmail(t *testing.T) {
// Not Parallel: mutates package-global ClientFactory + GmailEmailFactory.
withMockClient(t, &mockPeopleClient{
GetMeFunc: func(_ context.Context) (*people.Profile, error) {
return &people.Profile{ResourceName: "people/c1", DisplayName: "Ada"}, nil
},
})
withMockGmailEmail(t, func(_ context.Context) (string, error) {
return "ada@example.com", nil
})
var out bytes.Buffer
if err := run(context.Background(), &out, &bytes.Buffer{}, true, false); err != nil {
t.Fatal(err)
}
if got := out.String(); got != "ada@example.com\n" {
t.Fatalf("got %q, want 'ada@example.com\\n'", got)
}
}

func TestRunIDOnlyEmitsEmail(t *testing.T) {
// Not Parallel: mutates package-global ClientFactory.
withMockClient(t, &mockPeopleClient{
Expand Down
19 changes: 19 additions & 0 deletions internal/cmd/me/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"strings"

"github.com/open-cli-collective/google-cli-common/gmail"
"github.com/open-cli-collective/google-cli-common/keychain"
"github.com/open-cli-collective/google-cli-common/people"
)
Expand All @@ -20,6 +21,24 @@ var ClientFactory = func(ctx context.Context) (PeopleClient, error) {
return people.NewClient(ctx)
}

// GmailEmailFactory resolves the authenticated account's email address via
// the Gmail profile. It backs the fallback for People's missing email (see
// run): gro requests userinfo.profile but deliberately not userinfo.email,
// so people/me carries no email address on most tokens — while the Gmail
// profile always knows it under the always-granted gmail.modify scope.
// Override in tests to inject mocks.
var GmailEmailFactory = func(ctx context.Context) (string, error) {
c, err := gmail.NewClient(ctx)
if err != nil {
return "", err
}
p, err := c.GetProfile(ctx)
if err != nil {
return "", err
}
return p.EmailAddress, nil
}

// Extras is the data shown by --extended that doesn't come from People.
type Extras struct {
GrantedScopes []string
Expand Down
Loading