Skip to content

Commit

Permalink
Write new tests for Update API endpoint (#259)
Browse files Browse the repository at this point in the history 
Switch to table-driven tests, make it more clear what each test is
testing, and add additional test coverage. All tests pass to document
the current behavior of the Update endpoint. Some of the behavior of
Update is incorrect; tests for those are commented with TODOs indicating
what their correct behavior should be.
  • Loading branch information
@nharper
nharper authored Sep 10, 2024
1 parent 91f739d commit 516aec6
Show file tree
Hide file tree
Showing 2 changed files with 276 additions and 68 deletions.
338 changes: 271 additions & 67 deletions api/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,82 +7,286 @@ import (
"testing"
"time"

"github.com/chromium/hstspreload"
"github.com/chromium/hstspreload.org/database"
"github.com/chromium/hstspreload/chromium/preloadlist"
)

const ()

// TestPolicyType tests that PolicyType is populated within the database when the Update endpoint is called.
func TestPolicyType(t *testing.T) {
api, _, mockHstspreload, mockPreloadlist := mockAPI(0 * time.Second)

TestPreloadableResponses := map[string]hstspreload.Issues{
"garron.net": emptyIssues,
"badssl.com": issuesWithWarnings,
"example.com": issuesWithErrors,
"removal-pending-eligible.test": emptyIssues,
"removal-pending-ineligible.test": emptyIssues,
}
TestRemovableResponses := map[string]hstspreload.Issues{
"removal-preloaded-bulk-eligible.test": emptyIssues,
"removal-preloaded-not-bulk-eligible.test": emptyIssues,
"removal-preloaded-bulk-ineligible.test": issuesWithErrors,
"removal-pending-eligible.test": emptyIssues,
"removal-pending-ineligible.test": issuesWithErrors,
}

TestPreloadlist := preloadlist.PreloadList{Entries: []preloadlist.Entry{
{Name: "garron.net", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: true, Policy: preloadlist.Bulk18Weeks},
{Name: "chromium.org", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: false, Policy: preloadlist.Custom},
{Name: "removal-preloaded-bulk-eligible.test", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: true, Policy: preloadlist.UnspecifiedPolicyType},
{Name: "removal-preloaded-not-bulk-eligible.test", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: true, Policy: preloadlist.Bulk1Year},
{Name: "removal-preloaded-bulk-ineligible.test", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: true, Policy: preloadlist.Test},
{Name: "godoc.og", Mode: "", IncludeSubDomains: true, Policy: preloadlist.Custom},
{Name: "dev", Mode: preloadlist.ForceHTTPS, IncludeSubDomains: true, Policy: preloadlist.UnspecifiedPolicyType},
}}

mockHstspreload.preloadableResponses = TestPreloadableResponses
mockHstspreload.removableResponses = TestRemovableResponses
mockPreloadlist.list = TestPreloadlist

// tests for correct behavior when a domain changes from pending status to preloaded status
testPendingToPreloaded := database.DomainState{Name: "garron.net", Status: database.StatusPending, IncludeSubDomains: true, Policy: preloadlist.Custom}
api.database.PutState(testPendingToPreloaded)

w := httptest.NewRecorder()
w.Body = &bytes.Buffer{}

r, err := http.NewRequest("GET", "", nil)
if err != nil {
t.Fatalf("[%s] %s", "NewRequest Failed", err)
func TestUpdate(t *testing.T) {
tests := []struct {
name string
initialDatabaseEntries []database.DomainState
preloadListEntries []preloadlist.Entry
expectedDatabaseEntries []database.DomainState
}{
{
"add new domains",
nil,
[]preloadlist.Entry{
{
Name: "custom-policy.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Custom,
},
{
Name: "bulk-legacy.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.BulkLegacy,
},
{
Name: "bulk-18-weeks.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk18Weeks,
},
{
Name: "bulk-1-year.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
[]database.DomainState{
{
Name: "custom-policy.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Custom,
},
{
Name: "bulk-legacy.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.BulkLegacy,
},
{
Name: "bulk-18-weeks.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Bulk18Weeks,
},
{
Name: "bulk-1-year.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
},
{
"update pending preloading",
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPending,
IncludeSubDomains: true,
},
},
[]preloadlist.Entry{
{
Name: "preloaded.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
},
{
// Test the state transition when a manually managed
// domain gets removed from the list.
"domain removal",
[]database.DomainState{
{
Name: "preloaded-custom.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Custom,
},
},
nil,
[]database.DomainState{
{
Name: "preloaded-custom.test",
Status: database.StatusRemoved,
IncludeSubDomains: true,
// TODO: the policy field should get cleared for removed domains.
Policy: preloadlist.Custom,
},
},
},
{
"update pending removal",
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPendingRemoval,
IncludeSubDomains: true,
},
},
nil,
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusRejected,
// TODO: This state transition should not have this message.
Message: "Domain was added and removed without being preloaded.",
IncludeSubDomains: true,
},
},
},
{
"update pending automated removal",
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPendingAutomatedRemoval,
IncludeSubDomains: true,
},
},
nil,
[]database.DomainState{
{
Name: "preloaded.test",
// TODO: This should be StatusRejected.
Status: database.StatusPendingAutomatedRemoval,
IncludeSubDomains: true,
},
},
},
{
"pending removals stay pending when on list",
[]database.DomainState{
{
Name: "pending-removal.test",
Status: database.StatusPendingRemoval,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
{
Name: "pending-automated-removal.test",
Status: database.StatusPendingAutomatedRemoval,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
[]preloadlist.Entry{
{
Name: "pending-removal.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
{
Name: "pending-automated-removal.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
[]database.DomainState{
{
Name: "pending-removal.test",
Status: database.StatusPendingRemoval,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
{
Name: "pending-automated-removal.test",
// TODO: This should be StatusPendingAutomatedRemoval
Status: database.StatusPreloaded,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
},
{
// Test that Update adds missing policy information for
// an entry whose status is unchanged.
"add missing policy",
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
},
},
[]preloadlist.Entry{
{
Name: "preloaded.test",
Mode: preloadlist.ForceHTTPS,
IncludeSubDomains: true,
Policy: preloadlist.Bulk1Year,
},
},
[]database.DomainState{
{
Name: "preloaded.test",
Status: database.StatusPreloaded,
IncludeSubDomains: true,
// TODO: Uncomment the following line when
// the functionality under test is fixed.
// Policy: preloadlist.Bulk1Year,
},
},
},
}

api.Update(w, r)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// Set up test state
api, _, _, mockPreloadlist := mockAPI(0 * time.Second)
for _, domainState := range test.initialDatabaseEntries {
api.database.PutState(domainState)
}
mockPreloadlist.list = preloadlist.PreloadList{Entries: test.preloadListEntries}

if w.Code != 200 {
t.Errorf("HTTP Response Invalid: Status code is not 200")
}
// Make the call to Update
w := httptest.NewRecorder()
w.Body = &bytes.Buffer{}
r, err := http.NewRequest("GET", "", nil)
if err != nil {
t.Fatalf("NewRequest failed: %v", err)
}
api.Update(w, r)
if w.Code != 200 {
t.Errorf("Expected HTTP status code 200, got %d", w.Code)
}

states, err := api.database.AllDomainStates()
if err != nil {
t.Fatalf("Couldn't get the states of all domains in the database.")
}
// Verify that the updated domain states in the database
// match the expected state.
gotStates, err := api.database.AllDomainStates()
if err != nil {
t.Fatalf("Failed to get database domain states: %v", err)
}

expectedPolicies := map[string]preloadlist.PolicyType{
"garron.net": preloadlist.Bulk18Weeks,
"chromium.org": preloadlist.Custom,
"removal-preloaded-bulk-eligible.test": preloadlist.UnspecifiedPolicyType,
"removal-preloaded-not-bulk-eligible.test": preloadlist.Bulk1Year,
"removal-preloaded-bulk-ineligible.test": preloadlist.Test,
"godoc.og": preloadlist.Custom,
"dev": preloadlist.UnspecifiedPolicyType,
}
expectedStateByName := make(map[string]database.DomainState)
for _, expectedEntry := range test.expectedDatabaseEntries {
expectedStateByName[expectedEntry.Name] = expectedEntry
}

for _, state := range states {
if state.Policy != expectedPolicies[state.Name] {
t.Errorf("Policy field not accurately populated in the database for %s with %s", state.Policy, expectedPolicies[state.Name])
}
if len(gotStates) != len(test.expectedDatabaseEntries) {
t.Errorf("Expected %d entries in the database; found %d", len(test.expectedDatabaseEntries), len(gotStates))
}
for _, gotState := range gotStates {
name := gotState.Name
wantState, ok := expectedStateByName[name]
if !ok {
t.Errorf("State for %q unexpectedly in database", name)
continue
}
if !gotState.MatchesWanted(wantState) {
t.Errorf("For domain %q: got %+v, want %+v", name, gotState, wantState)
}
}
})
}
}
6 changes: 5 additions & 1 deletion database/domainstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func (s DomainState) MatchesWanted(wanted DomainState) bool {
if wanted.Status != s.Status {
return false
}
if wanted.Policy != s.Policy {
return false
}
if wanted.Message != "" && wanted.Message != s.Message {
return false
}
Expand All @@ -71,7 +74,8 @@ func (s DomainState) Equal(s2 DomainState) bool {
return s.Name == s2.Name && s.Status == s2.Status &&
s.Message == s2.Message &&
s.SubmissionDate.Equal(s2.SubmissionDate) &&
s.IncludeSubDomains == s2.IncludeSubDomains
s.IncludeSubDomains == s2.IncludeSubDomains &&
s.Policy == s2.Policy
}

// ToEntry converts a DomainState to a preloadlist.Entry.
Expand Down

0 comments on commit 516aec6

Please sign in to comment.