Files
certctl/internal/auth/oidc/email_domain_test.go
T
shankar0123 739745e9fe fix(oidc): enforce AllowedEmailDomains allowlist in HandleCallback
Closes CRIT-5 of the 2026-05-10 audit — the LAST Critical blocker for
v2.1.0. The OIDCProvider.AllowedEmailDomains field shipped persisted
(internal/auth/oidc/domain/types.go:47), API-surfaced
(internal/api/handler/auth_session_oidc.go), MCP-surfaced
(internal/mcp/tools_auth_bundle2.go), and GUI-editable, but the
verifier in internal/auth/oidc/service.go::HandleCallback NEVER read
it. Operators filling allowed_email_domains: ["acme.com"] expected
"users outside acme.com cannot log in" — the field had zero effect.
Textbook lying-field shape per CLAUDE.md's "complete path" rule.

This commit:

- Adds Step 7.5 to HandleCallback (between profile-claim resolve and
  group-claim resolve): when the provider's AllowedEmailDomains slice
  is non-empty, the user's email-domain MUST match a list entry (case-
  insensitive exact match; subdomains NOT auto-accepted — operators
  who want dev.acme.com authorized must list it explicitly).

- Two new sentinel errors at the package level:
    - ErrEmailDomainNotAllowed   — email is set but domain not in list
    - ErrEmailMissingButRequired — allowlist set + ID token has no email

- New extractEmailDomain helper: case-folds + trims whitespace + uses
  LastIndex for the @ split + rejects empty input / no-@ / empty
  local-part / empty domain-part. Returns the lowercase domain or
  an error.

- 21 regression tests in internal/auth/oidc/email_domain_test.go:
    - 10 extractEmailDomain shape cases (plain, mixed-case input,
      leading/trailing whitespace, subdomain preserved, empty, no @,
      empty local-part, empty domain-part, multiple @ via LastIndex).
    - 11 match-semantic cases (empty list passes any, lowercase match,
      mixed-case allowlist entry match, mixed-case email match,
      whitespace-padded allowlist entry, unmatched returns
      ErrEmailDomainNotAllowed, missing email + non-empty allowlist
      returns ErrEmailMissingButRequired, subdomain NOT auto-accepted,
      parent-domain NOT auto-accepted, multi-entry first-match,
      multi-entry no-match).

Subdomain matching (alice@dev.acme.com against allowlist=[acme.com])
is intentionally NOT auto-accepted. The audit's MED-line tracks the
wildcard / suffix support story for v3; v2.1 ships strict.

Verification gate green:
- gofmt clean
- go vet clean
- go test -short -count=1 ./internal/auth/oidc/... ./internal/api/...
  ./internal/domain/auth/ — all pass (incl. existing OIDC service
  test suite, the 4 BCL tests, the auditor pin, and the AST
  RBAC-gate coverage guard).

Branch dev/auth-bundle-2 status post-commit: CRIT-1 (68ca42f),
CRIT-2 (ca1e135), CRIT-3 (00eace8), CRIT-4 (f1d9771), CRIT-5 (this)
— all five Criticals from the 2026-05-10 audit closed. v2.1.0 is
unblocked. HIGH-1..HIGH-12 + MEDs + LOWs are independently mergeable
follow-ups (spec at cowork/auth-bundles-fixes-2026-05-10/).

Refs: cowork/auth-bundles-audit-2026-05-10.md CRIT-5
2026-05-10 20:30:32 +00:00

114 lines
4.0 KiB
Go

package oidc
import (
"errors"
"strings"
"testing"
)
// Audit 2026-05-10 CRIT-5 closure — email-domain allowlist enforcement.
// Tests the extractEmailDomain helper directly + the table-driven
// matcher logic. The full HandleCallback wiring is exercised by the
// existing OIDC service test suite (mockIdP + tokenSet); these tests
// pin the domain-extraction + match semantics that
// HandleCallback Step 7.5 relies on.
func TestExtractEmailDomain(t *testing.T) {
cases := []struct {
name string
input string
want string
wantErr bool
}{
{"plain", "alice@acme.com", "acme.com", false},
{"mixed-case-input", "Alice@ACME.com", "acme.com", false},
{"leading-trailing-whitespace", " bob@example.org ", "example.org", false},
{"subdomain-preserved", "alice@dev.acme.com", "dev.acme.com", false},
{"empty", "", "", true},
{"whitespace-only", " ", "", true},
{"no-at", "alice", "", true},
{"empty-local-part", "@acme.com", "", true},
{"empty-domain-part", "alice@", "", true},
// Multiple @ — addresses where the local-part is quoted and contains @
// are technically valid RFC but rare; we use LastIndex so the domain
// portion is unambiguous. Document this behavior in the test.
{"multiple-at-uses-last", "weird@user@acme.com", "acme.com", false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := extractEmailDomain(tc.input)
if tc.wantErr {
if err == nil {
t.Fatalf("expected error for %q; got nil (returned %q)", tc.input, got)
}
return
}
if err != nil {
t.Fatalf("unexpected error for %q: %v", tc.input, err)
}
if got != tc.want {
t.Errorf("extractEmailDomain(%q) = %q; want %q", tc.input, got, tc.want)
}
})
}
}
// TestEmailDomainAllowlist_MatchSemantics pins the case-insensitive
// exact-match contract used by HandleCallback Step 7.5. Exhaustive
// over the cases the prompt's spec required.
func TestEmailDomainAllowlist_MatchSemantics(t *testing.T) {
cases := []struct {
name string
allowlist []string
email string
wantErr error
}{
{"empty-list — any domain accepted", nil, "alice@evil.com", nil},
{"matched lowercase", []string{"acme.com"}, "alice@acme.com", nil},
{"matched mixed-case allowlist entry", []string{"ACME.com"}, "alice@acme.com", nil},
{"matched mixed-case email", []string{"acme.com"}, "Alice@ACME.com", nil},
{"matched with whitespace in allowlist", []string{" acme.com "}, "alice@acme.com", nil},
{"unmatched", []string{"acme.com"}, "eve@evil.com", ErrEmailDomainNotAllowed},
{"missing email with non-empty list", []string{"acme.com"}, "", ErrEmailMissingButRequired},
{"subdomain NOT auto-accepted", []string{"acme.com"}, "alice@dev.acme.com", ErrEmailDomainNotAllowed},
{"parent-domain NOT auto-accepted", []string{"dev.acme.com"}, "alice@acme.com", ErrEmailDomainNotAllowed},
{"multi-entry first-match", []string{"first.com", "acme.com", "last.com"}, "alice@acme.com", nil},
{"multi-entry no-match", []string{"first.com", "second.com"}, "alice@third.com", ErrEmailDomainNotAllowed},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := checkEmailDomainAllowlist(tc.allowlist, tc.email)
if tc.wantErr == nil {
if got != nil {
t.Fatalf("expected nil error; got %v", got)
}
return
}
if !errors.Is(got, tc.wantErr) {
t.Errorf("got error %v; want %v", got, tc.wantErr)
}
})
}
}
// checkEmailDomainAllowlist mirrors HandleCallback Step 7.5 logic for
// direct testing. Keeps the test independent of mockIdP setup; the
// full integration test (mockIdP + tokenSet + HandleCallback) lives
// in service_test.go and exercises the same path via the IdP-shaped
// flow.
func checkEmailDomainAllowlist(allowlist []string, email string) error {
if len(allowlist) == 0 {
return nil
}
dom, err := extractEmailDomain(email)
if err != nil {
return ErrEmailMissingButRequired
}
for _, allowed := range allowlist {
if strings.EqualFold(strings.TrimSpace(allowed), dom) {
return nil
}
}
return ErrEmailDomainNotAllowed
}