browser auth fix

.gitignore

@@ -2,6 +2,8 @@
 /iontk
 .env
 *.txt
+# go:embed cannot find a file git never took, and *.txt is blanket-ignored.
+!internal/clock/zones.txt
 *.json
 *.md
 # The release job builds into dist/, and a local reproduction of it leaves

cmd/iontk/flagdoc.go

@@ -68,7 +68,7 @@ var flagGroups = []struct {
 	{"Rich data", "The TUI's third element mode reads the portal instead of opening a session. Its panes are defined in YAML: `--views` points at a file instead of `views.yaml` in the config directory, which is also how a `--fixtures` run gets views of its own.", []string{
 		"views",
 	}},
-	{"Appearance", "Colour schemes are YAML too: `--themes` points at a file whose schemes are added to the built-in ones, or replace one of the same name, instead of `themes.yaml` in the config directory. The same escape hatch as `--views`, for the same reason.", []string{
+	{"Appearance", "Color schemes are YAML too: `--themes` points at a file whose schemes are added to the built-in ones, or replace one of the same name, instead of `themes.yaml` in the config directory. The same escape hatch as `--views`, for the same reason.", []string{
 		"themes",
 	}},
 }

cmd/iontk/root.go

@@ -123,7 +123,7 @@ func init() {
 	pf.BoolVar(&app.Debug, "debug", false, "run against a throwaway config with no saved tenants and secrets held only in memory")
 	pf.StringVar(&flagFixtures, "fixtures", "", "YAML file of fake tenants, elements and CLI responses to run against instead of real hardware")
 	pf.StringVar(&flagViews, "views", "", "YAML file of rich-data views to use instead of views.yaml in the config directory")
-	pf.StringVar(&flagThemes, "themes", "", "YAML file of colour schemes to add to the built-in ones, instead of themes.yaml in the config directory")
+	pf.StringVar(&flagThemes, "themes", "", "YAML file of color schemes to add to the built-in ones, instead of themes.yaml in the config directory")
 	pf.StringVar(&app.Defaults.TSGID, "tsg", "", "tenant service group ID (overrides SCM_TSG_ID)")
 	pf.StringVar(&flagFormat, "format", "", `batch output format: "json", "jsonl", "text", "table" or "csv" (default from settings, else json)`)
 	pf.StringVar(&flagOrient, "orientation", "", `row order for "table" and "csv": "ion" groups each element's commands, "command" puts one command across the fleet together (default from settings, else ion)`)

internal/app/auth.go

@@ -115,13 +115,18 @@ func ShortAuthError(err error) string {
 	switch {
 	case strings.Contains(text, "did not complete within"):
 		return "sign-in timed out"
+	case errors.Is(err, saseauth.ErrNoCredential):
+		return "no sign-in for this tenant"
 	case strings.Contains(text, "invalid_grant"),
 		strings.Contains(text, "stored refresh token rejected"):
 		return "sign-in expired"
-	case errors.Is(err, saseauth.ErrInteractiveRequired),
-		strings.Contains(text, "interaction_required"),
+	case errors.Is(err, saseauth.ErrNoSession):
+		return "sign-in needed"
+	case strings.Contains(text, "interaction_required"),
 		strings.Contains(text, "login_required"):
 		return "not accessible with this sign-in"
+	case errors.Is(err, saseauth.ErrInteractiveRequired):
+		return "sign-in needed"
 	case IsAuthFailure(text):
 		return "credential rejected"
 	}

internal/app/auth_test.go

@@ -1,8 +1,11 @@
 package app
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
@@ -189,6 +192,24 @@ func TestAccountSeedIsSharedBetweenTenants(t *testing.T) {
 	}
 }
 
+func TestWarmingATenantSpendsTheCredentialsAlreadyHeld(t *testing.T) {
+	st := testStore(t)
+	if err := st.Secrets.Set(config.AccountRefreshTokenKey(), "account-rt"); err != nil {
+		t.Fatalf("seed account token: %v", err)
+	}
+	if err := st.Secrets.Set(config.RefreshTokenKey("1"), "tenant-rt"); err != nil {
+		t.Fatalf("seed tenant token: %v", err)
+	}
+
+	src := tenantSource(st, nil, "1")
+	if got := src.RefreshToken(); got != "tenant-rt" {
+		t.Errorf("RefreshToken() = %q, want the stored token tried before any browser", got)
+	}
+	if src.Seed() == nil {
+		t.Error("Seed() = nil, want the account token offered too, or login opens a second window")
+	}
+}
+
 func TestATenantsOwnTokenComesFirstButTheSeedIsStillThere(t *testing.T) {
 	st := testStore(t)
 	if err := st.Secrets.Set(config.AccountRefreshTokenKey(), "account-rt"); err != nil {
@@ -275,6 +296,72 @@ func TestFixturesSeedTenantsThatResolveWithoutASignIn(t *testing.T) {
 	}
 }
 
+func TestATenantWithNoSignInReadsTheSameWhateverTheAccountCredentialDid(t *testing.T) {
+	const (
+		good = "1984021017"
+		bad  = "1538107912"
+	)
+
+	tests := []struct {
+		name string
+		seed func(t *testing.T, tokenURL string, client *http.Client) *saseauth.Seed
+	}{
+		{
+			name: "no account credential at all",
+			seed: func(*testing.T, string, *http.Client) *saseauth.Seed { return nil },
+		},
+		{
+			name: "another tenant narrowed it first",
+			seed: func(t *testing.T, tokenURL string, client *http.Client) *saseauth.Seed {
+				s := saseauth.NewSeed("acct")
+				opt := saseauth.Options{TokenURL: tokenURL, Client: client, Seed: s}
+				if _, _, err := saseauth.NewSource(good, "", opt, nil).Token(context.Background()); err != nil {
+					t.Fatalf("the first tenant could not narrow the account credential: %v", err)
+				}
+				return s
+			},
+		},
+		{
+			name: "it was refused outright",
+			seed: func(*testing.T, string, *http.Client) *saseauth.Seed {
+				return saseauth.NewSeed("dead")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				r.ParseForm()
+				if r.PostFormValue("refresh_token") == "acct" {
+					w.Write([]byte(`{"access_token":"at","refresh_token":"narrowed","expires_in":899}`))
+					return
+				}
+				w.WriteHeader(http.StatusBadRequest)
+				w.Write([]byte(`{"error":"invalid_grant"}`))
+			}))
+			defer srv.Close()
+
+			opt := saseauth.Options{
+				TokenURL: srv.URL,
+				Client:   srv.Client(),
+				Seed:     tt.seed(t, srv.URL, srv.Client()),
+			}
+			_, _, err := saseauth.NewSource(bad, "", opt, nil).Token(context.Background())
+			if err == nil {
+				t.Fatal("Token() succeeded for a tenant with no sign-in of its own")
+			}
+			if got := ShortAuthError(err); got != "no sign-in for this tenant" {
+				t.Errorf("ShortAuthError() = %q, want %q — the message must not report what the "+
+					"shared account credential did as a verdict on this tenant", got, "no sign-in for this tenant")
+			}
+			if !errors.Is(err, saseauth.ErrInteractiveRequired) {
+				t.Errorf("Token() error = %v, want it to still wrap ErrInteractiveRequired so the TUI offers a sign-in", err)
+			}
+		})
+	}
+}
+
 func TestShortAuthErrorSaysWhatToDoAboutIt(t *testing.T) {
 	tests := []struct {
 		name string
@@ -290,9 +377,27 @@ func TestShortAuthErrorSaysWhatToDoAboutIt(t *testing.T) {
 			want: "sign-in expired",
 		},
 		{
-			name: "no access to the tenant",
-			err: fmt.Errorf("%w: no existing browser session: authorization failed: interaction_required",
-				saseauth.ErrInteractiveRequired),
+			name: "never signed in to this tenant",
+			err: fmt.Errorf("%w: %w; %w: authorization failed: interaction_required",
+				saseauth.ErrInteractiveRequired, saseauth.ErrNoCredential, saseauth.ErrNoSession),
+			want: "no sign-in for this tenant",
+		},
+		{
+			name: "another tenant had already spent the account credential",
+			err: fmt.Errorf("%w: %w; account credential rejected: token: status 400 Bad Request: "+
+				`{"error":"invalid_grant"}`+"; %w: authorization failed: interaction_required",
+				saseauth.ErrInteractiveRequired, saseauth.ErrNoCredential, saseauth.ErrNoSession),
+			want: "no sign-in for this tenant",
+		},
+		{
+			name: "nothing silent is left to try",
+			err: fmt.Errorf("%w: %w: authorization failed: interaction_required",
+				saseauth.ErrInteractiveRequired, saseauth.ErrNoSession),
+			want: "sign-in needed",
+		},
+		{
+			name: "the authorization server refused an interactive sign-in",
+			err:  errors.New("authorization failed: login_required: no access to this tsg"),
 			want: "not accessible with this sign-in",
 		},
 		{

internal/app/signin.go

@@ -35,27 +35,50 @@ func SignIn(ctx context.Context, st *config.Store, w io.Writer) (saseauth.Discov
 	return d, nil
 }
 
-func WarmTenants(ctx context.Context, st *config.Store, tenants []config.Tenant, w io.Writer) error {
-	httpClient := &http.Client{Timeout: Defaults.HTTPTimeout}
-	var failed int
+type TenantWarm struct {
+	Tenant  config.Tenant
+	Skipped bool
+	Err     error
+}
 
+// rotated as it is redeemed, so it narrows to one tenant
+func WarmEach(ctx context.Context, st *config.Store, tenants []config.Tenant) []TenantWarm {
+	httpClient := &http.Client{Timeout: Defaults.HTTPTimeout}
+	out := make([]TenantWarm, 0, len(tenants))
 	for _, t := range tenants {
 		if !t.UsesBrowser() {
-			fmt.Fprintf(w, "  %-28s service account, nothing to sign in\n", t.Name)
+			out = append(out, TenantWarm{Tenant: t, Skipped: true})
+			continue
+		}
+		_, _, err := tenantSource(st, httpClient, t.TSGID).Token(ctx)
+		out = append(out, TenantWarm{Tenant: t, Err: err})
+		if ctx.Err() != nil {
+			return out
+		}
+	}
+	return out
+}
+
+func WarmTenants(ctx context.Context, st *config.Store, tenants []config.Tenant, w io.Writer) error {
+	var failed, browser int
+
+	for _, r := range WarmEach(ctx, st, tenants) {
+		if r.Skipped {
+			fmt.Fprintf(w, "  %-28s service account, nothing to sign in\n", r.Tenant.Name)
 			continue
 		}
-		src := saseauth.NewSource(t.TSGID, "", BrowserOptions(httpClient, st),
-			func(rt string) error { return st.Secrets.Set(config.RefreshTokenKey(t.TSGID), rt) })
-		if _, _, err := src.Token(ctx); err != nil {
+		browser++
+		if r.Err != nil {
 			failed++
-			fmt.Fprintf(w, "  %-28s %s\n", t.Name, ShortAuthError(err))
+			fmt.Fprintf(w, "  %-28s %s\n", r.Tenant.Name, ShortAuthError(r.Err))
 			continue
 		}
-		fmt.Fprintf(w, "  %-28s ready\n", t.Name)
+		fmt.Fprintf(w, "  %-28s ready\n", r.Tenant.Name)
 	}
 
-	if failed > 0 && failed == len(tenants) {
-		return fmt.Errorf("no tenant could be signed in")
+	if failed > 0 && failed == browser {
+		return fmt.Errorf("no tenant could be signed in: every one failed the same way, " +
+			"so it is the browser sign-in that did not take and not access to any one tenant")
 	}
 	if failed > 0 {
 		fmt.Fprintf(w, "%d of %d tenants could not be signed in; this account may not have access to them\n",
@@ -64,6 +87,10 @@ func WarmTenants(ctx context.Context, st *config.Store, tenants []config.Tenant,
 	return nil
 }
 
+func tenantSource(st *config.Store, httpClient *http.Client, tsg string) *saseauth.Source {
+	return BrowserSource(Flags{TSGID: tsg, BrowserAuth: true}, httpClient, st)
+}
+
 func SessionLeft(st *config.Store, tsg string) (time.Duration, bool) {
 	if st == nil {
 		return 0, false

internal/clock/clock.go

@@ -53,6 +53,14 @@ func Valid(spec string) error {
 	return err
 }
 
+func StampIn(spec string, t time.Time) (string, error) {
+	l, _, err := load(spec)
+	if err != nil {
+		return "", err
+	}
+	return t.In(l).Format(layout), nil
+}
+
 func Zone() *time.Location {
 	mu.RLock()
 	defer mu.RUnlock()

internal/clock/zones.go

@@ -0,0 +1,29 @@
+package clock
+
+import (
+	_ "embed"
+	"strings"
+	"sync"
+
+	// windows ships no zoneinfo, so LoadLocation needs the embedded copy
+	_ "time/tzdata"
+)
+
+// generated from the Go distribution's lib/time/zoneinfo.zip; the stdlib lists no zones
+//
+//go:embed zones.txt
+var zoneList string
+
+var zones = sync.OnceValue(func() []string {
+	lines := strings.Split(zoneList, "\n")
+	out := make([]string, 0, len(lines)+2)
+	out = append(out, UTC, Local)
+	for _, l := range lines {
+		if l = strings.TrimSpace(l); l != "" {
+			out = append(out, l)
+		}
+	}
+	return out
+})
+
+func Zones() []string { return zones() }