repair the tests I rebased onto deletions

I verified green, rebased onto three commits from today, and pushed without
re-verifying. Two packages did not compile on main for the minutes in between.
The green I trusted was measured against a tree that no longer existed.

What moved underneath: 415ad8ef deleted IsWidgetKey/IsAnalyticsKey and their
re-exports, and dfae0894 deleted normalizePhoneCandidates and isAlreadyExistsMsg
when IAM started being addressed by its nouns. All four deletions are right —
the key predicates had no caller, which is the same thing I had observed and
written into the commit message an hour earlier.

The tests now cover what exists. One assertion is worth more than it was: no
door resolves hz-, hi- or ha-, so a caller bearing one reaches what an anonymous
caller reaches, and with the naming predicates gone that rests entirely on
IsAPIKey answering false. TestUnresolvedPrefixesAreNotAPIKeys is the guard on
it — if one starts returning true, a public string becomes a credential quietly.

Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
2026-08-24 10:39:10 -07:00
co-authored by Hanzo Dev
parent ae87864592
commit 35a69093e4
2 changed files with 32 additions and 132 deletions
+32 -65
View File
@@ -14,26 +14,24 @@ import (
// under /v1/bases by KIND rather than by method — so these predicates ARE the
// authorization, and this is the import path the estate is told to use.
//
// The table states every prefix once, so a new kind added to one predicate and
// forgotten in another shows up as a row that disagrees with itself.
// Three prefixes are resolved and the table states each once, so a kind added to
// one predicate and forgotten in another shows up as a row disagreeing with
// itself.
func TestKeyKindsAreClassifiedByPrefix(t *testing.T) {
for _, tc := range []struct {
token string
publishable, secret, apiKey, analytics, widget bool
token string
publishable, secret, apiKey bool
}{
{"pk-live-abc", true, false, true, false, false},
{"sk-live-abc", false, true, true, false, false},
{"hk-abc", false, false, true, false, false},
{"hi-abc", false, false, false, true, false},
{"ha-abc", false, false, false, true, false},
{"hz-abc", false, false, false, false, true},
{"pk-live-abc", true, false, true},
{"sk-live-abc", false, true, true},
{"hk-abc", false, false, true},
// Not keys at all: a JWT, an empty string, and near-misses.
{"eyJhbGciOiJIUzI1NiJ9.e30.sig", false, false, false, false, false},
{"", false, false, false, false, false},
{"pk_live_abc", false, false, false, false, false}, // underscore, not dash
{"PK-LIVE-ABC", false, false, false, false, false}, // prefixes are lower-case
{"xpk-live-abc", false, false, false, false, false}, // prefix must lead
{"pk-", true, false, true, false, false}, // bare prefix still classifies
{"eyJhbGciOiJIUzI1NiJ9.e30.sig", false, false, false},
{"", false, false, false},
{"pk_live_abc", false, false, false}, // underscore, not dash
{"PK-LIVE-ABC", false, false, false}, // prefixes are lower-case
{"xpk-live-abc", false, false, false}, // the prefix must lead
{"pk-", true, false, true}, // a bare prefix still classifies
} {
t.Run(tc.token, func(t *testing.T) {
if got := iam.IsPublishable(tc.token); got != tc.publishable {
@@ -45,26 +43,23 @@ func TestKeyKindsAreClassifiedByPrefix(t *testing.T) {
if got := iam.IsAPIKey(tc.token); got != tc.apiKey {
t.Errorf("IsAPIKey(%q) = %v, want %v", tc.token, got, tc.apiKey)
}
if got := iam.IsAnalytics(tc.token); got != tc.analytics {
t.Errorf("IsAnalytics(%q) = %v, want %v", tc.token, got, tc.analytics)
}
if got := iam.IsWidget(tc.token); got != tc.widget {
t.Errorf("IsWidget(%q) = %v, want %v", tc.token, got, tc.widget)
}
})
}
}
// IsAPIKey is what the key middleware asks before it will resolve a credential
// at all (plugins/org/plugin.go). It admits hk-, pk- and sk- and NOTHING else —
// so an analytics or widget key is not an API key here, and a request bearing
// one is refused rather than resolved. Stated as its own test because the
// asymmetry reads like an omission and is not: those kinds are classified for a
// caller to route on, never admitted by this door.
func TestAnalyticsAndWidgetKeysAreNotAPIKeys(t *testing.T) {
for _, token := range []string{"hi-abc", "ha-abc", "hz-abc"} {
// hz-, hi- and ha- are prefixes IAM mints and no door here resolves, so a caller
// bearing one reaches exactly what an anonymous caller reaches. That is the whole
// reason IsWidgetKey and IsAnalyticsKey could be deleted (30c91ac7) — and it now
// rests entirely on IsAPIKey answering false, with no predicate left naming those
// kinds. If one ever starts returning true here, a key that was public becomes a
// credential, silently. This is the guard on that.
func TestUnresolvedPrefixesAreNotAPIKeys(t *testing.T) {
for _, token := range []string{"hz-abc", "hi-abc", "ha-abc"} {
if iam.IsAPIKey(token) {
t.Fatalf("IsAPIKey(%q) = true — the key middleware would admit it", token)
t.Fatalf("IsAPIKey(%q) = true — the key middleware would resolve a prefix no door serves", token)
}
if iam.IsPublishable(token) || iam.IsSecret(token) {
t.Fatalf("%q classified as a publishable or secret key", token)
}
}
for _, token := range []string{"hk-abc", "pk-abc", "sk-abc"} {
@@ -74,41 +69,13 @@ func TestAnalyticsAndWidgetKeysAreNotAPIKeys(t *testing.T) {
}
}
// No token is two kinds at once. It matters for pk- specifically: the middleware
// refuses a publishable key by kind, and a token that were also something else
// would offer a second reading to gate on.
// No token is both publishable and secret. It matters for pk- specifically: the
// middleware refuses a publishable key by kind, and a token that were also
// something else would offer a second reading to gate on.
func TestNoTokenIsTwoKinds(t *testing.T) {
for _, token := range []string{"pk-a", "sk-a", "hk-a", "hi-a", "ha-a", "hz-a"} {
n := 0
for _, is := range []bool{iam.IsPublishable(token), iam.IsSecret(token), iam.IsAnalytics(token), iam.IsWidget(token)} {
if is {
n++
}
}
if n > 1 {
t.Fatalf("%q matched %d kinds, want at most 1", token, n)
}
}
}
// hk- is the one kind with no predicate of its own: it is an API key and nothing
// narrower says so. That is a real hole in the surface rather than a rounding
// error — a caller routing on kind has no way to name a hosted key, and asking
// "is it an API key but none of the others" is a definition by subtraction that
// silently acquires every kind added later.
func TestHostedKeyIsAnAPIKeyWithNoKindOfItsOwn(t *testing.T) {
const hosted = "hk-abc"
if !iam.IsAPIKey(hosted) {
t.Fatalf("IsAPIKey(%q) = false", hosted)
}
for name, is := range map[string]bool{
"IsPublishable": iam.IsPublishable(hosted),
"IsSecret": iam.IsSecret(hosted),
"IsAnalytics": iam.IsAnalytics(hosted),
"IsWidget": iam.IsWidget(hosted),
} {
if is {
t.Fatalf("%s(%q) = true — hk- has acquired a kind; give it a named predicate", name, hosted)
for _, token := range []string{"pk-a", "sk-a", "hk-a", "hz-a", "hi-a", "ha-a"} {
if iam.IsPublishable(token) && iam.IsSecret(token) {
t.Fatalf("%q is both publishable and secret", token)
}
}
}
-67
View File
@@ -44,70 +44,3 @@ func TestTruncateClipsRunesAndStaysValidUTF8(t *testing.T) {
})
}
}
// The candidates are the shapes IAM might have STORED for a phone the caller
// states one way. It expands downward only — strips "+", then a US "+1" — so a
// caller holding E.164 finds a row saved either way.
//
// It does not expand upward, and that asymmetry is the behaviour, not an
// oversight to read past: a caller passing a raw national number probes exactly
// that one shape and will not find the same person stored as "+1…". Adding the
// country code back is a guess about which country, which is why the function
// does not make it. Today's only production caller looks up by email
// (plugins/bootnode/api_team.go), so nothing depends on the upward direction.
func TestNormalizePhoneCandidates(t *testing.T) {
for _, tc := range []struct {
name string
in string
want []string
}{
{"empty yields nothing", "", nil},
{"E.164 US expands to three", "+16125551234", []string{"+16125551234", "16125551234", "6125551234"}},
{"E.164 non-US drops only the plus", "+442071838750", []string{"+442071838750", "442071838750"}},
{"raw digits stay one shape", "6125551234", []string{"6125551234"}},
{"a bare plus yields itself", "+", []string{"+"}},
{"plus one yields itself and the one", "+1", []string{"+1", "1"}},
} {
t.Run(tc.name, func(t *testing.T) {
got := normalizePhoneCandidates(tc.in)
if len(got) != len(tc.want) {
t.Fatalf("normalizePhoneCandidates(%q) = %v, want %v", tc.in, got, tc.want)
}
for i := range got {
if got[i] != tc.want[i] {
t.Fatalf("normalizePhoneCandidates(%q)[%d] = %q, want %q", tc.in, i, got[i], tc.want[i])
}
}
seen := map[string]bool{}
for _, v := range got {
if seen[v] {
t.Fatalf("normalizePhoneCandidates(%q) repeated %q: %v", tc.in, v, got)
}
seen[v] = true
}
})
}
}
// A collision on add-user is not an error the caller should surface — EnsureUser
// treats it as "already provisioned" and proceeds. IAM spells the message more
// than one way, so the match is on the phrase and is case-insensitive.
func TestIsAlreadyExistsMsg(t *testing.T) {
for _, tc := range []struct {
in string
want bool
}{
{"", false},
{"user already exists", true},
{"Username already exists.", true},
{"EMAIL ALREADY EXISTS", true},
{"phone: already exists in organization hanzo", true},
{"user not found", false},
{"already", false},
{"exists", false},
} {
if got := isAlreadyExistsMsg(tc.in); got != tc.want {
t.Fatalf("isAlreadyExistsMsg(%q) = %v, want %v", tc.in, got, tc.want)
}
}
}