From 57e545f1abb0fa9bd31648e8bc8b45594d72a931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl-Einar=20Bergstr=C3=B8m?= Date: Sun, 11 Oct 2026 09:41:57 +0200 Subject: [PATCH] fix: never show one az account's subscriptions under another Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/provider/azure/accounts.go | 4 +- internal/provider/azure/accounts_test.go | 74 +++++++++++++++++- internal/provider/azure/cli.go | 98 ++++++++++++++++++++---- 3 files changed, 158 insertions(+), 18 deletions(-) diff --git a/internal/provider/azure/accounts.go b/internal/provider/azure/accounts.go index 46d3da8..1c6d194 100644 --- a/internal/provider/azure/accounts.go +++ b/internal/provider/azure/accounts.go @@ -151,7 +151,7 @@ func NewAccounts(s Settings) ([]*Provider, error) { if len(accounts) == 0 { // Not signed in yet: the first call fails with an AuthError and // the UI offers az login. - cred := authCredential{cred: newCLICredential("", ""), login: []string{"az", "login"}} + cred := authCredential{cred: newCLICredential("", "", nil), login: []string{"az", "login"}} p, err := newWithOptions(cred, options{Settings: s}) if err != nil { return nil, err @@ -171,7 +171,7 @@ func NewAccounts(s Settings) ([]*Provider, error) { label = string(provider.KindAzure) + " ยท " + labels[i] } cred := authCredential{ - cred: newCLICredential(a.anchor, a.tenant), + cred: newCLICredential(a.user, a.tenant, func() ([]account, error) { return readAccounts(path) }), login: []string{"az", "login", "--tenant", a.tenant}, account: label, // so "sign-in required" says which account } diff --git a/internal/provider/azure/accounts_test.go b/internal/provider/azure/accounts_test.go index d2898f5..b9337de 100644 --- a/internal/provider/azure/accounts_test.go +++ b/internal/provider/azure/accounts_test.go @@ -2,6 +2,7 @@ package azure import ( "context" + "encoding/base64" "errors" "fmt" "os" @@ -11,6 +12,7 @@ import ( "testing" "time" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" ) @@ -67,7 +69,9 @@ func TestAccountLabels(t *testing.T) { func TestCLICredentialPinsAccount(t *testing.T) { var calls [][]string - c := newCLICredential("s-alice-2", "t1") + c := newCLICredential("alice@contoso.com", "t1", func() ([]account, error) { + return []account{{user: "alice@contoso.com", tenant: "t1", anchor: "s-alice-2"}}, nil + }) c.run = func(_ context.Context, args ...string) ([]byte, error) { calls = append(calls, args) exp := time.Now().Add(time.Hour).Unix() @@ -102,7 +106,7 @@ func TestCLICredentialPinsAccount(t *testing.T) { func TestCLICredentialRenewsAndReportsErrors(t *testing.T) { n := 0 - c := newCLICredential("", "") + c := newCLICredential("", "", nil) c.run = func(context.Context, ...string) ([]byte, error) { n++ if n == 2 { @@ -119,3 +123,69 @@ func TestCLICredentialRenewsAndReportsErrors(t *testing.T) { t.Errorf("err = %v", err) } } + +// jwt is an unsigned token with claims, as az would print it. +func jwt(claims string) string { + enc := base64.RawURLEncoding.EncodeToString + return enc([]byte(`{"alg":"none"}`)) + "." + enc([]byte(claims)) + ".sig" +} + +func TestCLICredentialFollowsAzAndRefusesOtherUsers(t *testing.T) { + // az's profile has one entry per subscription: signing in as bob, + // who also sees s-shared, moved it from alice to bob. + profile := []account{ + {user: "alice@contoso.com", tenant: "t1", anchor: "s-shared"}, + {user: "bob@contoso.com", tenant: "t1", anchor: "s-bob"}, + } + var args []string + issuedTo := "alice@contoso.com" + c := newCLICredential("alice@contoso.com", "t1", func() ([]account, error) { return profile, nil }) + c.run = func(_ context.Context, a ...string) ([]byte, error) { + args = a + exp := time.Now().Add(time.Hour).Unix() + return []byte(fmt.Sprintf(`{"accessToken": %q, "expires_on": %d}`, jwt(`{"upn":"`+issuedTo+`"}`), exp)), nil + } + arm := policy.TokenRequestOptions{Scopes: []string{"https://management.azure.com/.default"}} + if _, err := c.GetToken(context.Background(), arm); err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(args, " "), "--subscription s-shared") { + t.Errorf("args %q", args) + } + + // Now alice has another subscription, and az would hand out bob's + // token for the old one: the new pin is used, and bob's token refused. + profile[0].anchor = "s-alice" + c.cache = map[string]azcore.AccessToken{} + issuedTo = "bob@contoso.com" + _, err := c.GetToken(context.Background(), arm) + if !strings.Contains(strings.Join(args, " "), "--subscription s-alice") { + t.Errorf("args %q", args) + } + if err == nil || !strings.Contains(err.Error(), "token for bob@contoso.com, not alice@contoso.com") { + t.Errorf("err = %v", err) + } + + // Alice gone from az entirely: sign in again. + profile = profile[1:] + if _, err := c.GetToken(context.Background(), arm); err == nil || !strings.Contains(err.Error(), "sign in as alice@contoso.com") { + t.Errorf("err = %v", err) + } +} + +func TestTokenUser(t *testing.T) { + users := []string{"bob@contoso.com"} + for claims, want := range map[string]bool{ + `{"upn":"Bob@Contoso.com"}`: true, + `{"unique_name":"live.com#bob@contoso.com"}`: true, + `{"upn":"alice@contoso.com"}`: false, + `{"appid":"00000000-0000-0000-0000-000000000000"}`: false, + } { + if _, got := tokenUser(jwt(claims), users); got != want { + t.Errorf("%s: %v", claims, got) + } + } + if _, got := tokenUser("not-a-jwt", users); got { + t.Error("not a JWT") + } +} diff --git a/internal/provider/azure/cli.go b/internal/provider/azure/cli.go index 86ba775..e37508c 100644 --- a/internal/provider/azure/cli.go +++ b/internal/provider/azure/cli.go @@ -3,6 +3,7 @@ package azure import ( "bytes" "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -24,19 +25,51 @@ import ( // every Key Vault request (the vault's auth challenge names a tenant), so // this sends --tenant only for vaults in another tenant. // +// Which account a subscription belongs to changes: az's profile has one +// entry per subscription, and signing in to another account that sees it +// moves it there. So the subscription is looked up for every new token, +// and a token az issues for another of the signed-in users is refused. +// // Tokens are cached in memory until shortly before they expire, so the // many SDK clients don't each start an az process. type cliCredential struct { - subscription string // pins the account; "" for az's default account - tenant string // the account's tenant, if known - run func(ctx context.Context, args ...string) ([]byte, error) + user string // the account's user; "" for az's default account + tenant string // the account's tenant, if known + // accounts reads az's accounts, to pin the user; nil for the default + // account. + accounts func() ([]account, error) + run func(ctx context.Context, args ...string) ([]byte, error) mu sync.Mutex cache map[string]azcore.AccessToken // by resource and tenant argument } -func newCLICredential(subscription, tenant string) *cliCredential { - return &cliCredential{subscription: subscription, tenant: tenant, run: runAz, cache: map[string]azcore.AccessToken{}} +func newCLICredential(user, tenant string, accounts func() ([]account, error)) *cliCredential { + return &cliCredential{user: user, tenant: tenant, accounts: accounts, run: runAz, cache: map[string]azcore.AccessToken{}} +} + +// pin is a subscription az maps to the user now, and the other users az +// holds. +func (c *cliCredential) pin() (subscription string, others []string, err error) { + if c.accounts == nil { + return "", nil, nil + } + accts, err := c.accounts() + if err != nil { + return "", nil, err + } + for _, a := range accts { + switch { + case a.user == c.user && a.tenant == c.tenant: + subscription = a.anchor + case !strings.EqualFold(a.user, c.user): + others = append(others, a.user) + } + } + if subscription == "" { + return "", nil, fmt.Errorf("az has no subscriptions for %s any more: sign in as %s", c.user, c.user) + } + return subscription, others, nil } // refreshMargin renews tokens this long before they expire. @@ -50,21 +83,25 @@ func (c *cliCredential) GetToken(ctx context.Context, opts policy.TokenRequestOp return azcore.AccessToken{}, errors.New("az: this request needs multifactor authentication; run az login again") } resource := strings.TrimSuffix(opts.Scopes[0], "/.default") - args := []string{"account", "get-access-token", "--output", "json", "--resource", resource} - switch { - case opts.TenantID != "" && opts.TenantID != c.tenant: - // Another tenant: az picks the account itself. - args = append(args, "--tenant", opts.TenantID) - case c.subscription != "": - args = append(args, "--subscription", c.subscription) - } - key := strings.Join(args[5:], " ") + otherTenant := opts.TenantID != "" && opts.TenantID != c.tenant + key := resource + " " + opts.TenantID c.mu.Lock() defer c.mu.Unlock() if tok, ok := c.cache[key]; ok && time.Until(tok.ExpiresOn) > refreshMargin { return tok, nil } + args := []string{"account", "get-access-token", "--output", "json", "--resource", resource} + subscription, others, err := c.pin() + switch { + case otherTenant: + // Another tenant: az picks the account itself. + args = append(args, "--tenant", opts.TenantID) + case err != nil: + return azcore.AccessToken{}, err + case subscription != "": + args = append(args, "--subscription", subscription) + } out, err := c.run(ctx, args...) if err != nil { return azcore.AccessToken{}, err @@ -73,10 +110,43 @@ func (c *cliCredential) GetToken(ctx context.Context, opts policy.TokenRequestOp if err != nil { return azcore.AccessToken{}, err } + if u, ok := tokenUser(tok.Token, others); ok { + return azcore.AccessToken{}, fmt.Errorf("az gave a token for %s, not %s: sign in as %s again", u, c.user, c.user) + } c.cache[key] = tok return tok, nil } +// tokenUser reports which of users a token was issued to, if any, from +// its claims. The token isn't verified: this only catches az picking the +// wrong signed-in account. +func tokenUser(token string, users []string) (string, bool) { + parts := strings.Split(token, ".") + if len(parts) != 3 || len(users) == 0 { + return "", false + } + b, err := base64.RawURLEncoding.DecodeString(strings.TrimRight(parts[1], "=")) + if err != nil { + return "", false + } + var claims map[string]any + if json.Unmarshal(b, &claims) != nil { + return "", false + } + for _, name := range []string{"upn", "unique_name", "preferred_username", "email"} { + v, _ := claims[name].(string) + if i := strings.LastIndexByte(v, '#'); i >= 0 { + v = v[i+1:] // live.com#user@example.com + } + for _, u := range users { + if v != "" && strings.EqualFold(v, u) { + return u, true + } + } + } + return "", false +} + func parseCLIToken(b []byte) (azcore.AccessToken, error) { var t struct { AccessToken string `json:"accessToken"`