fix: never show one az account's subscriptions under another
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
17d2705e12
commit
57e545f1ab
3 files changed
+158
-18
No files matched your search
@@ -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
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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"`
|
||||
|
||||
Reference in new issue
Block a user