Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/conductorone/baton-hubspot
go 1.25.2

require (
github.com/conductorone/baton-sdk v0.26.0
github.com/conductorone/baton-sdk v0.28.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: the PR description says this pins an unmerged SDK branch pseudo-version, but the diff now pins released v0.28.0 — worth updating the description. Also worth confirming go mod vendor was re-run after the retarget: the vendored pkg/sdk/version.go reports v0.27.0 under a v0.28.0 module, and go build -mod=vendor won't catch vendored content that doesn't match the released module. (low confidence — the const may simply lag upstream releases.)

github.com/ennyjfrick/ruleguard-logfatal v0.0.2
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
github.com/quasilyte/go-ruleguard/dsl v0.3.23
Expand Down Expand Up @@ -49,9 +49,9 @@ require (
github.com/cockroachdb/redact v1.1.5 // indirect
github.com/cockroachdb/swiss v0.0.0-20251224182025-b0f6560f979b // indirect
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
github.com/conductorone/dpop v0.2.6 // indirect
github.com/conductorone/dpop/integrations/dpop_grpc v0.2.4 // indirect
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.2.5 // indirect
github.com/conductorone/dpop v0.3.0 // indirect
github.com/conductorone/dpop/integrations/dpop_grpc v0.3.0 // indirect
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.3.0 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/deckarep/golang-set/v2 v2.9.0 // indirect
github.com/doug-martin/goqu/v9 v9.19.0 // indirect
Expand Down
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ github.com/cockroachdb/swiss v0.0.0-20251224182025-b0f6560f979b h1:VXvSNzmr8hMj8
github.com/cockroachdb/swiss v0.0.0-20251224182025-b0f6560f979b/go.mod h1:yBRu/cnL4ks9bgy4vAASdjIW+/xMlFwuHKqtmh3GZQg=
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 h1:zuQyyAKVxetITBuuhv3BI9cMrmStnpT18zmgmTxunpo=
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ=
github.com/conductorone/baton-sdk v0.26.0 h1:aNKg81BhPAVGyYe+W4czZJnL9hzJEUkDinbt0klOeo4=
github.com/conductorone/baton-sdk v0.26.0/go.mod h1:SKm95z4KkQ23Tufo2ys88lVzbwKb0AQEbKee5GE0Lig=
github.com/conductorone/dpop v0.2.6 h1:fakwai/Xm2b/fcDUwJN41WtcSI/2UhQOyRIVvnnrrNA=
github.com/conductorone/dpop v0.2.6/go.mod h1:gyo8TtzB9SCFCsjsICH4IaLZ7y64CcrDXMOPBwfq/3s=
github.com/conductorone/dpop/integrations/dpop_grpc v0.2.4 h1:lYxYi9/WTSL9sE96CO0QF2BY3kehs8dTTApI134TGCA=
github.com/conductorone/dpop/integrations/dpop_grpc v0.2.4/go.mod h1:LYNoUc1lkvozk9HBio+xI2w8YyfYy0v2cAJtIgrkj8o=
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.2.5 h1:x/ZtD0YLNwlmoSv9SE4OBPJB9Hj2cpwyE5BAfia7aY8=
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.2.5/go.mod h1:2eI0qv+XaEhoCw0GKFF1yH4X8Mp4KLVEVnQKRFEy4zs=
github.com/conductorone/baton-sdk v0.28.0 h1:XDOkPYeC8f3sc3UupzfqYRShjD1uvY3ux0BT5gMmSio=
github.com/conductorone/baton-sdk v0.28.0/go.mod h1:i4DXDGaiyHyg164r4VbLu734z+MF0TyYjscOaRYd92M=
github.com/conductorone/dpop v0.3.0 h1:j5fZk0VqepGKYo+/NDikCOMsZcgs4HO4i0k56wRel5g=
github.com/conductorone/dpop v0.3.0/go.mod h1:gyo8TtzB9SCFCsjsICH4IaLZ7y64CcrDXMOPBwfq/3s=
github.com/conductorone/dpop/integrations/dpop_grpc v0.3.0 h1:R2uxHBtStgUn7cxbAnT3mAj6/e1akfP2Pj/hEPFB0D8=
github.com/conductorone/dpop/integrations/dpop_grpc v0.3.0/go.mod h1:f30gFNZHGkbPlufIDqzg5cr7llQQS0r5VQ+bJCnrDik=
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.3.0 h1:g9OX0PW9DQyrGy/Tp2lesky/V1VH55cTY8z0Iq0ksOo=
github.com/conductorone/dpop/integrations/dpop_oauth2 v0.3.0/go.mod h1:RZqiSQdi4NXnoZtB3qG3XYN/6L4dHZy6jctEr5K5oPk=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
39 changes: 26 additions & 13 deletions pkg/hubspot/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ func (c *Client) usersURL() string {
return c.baseURL.JoinPath("settings/users/2026-03").String()
}

// listUsersURL is the paginated list endpoint. 2026-09-beta returns
// paging.next.after with limit=50; 2026-03 omits paging and silently truncates.
func (c *Client) listUsersURL() string {
return c.baseURL.JoinPath("settings/users/2026-09-beta").String()
}

func (c *Client) userURL(userID string) string {
return c.baseURL.JoinPath("settings/users/2026-03", userID).String()
}
Expand All @@ -59,8 +53,20 @@ func (c *Client) accountLastLoginURL() string {
}

type UsersResponse struct {
Results []User `json:"results"`
Paging PaginationData `json:"paging"`
Results []User `json:"results"`
Paging *PaginationData `json:"paging"`
}

// HasPaginationData makes UsersResponse a uhttp.PaginatedResponse.
// WithPaginationData unmarshals the whole body into whatever it is given, so the
// receiver has to mirror the top level of the response; an inner field would
// decode against the wrong level and always report nothing.
//
// Paging is a pointer because encoding/json leaves a value field zero whether
// the key was absent or empty, and those mean opposite things here: absent is
// the endpoint truncating silently, empty is a legitimate last page.
func (u *UsersResponse) HasPaginationData() bool {
return u.Paging != nil
}

type AccountLoginResponse struct {
Expand Down Expand Up @@ -144,16 +150,17 @@ func (c *Client) GetUsers(ctx context.Context, getUsersVars GetUsersVars) ([]Use

annos, err := c.get(
ctx,
c.listUsersURL(),
c.usersURL(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Bug: this switches the user list back to settings/users/2026-03 (base used listUsersURL()2026-09-beta), and the comment deleted in this same PR states that 2026-03 "omits paging and silently truncates". Pairing that endpoint with the new hard WithPaginationData assertion means every list call — user sync, account grants (pkg/connector/account.go:87), role grants (pkg/connector/role.go:110) — fails with ErrMissingPaginationData, or, if paging is sometimes present, re-introduces the truncation this PR is meant to close. The endpoint change isn't mentioned in the PR description; if it wasn't intentional, restore c.listUsersURL(). (high confidence)

&userResponse,
queryParams,
uhttp.WithPaginationData(&userResponse),
)

if err != nil {
return nil, "", nil, err
}

if (userResponse.Paging != PaginationData{}) {
if userResponse.Paging != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If pagination is nil we should error the connector in a way that alerts us. Maybe we need a new well known error that pages. We expect this to be here. I think it is unlikely this will ever happen again for this service. This was a massive bug in their API, that feels like they will have resolved and will not flap back and forth. We generally should be able to expect that the documented API shape can be relied upon and when it doesn't in any way, not just missing pagination that it blows up in a way we or a self healing agent is notified to go fix right away.

return userResponse.Results, userResponse.Paging.Next.After, annos, nil
}

Expand Down Expand Up @@ -330,8 +337,14 @@ func (c *Client) GetUserLastLogin(ctx context.Context, userId string) (*time.Tim
return nil, annos, nil
}

func (c *Client) get(ctx context.Context, url string, resourceResponse interface{}, queryParams url.Values) (annotations.Annotations, error) {
return c.doRequest(ctx, url, http.MethodGet, nil, resourceResponse, queryParams)
func (c *Client) get(
ctx context.Context,
url string,
resourceResponse interface{},
queryParams url.Values,
doOptions ...uhttp.DoOption,
) (annotations.Annotations, error) {
return c.doRequest(ctx, url, http.MethodGet, nil, resourceResponse, queryParams, doOptions...)
}

func (c *Client) put(ctx context.Context, url string, data interface{}, resourceResponse interface{}) (annotations.Annotations, error) {
Expand All @@ -353,6 +366,7 @@ func (c *Client) doRequest(
data interface{},
resourceResponse interface{},
queryParams url.Values,
doOptions ...uhttp.DoOption,
) (annotations.Annotations, error) {
parsedURL, err := url.Parse(urlAddress)
if err != nil {
Expand All @@ -376,7 +390,6 @@ func (c *Client) doRequest(
return nil, err
}

var doOptions []uhttp.DoOption
if resourceResponse != nil {
doOptions = append(doOptions, uhttp.WithJSONResponse(resourceResponse))
}
Expand Down
75 changes: 75 additions & 0 deletions pkg/hubspot/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package hubspot

import (
"context"
"errors"
"net/http"
"net/http/httptest"
"testing"

"github.com/conductorone/baton-sdk/pkg/uhttp"
)

// newTestClient spins up a server that always replies with body and returns a
// client pointed at it. Each case gets its own server so uhttp's response cache
// never serves one case's body to another.
func newTestClient(t *testing.T, body string) *Client {
t.Helper()

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: the handler replies with the same body for any path, so nothing in the suite pins which endpoint GetUsers calls — that's why the 2026-09-beta2026-03 switch on client.go:153 passes green. Capture r.URL.Path in the handler and assert it in at least one case so the list endpoint is covered by tests.

_, _ = w.Write([]byte(body))
}))
t.Cleanup(server.Close)

client, err := NewClient("token", server.Client(), server.URL+"/")
if err != nil {
t.Fatalf("NewClient: %v", err)
}
return client
}

func TestGetUsersReturnsNextPage(t *testing.T) {
client := newTestClient(t, `{"results":[{"id":"1","email":"a@example.com"}],"paging":{"next":{"after":"50"}}}`)

users, nextPage, _, err := client.GetUsers(context.Background(), GetUsersVars{Limit: 50})
if err != nil {
t.Fatalf("GetUsers: %v", err)
}
if len(users) != 1 || users[0].Id != "1" {
t.Errorf("got users %+v, want a single user with id 1", users)
}
if nextPage != "50" {
t.Errorf("got next page %q, want %q", nextPage, "50")
}
}

// The last page still carries a paging object, just without a cursor.
func TestGetUsersLastPage(t *testing.T) {
client := newTestClient(t, `{"results":[{"id":"1","email":"a@example.com"}],"paging":{}}`)

users, nextPage, _, err := client.GetUsers(context.Background(), GetUsersVars{Limit: 50})
if err != nil {
t.Fatalf("GetUsers: %v", err)
}
if len(users) != 1 {
t.Errorf("got %d users, want 1", len(users))
}
if nextPage != "" {
t.Errorf("got next page %q, want empty", nextPage)
}
}

// A 200 with results but no paging object means the API truncated the list
// without telling us. That must be an error, not a short sync.
func TestGetUsersMissingPagingErrors(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion (high confidence): all three tests call GetUsers with GetUsersVars{Limit: 50}, i.e. After == "", so the new first-page-only conditional is entirely unexercised — deleting the if would leave this suite green. Add a case that passes After: "50" against a body with no paging and asserts it returns no error and an empty next page, which pins the "later pages are deliberately not asserted" behavior in place.

client := newTestClient(t, `{"results":[{"id":"1","email":"a@example.com"}]}`)

_, _, _, err := client.GetUsers(context.Background(), GetUsersVars{Limit: 50})
if err == nil {
t.Fatal("GetUsers: expected an error when the response omits paging")
}
if !errors.Is(err, uhttp.ErrMissingPaginationData) {
t.Errorf("got error %v, want it to wrap ErrMissingPaginationData", err)
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion vendor/github.com/conductorone/baton-sdk/pkg/sync/syncer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions vendor/github.com/conductorone/baton-sdk/pkg/uhttp/pagination.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 11 additions & 4 deletions vendor/github.com/conductorone/baton-sdk/pkg/uhttp/wrapper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading