refactor: no group list in an account's details, and no Microsoft Graph call for it

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs/azure.md

@@ -16,12 +16,10 @@ A service principal is used instead when `AZURE_CLIENT_ID`,
 On an account's group row, the fourth pane shows who it signs in as: name,
 user, tenant, object ID, whether the session used MFA, directory roles
 (built-in ones by name) and its subscriptions, read from az's account list
-and the token lazysecrets already has. Below them are the groups the
+and the token lazysecrets already has: no requests of their own. `y` on
-account is in, nested ones too: those come from Microsoft Graph, once a
+the row copies the object ID, the one access policies and role
-session, the first time the pane shows them. `y` on the row copies the
+assignments ask for; `l` goes into the pane, where `j`/`k` pick any
-object ID, the one access policies and role assignments ask for; `tab`
+field, `/` filters them (`/tenant`) and `y` copies the selected one.
-goes into the pane, where `j`/`k` pick any field or group, `/` filters
-them (`/tenant`, `/platform`) and `y` copies the selected one.
 
 If an account's az login has expired, the status line says so once, and the
 Details pane on that account's rows says it until you sign in: press `L`

internal/provider/azure/account.go

@@ -1,15 +1,10 @@
 package azure
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
-	"net/http"
 	"slices"
 	"strings"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
-
 	"git.kebcloud.no/keb/lazysecrets/internal/provider"
 )
 
@@ -160,68 +155,3 @@ func directoryRoles(wids any) string {
 	slices.Sort(names)
 	return strings.Join(names, ", ")
 }
-
-// graphGroups lists the signed-in user's groups, nested ones too, from
-// Microsoft Graph with az's token for it.
-const graphGroups = "https://graph.microsoft.com/v1.0/me/transitiveMemberOf?$select=displayName&$top=999"
-
-var _ provider.AccountGroups = (*Provider)(nil)
-
-// Groups lists the groups the account is a member of, by name.
-func (p *Provider) Groups(ctx context.Context) ([]string, error) {
-	if p.account == nil || (p.account.cloud != "" && p.account.cloud != "AzureCloud") {
-		return nil, provider.ErrUnsupported
-	}
-	tok, err := p.cred.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{"https://graph.microsoft.com/.default"}})
-	if err != nil {
-		return nil, err
-	}
-	var names []string
-	for next := graphGroups; next != ""; {
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, next, nil)
-		if err != nil {
-			return nil, err
-		}
-		req.Header.Set("Authorization", "Bearer "+tok.Token)
-		resp, err := p.http().Do(req)
-		if err != nil {
-			return nil, err
-		}
-		var page struct {
-			Value []struct {
-				Type string `json:"@odata.type"`
-				Name string `json:"displayName"`
-			} `json:"value"`
-			Next  string `json:"@odata.nextLink"`
-			Error *struct {
-				Message string `json:"message"`
-			} `json:"error"`
-		}
-		err = json.NewDecoder(resp.Body).Decode(&page)
-		resp.Body.Close()
-		switch {
-		case err != nil:
-			return nil, fmt.Errorf("listing groups from Microsoft Graph: %w", err)
-		case page.Error != nil:
-			return nil, fmt.Errorf("listing groups from Microsoft Graph: %s", page.Error.Message)
-		case resp.StatusCode != http.StatusOK:
-			return nil, fmt.Errorf("listing groups from Microsoft Graph: %s", resp.Status)
-		}
-		for _, v := range page.Value {
-			if v.Type == "#microsoft.graph.group" && v.Name != "" {
-				names = append(names, v.Name)
-			}
-		}
-		next = page.Next
-	}
-	slices.SortFunc(names, func(a, b string) int { return strings.Compare(strings.ToLower(a), strings.ToLower(b)) })
-	return names, nil
-}
-
-// http is the client for Microsoft Graph; tests replace it.
-func (p *Provider) http() *http.Client {
-	if p.graphClient != nil {
-		return p.graphClient
-	}
-	return http.DefaultClient
-}

internal/provider/azure/account_test.go

@@ -1,17 +1,7 @@
 package azure
 
 import (
-	"context"
-	"errors"
-	"io"
-	"net/http"
-	"slices"
-	"strings"
 	"testing"
-	"time"
-
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 
 	"git.kebcloud.no/keb/lazysecrets/internal/provider"
 )
@@ -75,40 +65,3 @@ func TestSignInMethods(t *testing.T) {
 		}
 	}
 }
-
-type staticToken struct{}
-
-func (staticToken) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) {
-	return azcore.AccessToken{Token: "tok", ExpiresOn: time.Now().Add(time.Hour)}, nil
-}
-
-type graphPages map[string]string // by URL
-
-func (g graphPages) RoundTrip(r *http.Request) (*http.Response, error) {
-	body, ok := g[r.URL.String()]
-	if !ok || r.Header.Get("Authorization") != "Bearer tok" {
-		return &http.Response{StatusCode: 404, Status: "404 Not Found", Body: io.NopCloser(strings.NewReader(`{}`))}, nil
-	}
-	return &http.Response{StatusCode: 200, Status: "200 OK", Body: io.NopCloser(strings.NewReader(body))}, nil
-}
-
-func TestGroups(t *testing.T) {
-	next := "https://graph.microsoft.com/v1.0/next"
-	p := &Provider{
-		cred:    staticToken{},
-		account: &accountDetails{account: account{user: "alice@contoso.com"}},
-		graphClient: &http.Client{Transport: graphPages{
-			graphGroups: `{"value":[{"@odata.type":"#microsoft.graph.group","displayName":"vault readers"},
-				{"@odata.type":"#microsoft.graph.directoryRole","displayName":"Global Reader"}],"@odata.nextLink":"` + next + `"}`,
-			next: `{"value":[{"@odata.type":"#microsoft.graph.group","displayName":"Platform team"}]}`,
-		}},
-	}
-	got, err := p.Groups(context.Background())
-	if err != nil || !slices.Equal(got, []string{"Platform team", "vault readers"}) {
-		t.Errorf("groups %q, %v", got, err)
-	}
-	p.account.cloud = "AzureChinaCloud"
-	if _, err := p.Groups(context.Background()); !errors.Is(err, provider.ErrUnsupported) {
-		t.Errorf("other cloud: %v", err)
-	}
-}

internal/provider/azure/azure.go

@@ -9,7 +9,6 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
-	"net/http"
 	"regexp"
 	"slices"
 	"strings"
@@ -37,12 +36,11 @@ var objectTypes = []provider.ObjectType{provider.TypeSecret, provider.TypeKey, p
 type Provider struct {
 	id, label string // "" for the single-account defaults
 
-	cred        azcore.TokenCredential
+	cred    azcore.TokenCredential
-	opts        options
+	opts    options
-	account     *accountDetails // nil for a service principal from AZURE_*
+	account *accountDetails // nil for a service principal from AZURE_*
-	graphClient *http.Client    // for Groups; nil for the default
+	subs    *armsubscriptions.Client
-	subs        *armsubscriptions.Client
+	graph   *armresourcegraph.Client
-	graph       *armresourcegraph.Client
 
 	mu      sync.Mutex
 	vaults  map[string]*vaultClients // by vault URL

internal/provider/provider.go

@@ -227,10 +227,3 @@ func (d *Denied) Unwrap() error { return d.Err }
 type ContainerDetails interface {
 	Details(ctx context.Context, c Container) ([]Attr, error)
 }
-
-// AccountGroups is implemented by providers that can list the groups
-// their account is a member of, directly or through other groups. It
-// goes over the network.
-type AccountGroups interface {
-	Groups(ctx context.Context) ([]string, error)
-}

internal/ui/app/app.go

@@ -172,12 +172,10 @@ type Model struct {
 	clip      Clipboard
 	clipClear time.Duration
 	openURL   func(string) error
-	// fields is the fourth pane for anything but a secret, with each
+	// fields is the fourth pane for anything but a secret; lastLeft is
-	// account's groups as they load; lastLeft is the left pane it
+	// the left pane it describes while the fourth has focus (fields.go).
-	// describes while the fourth has focus (fields.go).
 	fields   panes.Fields
 	lastLeft pane
-	groups   map[string]groupsState
 	details  map[string]detailsState // by containerKey
 	getenv   func(string) string
 	nvimOpen func(server, path string, line int) error // edit.go
@@ -377,7 +375,6 @@ func New(providers []provider.Provider, opts Options) Model {
 		getenv:        os.Getenv,
 		nvimOpen:      nvimOpen,
 		cache:         newCache(),
-		groups:        map[string]groupsState{},
 		details:       map[string]detailsState{},
 		fields:        panes.NewFields(),
 	}
@@ -590,9 +587,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	case persistedMsg:
 		m.handlePersisted(msg)
 
-	case groupsMsg:
-		m.groups[msg.provider] = groupsState{names: msg.names, err: msg.err}
-
 	case detailsMsg:
 		if msg.req == m.reqID[slotDetails] {
 			m.done(slotDetails)

internal/ui/app/app_test.go

@@ -1106,25 +1106,16 @@ func TestAccountInDetails(t *testing.T) {
 	}
 }
 
-// grouped is an account in two groups.
-type grouped struct{ described }
-
-func (grouped) Groups(context.Context) ([]string, error) {
-	return []string{"Platform team", "Vault readers"}, nil
-}
-
 func TestAccountPane(t *testing.T) {
 	t.Parallel()
-	d := newDriver(t, grouped{described{fake.Azure(0)}})
+	d := newDriver(t, described{fake.Azure(0)})
-	d.keys("g", "tab") // the account's row, then straight to its pane
+	d.keys("g", "l") // the account's row, then its pane
 	if d.m.focus != paneDetail {
 		t.Fatalf("focus %v, want the fourth pane", d.m.focus)
 	}
 	scr := d.screen()
-	for _, want := range []string{"Alice Example", "groups 2", "Platform team", "Vault readers"} {
+	if !strings.Contains(scr, "Alice Example") {
-		if !strings.Contains(scr, want) {
+		t.Errorf("account not shown:\n%s", scr)
-			t.Errorf("missing %q:\n%s", want, scr)
-		}
 	}
 	bar := lastLine(scr)
 	if !strings.Contains(bar, "copy") || strings.Contains(bar, "reveal") || strings.Contains(bar, "versions") {
@@ -1134,28 +1125,20 @@ func TestAccountPane(t *testing.T) {
 	if got := d.clip().content; got != "oid-123" {
 		t.Errorf("y on the object ID copied %q", got)
 	}
-	d.keys("G", "y")
-	if got := d.clip().content; got != "Vault readers" {
-		t.Errorf("y on a group copied %q", got)
-	}
 
 	// / filters it, labels too.
 	d.keys("/")
-	d.typeText("vault")
+	d.typeText("object")
 	d.keys("enter")
-	if scr := d.screen(); !strings.Contains(scr, "Vault readers") || strings.Contains(scr, "Platform team") {
+	if scr := d.screen(); !strings.Contains(scr, "oid-123") || strings.Contains(scr, "Alice Example") {
 		t.Errorf("filtered:\n%s", scr)
 	}
-	d.keys("y")
-	if got := d.clip().content; got != "Vault readers" {
-		t.Errorf("y in the filtered pane copied %q", got)
-	}
 	d.keys("esc")
-	if !strings.Contains(d.screen(), "Platform team") {
+	if !strings.Contains(d.screen(), "Alice Example") {
 		t.Errorf("filter not cleared:\n%s", d.screen())
 	}
 
-	d.keys("shift+tab")
+	d.keys("h")
 	if d.m.focus != paneSources {
 		t.Errorf("back to %v, want Sources", d.m.focus)
 	}

internal/ui/app/fields.go

@@ -1,7 +1,6 @@
 package app
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"strings"
@@ -14,20 +13,7 @@ import (
 )
 
 // The fourth pane shows Fields for anything but a secret. On a provider's
-// row: its account, from what the provider knows locally, then the
+// row: its account, from what the provider knows locally.
-// groups the account is in, loaded once a session when first shown.
-
-type groupsState struct {
-	names   []string
-	err     error
-	loading bool
-}
-
-type groupsMsg struct {
-	provider string
-	names    []string
-	err      error
-}
 
 // A vault's, scope's or file's details beyond its listing load once a
 // session, when the cursor rests on it, and again with refresh. They're
@@ -57,7 +43,6 @@ func (m Model) detailOf() pane {
 // syncFields shows the Fields pane for what the focused pane has selected,
 // unless that's a secret (or a vault with the Secrets pane in focus), which
 // the secret pane shows, as it does a revealed value whatever the focus.
-// An account's groups load the first time.
 func (m *Model) syncFields() tea.Cmd {
 	if m.detail.State() != panes.ValueHidden {
 		// A value revealed (or being fetched) from any pane stays in
@@ -76,7 +61,7 @@ func (m *Model) syncFields() tea.Cmd {
 		case m.selectedProblem() != "":
 			m.fields.Show(cmpOr(r.label, r.source.Name), m.problemFields(m.selectedProblem()))
 		case r.kind == rowGroup:
-			return m.showAccount(r)
+			m.showAccount(r)
 		case r.kind == rowSource:
 			m.fields.Show(r.source.Name, m.sourceFields(r.source))
 		default:
@@ -98,9 +83,8 @@ func (m *Model) syncFields() tea.Cmd {
 	return nil
 }
 
-// showAccount shows a provider's account, loading its groups the first
+// showAccount shows a provider's account.
-// time.
+func (m *Model) showAccount(r sourceRow) {
-func (m *Model) showAccount(r sourceRow) tea.Cmd {
 	p := m.byID[r.provider]
 	var fields []panes.Field
 	if a, ok := p.(provider.Accounts); ok {
@@ -110,28 +94,11 @@ func (m *Model) showAccount(r sourceRow) tea.Cmd {
 			}
 		}
 	}
-	var cmd tea.Cmd
-	if g, ok := p.(provider.AccountGroups); ok {
-		st, seen := m.groups[r.provider]
-		if !seen {
-			st = groupsState{loading: true}
-			m.groups[r.provider] = st
-			id, timeout := r.provider, m.timeout
-			cmd = func() tea.Msg {
-				ctx, cancel := context.WithTimeout(context.Background(), timeout)
-				defer cancel()
-				names, err := g.Groups(ctx)
-				return groupsMsg{provider: id, names: names, err: err}
-			}
-		}
-		fields = append(fields, groupFields(st)...)
-	}
 	if res, done := m.sourceResults[r.provider]; done && res.err == nil && len(res.sources) == 0 {
 		hint := emptyHint(p.Kind())
 		fields = append(fields, panes.Field{Label: "nothing", Value: "found: try", Info: true}, panes.Field{Value: hint.Value, URL: hint.URL})
 	}
 	m.fields.Show(p.Label(), fields)
-	return cmd
 }
 
 // sourceFields describe a subscription, workspace or repo.
@@ -248,30 +215,8 @@ func (m Model) problemFields(problem string) []panes.Field {
 	return out
 }
 
-// fieldName names a copied field: its label, or for a group, the group.
+// fieldName names a copied field: its label, or a row of several under
+// one label (a tag), its value.
 func fieldName(f panes.Field) string {
-	if f.Label == "" || strings.HasPrefix(f.Label, "groups") {
+	return cmpOr(f.Label, f.Value)
-		return "group " + f.Value
-	}
-	return f.Label
-}
-
-// groupFields are the groups' rows: labelled on the first.
-func groupFields(st groupsState) []panes.Field {
-	switch {
-	case st.loading:
-		return []panes.Field{{Label: "groups", Value: "loading…", Info: true}}
-	case errors.Is(st.err, provider.ErrUnsupported):
-		return nil
-	case st.err != nil:
-		return []panes.Field{{Label: "groups", Value: "couldn't list them: " + st.err.Error(), Info: true}}
-	case len(st.names) == 0:
-		return []panes.Field{{Label: "groups", Value: "none", Info: true}}
-	}
-	out := make([]panes.Field, len(st.names))
-	for i, n := range st.names {
-		out[i] = panes.Field{Value: n}
-	}
-	out[0].Label = fmt.Sprintf("groups %d", len(st.names))
-	return out
 }