Files
certctl/internal/auth/keystore_test.go
T
shankar0123 38072d3922 auth-bundle-1 follow-on: close coverage gaps to clear Phase 12 floors
CI run #486 (post-Bundle-1 merge + Go 1.25.10 bump) failed three
coverage-threshold gates:

  internal/api/handler   74.7% < floor 75 (-0.3pp)
  internal/auth          66.3% < floor 85 (-18.7pp)
  internal/service/auth  51.1% < floor 85 (-33.9pp)

The Phase 12 gate file's "85% with negative-test coverage" claim
turned out to be aspirational — the read-side and Update-path
methods on RoleService / PermissionService / ActorRoleService had
zero unit-test coverage, and internal/auth's keystore +
HasPermission helper had zero tests. This commit closes the gap
without lowering the gate.

Per-package CI-style averages after this commit (per
scripts/check-coverage-thresholds.sh's per-function-mean):

  internal/api/handler   76.1% (+1.4pp,  margin +1.1pp)
  internal/auth          90.5% (+24.2pp, margin +5.5pp)
  internal/service/auth  93.7% (+42.6pp, margin +8.7pp)

Tests added:

  internal/service/auth/service_test.go (+18 tests, +518 LOC):
    PermissionService.List, PermissionService.GetByName,
    RoleService.Get (4 paths), RoleService.List (system caller),
    RoleService.Update (4 paths), RoleService.ListPermissions
    (3 paths), RoleService.AddPermission/RemovePermission round-trip
    + gate paths, RoleService.Delete (success + nil-caller +
    no-perm + audit), RoleService.Create (nil-caller),
    ActorRoleService.ListForActor (self-bypass + cross-actor +
    nil-caller + system + with-perm), ActorRoleService.Effective-
    Permissions (same shape), ActorRoleService.ListKeys (3 paths +
    system bypass), ActorRoleService.Revoke (4 paths), Authorizer
    edge cases (empty actorID short-circuit, empty tenantID
    default, scoped-grant-without-scope-id no-match invariant,
    repo-error wrap-and-return, HoldsAnyOf early-exit), recordAudit
    nil-arm short-circuits.

  internal/auth/keystore_test.go (NEW, +175 LOC):
    StaticKeyStore.Len, StaticKeyStore.LookupByHash hit + miss,
    MutableKeyStore seeded lookup + Len, Add registers new key,
    AddHashed registers from precomputed hash, AddHashed replaces
    on duplicate hash (idempotent boot-loader contract),
    HasPermission no-actor / default-actor-type / checker-error /
    scoped-check threading.

  internal/auth/bootstrap/service_test.go (+36 LOC):
    Service.Available nil-receiver/nil-strategy short-circuit,
    Service.Available delegates to Strategy when configured.

  internal/api/handler/auth_test.go (+208 LOC):
    GetRole returns role + permissions, GetRole 404 + 401, UpdateRole
    200 + invalid-JSON-400 + 401, ListKeys returns actor list + 401,
    RemoveRolePermission 204 (global + scoped) + 401,
    rolePermToResponse scope encoding pin via GetRole.

Verified:
  gofmt -l . clean (touched files only).
  go vet ./internal/auth/... ./internal/service/auth/...
       ./internal/api/handler/ rc=0.
  go test -count=1 -short on the four packages green.
  CI-style per-function averages computed via the live
       scripts/check-coverage-thresholds.sh arithmetic — all three
       gated packages clear their floors with margin.

Per CLAUDE.md "complete path" + "do not lower the gate to make CI
green": gate file unchanged. The 85/85/75 floors stand.
2026-05-10 02:04:36 +00:00

185 lines
6.5 KiB
Go

package auth
import (
"context"
"errors"
"testing"
)
// =============================================================================
// Coverage-floor closure (post-Bundle-1 follow-on, 2026-05-09).
//
// CI run #486 caught internal/auth at 66.3% (CI global) / 72.8%
// (per-package), well below the 85 floor. The Phase 12 gate file
// claimed full negative-test coverage; turned out the keystore +
// HasPermission helper had zero tests. The tests below close the gap
// without lowering the gate. Each function listed had 0% coverage at
// the time of the closure:
//
// StaticKeyStore.Len 0%
// NewMutableKeyStore 0%
// MutableKeyStore.LookupByHash 0%
// MutableKeyStore.Add 0%
// MutableKeyStore.AddHashed 0%
// MutableKeyStore.Len 0%
// HasPermission 0%
// =============================================================================
func TestStaticKeyStore_LenReportsEntryCount(t *testing.T) {
ks := NewStaticKeyStore([]NamedAPIKey{
{Name: "alice", Key: "alice-key", Admin: true},
{Name: "bob", Key: "bob-key", Admin: false},
})
if got := ks.Len(); got != 2 {
t.Errorf("Len() = %d; want 2", got)
}
}
func TestStaticKeyStore_LookupHitAndMiss(t *testing.T) {
ks := NewStaticKeyStore([]NamedAPIKey{
{Name: "alice", Key: "alice-key", Admin: true},
})
got, ok := ks.LookupByHash(HashAPIKey("alice-key"))
if !ok {
t.Fatalf("LookupByHash(alice-key) ok=false; want true")
}
if got.Name != "alice" || !got.Admin {
t.Errorf("LookupByHash returned %+v; want alice/admin=true", got)
}
if _, ok := ks.LookupByHash(HashAPIKey("not-a-key")); ok {
t.Errorf("LookupByHash(unknown) ok=true; want false")
}
}
func TestMutableKeyStore_SeededLookupAndLen(t *testing.T) {
ks := NewMutableKeyStore([]NamedAPIKey{
{Name: "alice", Key: "alice-key", Admin: true},
})
if ks.Len() != 1 {
t.Errorf("Len after construction = %d; want 1", ks.Len())
}
got, ok := ks.LookupByHash(HashAPIKey("alice-key"))
if !ok {
t.Fatalf("LookupByHash(alice-key) ok=false; want true")
}
if got.Name != "alice" || !got.Admin {
t.Errorf("LookupByHash returned %+v; want alice/admin=true", got)
}
if _, ok := ks.LookupByHash(HashAPIKey("missing")); ok {
t.Errorf("LookupByHash(missing) ok=true; want false")
}
}
func TestMutableKeyStore_AddRegistersNewKey(t *testing.T) {
ks := NewMutableKeyStore(nil)
ks.Add(NamedAPIKey{Name: "carol", Key: "carol-key", Admin: false})
if ks.Len() != 1 {
t.Errorf("Len after Add = %d; want 1", ks.Len())
}
got, ok := ks.LookupByHash(HashAPIKey("carol-key"))
if !ok || got.Name != "carol" {
t.Errorf("LookupByHash after Add = (%+v, %v); want carol/true", got, ok)
}
}
func TestMutableKeyStore_AddHashedRegistersFromPrecomputedHash(t *testing.T) {
ks := NewMutableKeyStore(nil)
hash := HashAPIKey("dan-key")
ks.AddHashed("dan", hash, true)
got, ok := ks.LookupByHash(hash)
if !ok || got.Name != "dan" || !got.Admin {
t.Errorf("LookupByHash(dan-hash) = (%+v, %v); want dan/admin=true", got, ok)
}
}
func TestMutableKeyStore_AddHashedReplacesOnDuplicateHash(t *testing.T) {
// Same hash submitted twice with different name/admin must replace
// the existing entry in-place (idempotent boot-loader contract).
ks := NewMutableKeyStore(nil)
hash := HashAPIKey("eve-key")
ks.AddHashed("eve", hash, false)
ks.AddHashed("eve", hash, true) // same name, flipped admin
if ks.Len() != 1 {
t.Errorf("Len after duplicate-hash AddHashed = %d; want 1 (idempotent replace)", ks.Len())
}
got, _ := ks.LookupByHash(hash)
if !got.Admin {
t.Errorf("LookupByHash after second AddHashed: admin=%v; want true (replace took effect)", got.Admin)
}
}
// =============================================================================
// HasPermission convenience helper — used by handlers that branch on a
// permission rather than 403'ing the whole request.
// =============================================================================
func TestHasPermission_NoActorReturnsErrNoActor(t *testing.T) {
checker := &fakeChecker{check: func(_ context.Context, _, _, _, _, _ string, _ *string) (bool, error) {
t.Fatalf("checker should not be called when no actor in context")
return false, nil
}}
_, err := HasPermission(context.Background(), checker, "cert.read", "global", nil)
if !errors.Is(err, ErrNoActor) {
t.Errorf("HasPermission(no actor) err = %v; want ErrNoActor", err)
}
}
func TestHasPermission_DefaultsActorTypeToAPIKey(t *testing.T) {
var capturedActorType string
checker := &fakeChecker{check: func(_ context.Context, _, actorType, _, _, _ string, _ *string) (bool, error) {
capturedActorType = actorType
return true, nil
}}
// Set actor ID but NOT actor type → should default to APIKey.
ctx := context.WithValue(context.Background(), ActorIDKey{}, "alice")
ok, err := HasPermission(ctx, checker, "cert.read", "global", nil)
if err != nil {
t.Fatalf("HasPermission err: %v", err)
}
if !ok {
t.Errorf("HasPermission ok=false; want true")
}
if capturedActorType != ActorTypeAPIKey {
t.Errorf("HasPermission defaulted actor type to %q; want %q", capturedActorType, ActorTypeAPIKey)
}
}
func TestHasPermission_CheckerErrorPropagates(t *testing.T) {
sentinel := errors.New("repo: down")
checker := &fakeChecker{check: func(_ context.Context, _, _, _, _, _ string, _ *string) (bool, error) {
return false, sentinel
}}
ctx := context.WithValue(context.Background(), ActorIDKey{}, "alice")
ctx = context.WithValue(ctx, ActorTypeKey{}, ActorTypeAPIKey)
_, err := HasPermission(ctx, checker, "cert.read", "global", nil)
if !errors.Is(err, sentinel) {
t.Errorf("HasPermission err = %v; want propagated sentinel", err)
}
}
func TestHasPermission_ScopedCheckThreadsThrough(t *testing.T) {
var capturedScopeType string
var capturedScopeID *string
checker := &fakeChecker{check: func(_ context.Context, _, _, _, _, scopeType string, scopeID *string) (bool, error) {
capturedScopeType = scopeType
capturedScopeID = scopeID
return true, nil
}}
ctx := context.WithValue(context.Background(), ActorIDKey{}, "alice")
ctx = context.WithValue(ctx, ActorTypeKey{}, ActorTypeAPIKey)
scopeID := "p-corp"
ok, err := HasPermission(ctx, checker, "profile.edit", "profile", &scopeID)
if err != nil {
t.Fatalf("HasPermission err: %v", err)
}
if !ok {
t.Errorf("HasPermission ok=false; want true")
}
if capturedScopeType != "profile" {
t.Errorf("scopeType captured = %q; want profile", capturedScopeType)
}
if capturedScopeID == nil || *capturedScopeID != "p-corp" {
t.Errorf("scopeID captured = %v; want p-corp", capturedScopeID)
}
}