From 87d98d038dbe862093b2e61ca877a409f2166681 Mon Sep 17 00:00:00 2001 From: keb Date: Sun, 11 Oct 2026 09:55:43 +0200 Subject: [PATCH] fix: never show one az account's subscriptions under another (#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With two az accounts, signing in to one could make the other account's group list the first one's subscriptions. **Why:** az can't be told which user to get a token for, so each account is pinned with `--subscription`, using one of its subscriptions. The pin was chosen once at start. But az's profile has one entry per subscription, tagged with the account that signed in to it last. Signing in to another account that also sees the pinned subscription moves it there, and from then on the first account's token requests silently returned the other account's token. That's what happened here: the in-app sign-in as one account re-tagged the other account's default subscription. **Fix:** - The pin is looked up in az's profile for every new token (only when the in-memory token cache misses), so it follows az's current mapping. An account with no subscriptions left in az gets "sign in as …". - A token az issues for **another of the signed-in users** (by its `upn`/`unique_name`/`preferred_username`/`email` claim) is refused with "az gave a token for X, not Y: sign in as Y again", which shows as the usual sign-in prompt. Only a match against another known user is refused, so tokens without those claims (service principals) or with unusual guest names aren't turned away. Tests cover the subscription moving between users, a refused token, a user gone from az, and the claim matching. I also checked against two real az accounts (one signed in, one expired): the signed-in account lists its subscriptions, and the expired one shows the sign-in error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Karl-Einar Bergstrøm Reviewed-on: https://git.kebcloud.no/keb/lazysecrets/pulls/3 --- 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"`