mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 13:51:36 +00:00
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.
This commit is contained in:
shankar0123
@@ -434,3 +434,211 @@ func TestAuthHandler_MeReturnsActorIdentity(t *testing.T) {
|
||||
t.Errorf("effective_permissions wrong; got %+v", resp.EffectivePermissions)
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Coverage-floor closure (post-Bundle-1 follow-on, 2026-05-09).
|
||||
//
|
||||
// CI run #486 caught internal/api/handler at 74.7% — 0.3pp below the
|
||||
// 75 floor. The auth handlers added in Bundle 1 had several 0%-covered
|
||||
// methods: GetRole, UpdateRole, ListKeys, RemoveRolePermission. The
|
||||
// tests below close the gap.
|
||||
// =============================================================================
|
||||
|
||||
func TestAuthHandler_GetRoleReturnsRoleAndPermissions(t *testing.T) {
|
||||
h, roleSvc, _, _ := newAuthHandlerWithFakes()
|
||||
roleSvc.roles["r-admin"] = &authdomain.Role{ID: "r-admin", Name: "admin", Description: "the admin role"}
|
||||
scope := "p-corp"
|
||||
roleSvc.rolePerms["r-admin"] = []*authdomain.RolePermission{
|
||||
{RoleID: "r-admin", PermissionID: "p-cert.read", ScopeType: authdomain.ScopeTypeGlobal},
|
||||
{RoleID: "r-admin", PermissionID: "p-profile.edit", ScopeType: authdomain.ScopeTypeProfile, ScopeID: &scope},
|
||||
}
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodGet, "/api/v1/auth/roles/r-admin", nil), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-admin")
|
||||
rec := httptest.NewRecorder()
|
||||
h.GetRole(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("GetRole code = %d; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Role roleResponse `json:"role"`
|
||||
Permissions []rolePermissionResponse `json:"permissions"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if resp.Role.ID != "r-admin" || resp.Role.Name != "admin" {
|
||||
t.Errorf("Role envelope wrong: %+v", resp.Role)
|
||||
}
|
||||
if len(resp.Permissions) != 2 {
|
||||
t.Errorf("permissions length = %d; want 2", len(resp.Permissions))
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_GetRoleNotFoundReturns404(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodGet, "/api/v1/auth/roles/r-missing", nil), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-missing")
|
||||
rec := httptest.NewRecorder()
|
||||
h.GetRole(rec, req)
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("GetRole(missing) code = %d; want 404", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_GetRoleNoActorReturns401(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/roles/r-admin", nil)
|
||||
req.SetPathValue("id", "r-admin")
|
||||
rec := httptest.NewRecorder()
|
||||
h.GetRole(rec, req)
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Errorf("GetRole no-actor code = %d; want 401", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_UpdateRoleReturns200(t *testing.T) {
|
||||
h, roleSvc, _, _ := newAuthHandlerWithFakes()
|
||||
roleSvc.roles["r-x"] = &authdomain.Role{ID: "r-x", Name: "old", Description: ""}
|
||||
body := bytes.NewBufferString(`{"name":"new","description":"updated"}`)
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodPut, "/api/v1/auth/roles/r-x", body), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-x")
|
||||
rec := httptest.NewRecorder()
|
||||
h.UpdateRole(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("UpdateRole code = %d; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var resp roleResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if resp.Name != "new" || resp.Description != "updated" {
|
||||
t.Errorf("UpdateRole returned %+v; want Name=new, Description=updated", resp)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_UpdateRoleInvalidJSONReturns400(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
body := strings.NewReader(`{"name":`) // truncated
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodPut, "/api/v1/auth/roles/r-x", body), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-x")
|
||||
rec := httptest.NewRecorder()
|
||||
h.UpdateRole(rec, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("UpdateRole invalid JSON code = %d; want 400", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_UpdateRoleNoActorReturns401(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := httptest.NewRequest(http.MethodPut, "/api/v1/auth/roles/r-x", bytes.NewBufferString(`{"name":"new"}`))
|
||||
req.SetPathValue("id", "r-x")
|
||||
rec := httptest.NewRecorder()
|
||||
h.UpdateRole(rec, req)
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Errorf("UpdateRole no-actor code = %d; want 401", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_ListKeysReturnsActorList(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
actorSvc.roles = []*authdomain.ActorRole{
|
||||
{ID: "ar-1", ActorID: "alice", ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), TenantID: authdomain.DefaultTenantID, RoleID: "r-admin"},
|
||||
{ID: "ar-2", ActorID: "carol", ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), TenantID: authdomain.DefaultTenantID, RoleID: "r-viewer"},
|
||||
}
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodGet, "/api/v1/auth/keys", nil), "alice", auth.ActorTypeAPIKey)
|
||||
rec := httptest.NewRecorder()
|
||||
h.ListKeys(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("ListKeys code = %d; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Keys []struct {
|
||||
ActorID string `json:"actor_id"`
|
||||
ActorType string `json:"actor_type"`
|
||||
TenantID string `json:"tenant_id"`
|
||||
RoleIDs []string `json:"role_ids"`
|
||||
} `json:"keys"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(resp.Keys) != 2 {
|
||||
t.Errorf("ListKeys returned %d keys; want 2", len(resp.Keys))
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_ListKeysNoActorReturns401(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/keys", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
h.ListKeys(rec, req)
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Errorf("ListKeys no-actor code = %d; want 401", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RemoveRolePermissionReturns204(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete, "/api/v1/auth/roles/r-admin/permissions/cert.read", nil), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-admin")
|
||||
req.SetPathValue("perm", "cert.read")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RemoveRolePermission(rec, req)
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Errorf("RemoveRolePermission code = %d; want 204", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RemoveRolePermissionScopedReturns204(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete, "/api/v1/auth/roles/r-admin/permissions/profile.edit?scope_type=profile&scope_id=p-corp", nil), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-admin")
|
||||
req.SetPathValue("perm", "profile.edit")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RemoveRolePermission(rec, req)
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Errorf("RemoveRolePermission(scoped) code = %d; want 204", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RemoveRolePermissionNoActorReturns401(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := httptest.NewRequest(http.MethodDelete, "/api/v1/auth/roles/r-admin/permissions/cert.read", nil)
|
||||
req.SetPathValue("id", "r-admin")
|
||||
req.SetPathValue("perm", "cert.read")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RemoveRolePermission(rec, req)
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Errorf("RemoveRolePermission no-actor code = %d; want 401", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// Pin the rolePermToResponse helper indirectly via GetRole; the test
|
||||
// above already exercises both global + scoped permission encoding.
|
||||
// Add an explicit assertion here so the helper's nil-scope branch is
|
||||
// readable in coverage output.
|
||||
func TestAuthHandler_GetRoleRolePermResponseEncodesScope(t *testing.T) {
|
||||
h, roleSvc, _, _ := newAuthHandlerWithFakes()
|
||||
roleSvc.roles["r-x"] = &authdomain.Role{ID: "r-x", Name: "x"}
|
||||
scope := "iss-corp"
|
||||
roleSvc.rolePerms["r-x"] = []*authdomain.RolePermission{
|
||||
{RoleID: "r-x", PermissionID: "p-cert.read", ScopeType: authdomain.ScopeTypeGlobal, ScopeID: nil},
|
||||
{RoleID: "r-x", PermissionID: "p-issuer.edit", ScopeType: authdomain.ScopeTypeIssuer, ScopeID: &scope},
|
||||
}
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodGet, "/api/v1/auth/roles/r-x", nil), "alice", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "r-x")
|
||||
rec := httptest.NewRecorder()
|
||||
h.GetRole(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("GetRole code = %d", rec.Code)
|
||||
}
|
||||
if !bytes.Contains(rec.Body.Bytes(), []byte(`"scope_type":"issuer"`)) {
|
||||
t.Errorf("body should include scope_type=issuer; got %s", rec.Body.String())
|
||||
}
|
||||
if !bytes.Contains(rec.Body.Bytes(), []byte(`"scope_id":"iss-corp"`)) {
|
||||
t.Errorf("body should include scope_id=iss-corp; got %s", rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// ensure 'errors' import stays used after edits.
|
||||
var _ = errors.Is
|
||||
|
||||
Reference in New Issue
Block a user