From 30ffedebfe95ea9dc8a600506c821e00d4d38f04 Mon Sep 17 00:00:00 2001 From: spbsoluble <1661003+spbsoluble@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:41:09 -0700 Subject: [PATCH 1/2] fix(template): paginate GetTemplates to return all templates GetTemplates() issued a single GET /Templates with no query parameters. The Command /Templates endpoint returns a default page of 50 results (CommonName ascending) when no paging parameters are supplied, so the client silently truncated at 50 templates and never paged through the remainder. Callers resolving a template by name (e.g. the Terraform provider's template_role_binding resource) failed with a spurious "template not found" on instances with more than 50 templates. Paginate automatically with PageReturned/ReturnLimit, mirroring the existing ListApplications() pattern: accumulate pages and stop when a page returns fewer than the page size. Adds TestGetTemplates_Pagination (serves >1 page, asserts a late-sorting template is returned) and TestGetTemplates_SinglePage. Fixes #54 --- v3/api/template.go | 57 +++++++++++++++-------- v3/api/template_test.go | 101 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 v3/api/template_test.go diff --git a/v3/api/template.go b/v3/api/template.go index 73e7423..b8b450f 100644 --- a/v3/api/template.go +++ b/v3/api/template.go @@ -18,6 +18,8 @@ import ( "encoding/json" "errors" "fmt" + "log" + "strconv" ) // GetTemplate takes arguments for a template ID used to facilitate the retrieval @@ -59,9 +61,12 @@ func (c *Client) GetTemplate(Id interface{}) (*GetTemplateResponse, error) { return jsonResp, err } -// GetTemplates asks Keyfactor for a complete list of known certificate templates. A list of -// GetTemplateResponse structures is returned, containing the template context. +// GetTemplates asks Keyfactor for a complete list of known certificate templates, +// paginating automatically so that instances with more than the server's default +// page size (50) return all templates. A list of GetTemplateResponse structures +// is returned, containing the template context. func (c *Client) GetTemplates() ([]GetTemplateResponse, error) { + log.Println("[INFO] Listing certificate templates.") // Set Keyfactor-specific headers headers := &apiHeaders{ @@ -71,25 +76,37 @@ func (c *Client) GetTemplates() ([]GetTemplateResponse, error) { }, } - keyfactorAPIStruct := &request{ - Method: "GET", - Endpoint: "Templates/", - Headers: headers, - Query: nil, - Payload: nil, + const pageSize = 100 + var all []GetTemplateResponse + for page := 1; ; page++ { + keyfactorAPIStruct := &request{ + Method: "GET", + Endpoint: "Templates/", + Headers: headers, + Query: &apiQuery{ + Query: []StringTuple{ + {"PageReturned", strconv.Itoa(page)}, + {"ReturnLimit", strconv.Itoa(pageSize)}, + }, + }, + Payload: nil, + } + + resp, err := c.sendRequest(keyfactorAPIStruct) + if err != nil { + return nil, err + } + + var pageResults []GetTemplateResponse + if err = json.NewDecoder(resp.Body).Decode(&pageResults); err != nil { + return nil, err + } + all = append(all, pageResults...) + if len(pageResults) < pageSize { + break + } } - - resp, err := c.sendRequest(keyfactorAPIStruct) - if err != nil { - return nil, err - } - - var jsonResp []GetTemplateResponse - err = json.NewDecoder(resp.Body).Decode(&jsonResp) - if err != nil { - return nil, err - } - return jsonResp, err + return all, nil } // UpdateTemplate takes arguments for a UpdateTemplateArg structure used to facilitate the modification diff --git a/v3/api/template_test.go b/v3/api/template_test.go new file mode 100644 index 0000000..6c652cf --- /dev/null +++ b/v3/api/template_test.go @@ -0,0 +1,101 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "testing" +) + +// TestGetTemplates_Pagination verifies that GetTemplates fetches all pages. +// Before the fix, only the first 50 results were returned; a template sorted +// beyond position 50 (e.g. position 169 out of 278) would never be found by +// name, causing "Error template name not found" in keyfactor_template_role_binding. +func TestGetTemplates_Pagination(t *testing.T) { + const totalTemplates = 278 + + allTemplates := make([]GetTemplateResponse, totalTemplates) + for i := range allTemplates { + allTemplates[i] = GetTemplateResponse{ + Id: i + 1, + CommonName: "Template-" + strconv.Itoa(i+1), + } + } + // Place the known-problematic late-sorted template at index 168 (position 169). + allTemplates[168] = GetTemplateResponse{Id: 243, CommonName: "zzz-late-sorted-template"} + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page, _ := strconv.Atoi(r.URL.Query().Get("PageReturned")) + limit, _ := strconv.Atoi(r.URL.Query().Get("ReturnLimit")) + if page < 1 { + page = 1 + } + if limit < 1 { + limit = 50 + } + start := (page - 1) * limit + end := start + limit + if start >= len(allTemplates) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("[]")) + return + } + if end > len(allTemplates) { + end = len(allTemplates) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(allTemplates[start:end]) + })) + defer srv.Close() + + c := newTestClient(srv) + + templates, err := c.GetTemplates() + if err != nil { + t.Fatalf("GetTemplates() error: %v", err) + } + if len(templates) != totalTemplates { + t.Errorf("GetTemplates() returned %d templates, want %d (pagination broken)", len(templates), totalTemplates) + } + + // Verify the late-sorted target template is present. + found := false + for _, tmpl := range templates { + if tmpl.CommonName == "zzz-late-sorted-template" { + found = true + if tmpl.Id != 243 { + t.Errorf("target template ID = %d, want 243", tmpl.Id) + } + break + } + } + if !found { + t.Errorf("target template %q (position 169) not found — page 2+ results missing", "zzz-late-sorted-template") + } +} + +// TestGetTemplates_SinglePage verifies that a sub-pageSize result terminates +// the pagination loop in a single call. +func TestGetTemplates_SinglePage(t *testing.T) { + calls := 0 + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]GetTemplateResponse{{Id: 1, CommonName: "OnlyTemplate"}}) + })) + defer srv.Close() + + c := newTestClient(srv) + + templates, err := c.GetTemplates() + if err != nil { + t.Fatalf("GetTemplates() error: %v", err) + } + if len(templates) != 1 { + t.Errorf("got %d templates, want 1", len(templates)) + } + if calls != 1 { + t.Errorf("server called %d times, want 1", calls) + } +} From 5400f361d2f049a4b9b4daafe1b7e10b5168e757 Mon Sep 17 00:00:00 2001 From: spbsoluble <1661003+spbsoluble@users.noreply.github.com> Date: Fri, 19 Jun 2026 16:36:58 -0700 Subject: [PATCH 2/2] fix(template): harden GetTemplates pagination (audit fixes) Security/compliance audit follow-ups to the pagination change: - Bound the loop with getTemplatesMaxPages (10000) and an explicit empty-page break; return an error instead of looping forever if the server ignores paging or always returns a full page (DoS guard). - Close resp.Body every iteration (incl. the decode-error path) to avoid a per-page connection/fd leak. - Add an [INFO] outcome log (count + pages) and [ERROR] logs on the request/decode failure paths for audit traceability; no response bodies, tokens, or template contents are logged. Adds TestGetTemplates_MaxPagesGuard (server ignores paging -> bounded error, not a hang). --- v3/api/template.go | 25 +++++++++++++++++++++---- v3/api/template_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/v3/api/template.go b/v3/api/template.go index b8b450f..161f703 100644 --- a/v3/api/template.go +++ b/v3/api/template.go @@ -22,6 +22,11 @@ import ( "strconv" ) +// getTemplatesMaxPages is the maximum number of pages GetTemplates will fetch +// before aborting with an error. It is an unexported package-level var (not a +// const) so that tests can lower it without iterating thousands of times. +var getTemplatesMaxPages = 10000 + // GetTemplate takes arguments for a template ID used to facilitate the retrieval // of certificate template context. The primary query required to get certificate context is the template ID. A pointer // to a GetTemplateResponse structure is returned, containing the template context. @@ -78,7 +83,8 @@ func (c *Client) GetTemplates() ([]GetTemplateResponse, error) { const pageSize = 100 var all []GetTemplateResponse - for page := 1; ; page++ { + var page int + for page = 1; page <= getTemplatesMaxPages; page++ { keyfactorAPIStruct := &request{ Method: "GET", Endpoint: "Templates/", @@ -94,18 +100,29 @@ func (c *Client) GetTemplates() ([]GetTemplateResponse, error) { resp, err := c.sendRequest(keyfactorAPIStruct) if err != nil { + log.Printf("[ERROR] GetTemplates: request for page %d failed: %s", page, err) return nil, err } var pageResults []GetTemplateResponse - if err = json.NewDecoder(resp.Body).Decode(&pageResults); err != nil { - return nil, err + decodeErr := json.NewDecoder(resp.Body).Decode(&pageResults) + resp.Body.Close() + if decodeErr != nil { + log.Printf("[ERROR] GetTemplates: failed to decode page %d: %s", page, decodeErr) + return nil, decodeErr } + all = append(all, pageResults...) - if len(pageResults) < pageSize { + if len(pageResults) == 0 || len(pageResults) < pageSize { break } } + + if page > getTemplatesMaxPages { + return nil, fmt.Errorf("GetTemplates: exceeded max pages (%d); server may be ignoring pagination", getTemplatesMaxPages) + } + + log.Printf("[INFO] Listed %d certificate templates across %d page(s).", len(all), page) return all, nil } diff --git a/v3/api/template_test.go b/v3/api/template_test.go index 6c652cf..d6bc71a 100644 --- a/v3/api/template_test.go +++ b/v3/api/template_test.go @@ -75,6 +75,37 @@ func TestGetTemplates_Pagination(t *testing.T) { } } +// TestGetTemplates_MaxPagesGuard verifies that GetTemplates aborts with an error +// when the server always returns a full page (simulating a server that ignores +// pagination and would otherwise cause an infinite loop / unbounded memory growth). +func TestGetTemplates_MaxPagesGuard(t *testing.T) { + // Build a fixed full page of pageSize (100) items. + fullPage := make([]GetTemplateResponse, 100) + for i := range fullPage { + fullPage[i] = GetTemplateResponse{Id: i + 1, CommonName: "Template-" + strconv.Itoa(i+1)} + } + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Always return a full page regardless of PageReturned — simulates a + // server that ignores paging parameters. + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(fullPage) + })) + defer srv.Close() + + // Lower the safety bound so the test terminates quickly. + orig := getTemplatesMaxPages + getTemplatesMaxPages = 3 + defer func() { getTemplatesMaxPages = orig }() + + c := newTestClient(srv) + + _, err := c.GetTemplates() + if err == nil { + t.Fatal("GetTemplates() expected an error when max pages exceeded, got nil") + } +} + // TestGetTemplates_SinglePage verifies that a sub-pageSize result terminates // the pagination loop in a single call. func TestGetTemplates_SinglePage(t *testing.T) {