From 569331df11bef28956b6219a3b7074daf60815c4 Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Fri, 7 Aug 2026 01:49:19 -0300 Subject: [PATCH 1/2] fix(sendgrid): stop on-behalf-of teammate cache collision and 403 abort (CXP-860) uhttp's cache key doesn't include the on-behalf-of header, so every subuser's teammate list request collided with the parent account's cached response, silently suppressing every sub-account-only teammate. Bypass the cache for on-behalf-of requests with uhttp.WithNoCache(). GetTeammatesSubAccess also returns 403 by design for a teammate that only exists inside a subuser (subuser_access only applies to parent-account teammates), which aborted the whole Grants sync once those teammates started being emitted. Tolerate PermissionDenied there the same way listSubuserTeammates already does for List. Co-Authored-By: Claude Sonnet 5 --- pkg/connector/client/client.go | 10 ++++- pkg/connector/teammates.go | 16 +++++--- pkg/connector/teammates_test.go | 69 ++++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 8 deletions(-) diff --git a/pkg/connector/client/client.go b/pkg/connector/client/client.go index 6364bb44..ce5b5838 100644 --- a/pkg/connector/client/client.go +++ b/pkg/connector/client/client.go @@ -407,12 +407,18 @@ func getTokenValue(pToken *pagination.Token) (int, error) { // onBehalfOfOpts builds the optional on-behalf-of header request option. // Returns nil (no extra options) when onBehalfOf is empty, so the same call -// path works for parent-scope and subuser-scoped requests. +// path works for parent-scope and subuser-scoped requests. WithNoCache is +// required here: uhttp's cache key doesn't include on-behalf-of, so without +// it every subuser would be served the parent (or another subuser's) cached +// response for the same URL. func onBehalfOfOpts(onBehalfOf OnBehalfOf) []uhttp.RequestOption { if onBehalfOf == "" { return nil } - return []uhttp.RequestOption{uhttp.WithHeader(OnBehalfOfHeaderName, string(onBehalfOf))} + return []uhttp.RequestOption{ + uhttp.WithHeader(OnBehalfOfHeaderName, string(onBehalfOf)), + uhttp.WithNoCache(), + } } func (h *SendGridClient) doRequest( diff --git a/pkg/connector/teammates.go b/pkg/connector/teammates.go index 0e0be5b7..b3faa7b4 100644 --- a/pkg/connector/teammates.go +++ b/pkg/connector/teammates.go @@ -218,15 +218,21 @@ func (u *teammateBuilder) Grants(ctx context.Context, resource *v2.Resource, opt return nil, nil, err } - // Subuser access grants. + logger := ctxzap.Extract(ctx) + + // Subuser access grants. SendGrid returns 403 here for a teammate that + // only exists inside a subuser, by design, so treat it as "no access to + // report" rather than a fatal error. access, nextToken, err := u.client.GetTeammatesSubAccess(ctx, sgclient.Username(username), &opts.PageToken, sgclient.OnBehalfOf(onBehalfOf)) if err != nil { - return nil, nil, fmt.Errorf("baton-sendgrid: failed to get teammate subuser access for %s: %w", username, err) + if status.Code(err) == codes.PermissionDenied { + logger.Debug("baton-sendgrid: subuser_access forbidden for subuser-scoped teammate, skipping", zap.String("username", username), zap.Error(err)) + access, nextToken = nil, "" + } else { + return nil, nil, fmt.Errorf("baton-sendgrid: failed to get teammate subuser access for %s: %w", username, err) + } } - logger := ctxzap.Extract(ctx) - logger.Info("Teammate grants", zap.String("username", username), zap.Any("COUNT", access)) - for _, subAccess := range access { grants, err := createGrantSubuserFromTeammate(resource, subAccess) if err != nil { diff --git a/pkg/connector/teammates_test.go b/pkg/connector/teammates_test.go index cb293971..b01afeba 100644 --- a/pkg/connector/teammates_test.go +++ b/pkg/connector/teammates_test.go @@ -83,6 +83,16 @@ type fakeSendGridClient struct { subusers []models.Subuser // subuserTeammates maps subuser username -> teammates visible to it via on-behalf-of. subuserTeammates map[string][]*models.Teammate + + subAccess []*models.TeammateSubuser + subAccessErr error +} + +func (f *fakeSendGridClient) GetTeammatesSubAccess(_ context.Context, _ sgclient.Username, pToken *pagination.Token, _ sgclient.OnBehalfOf) ([]*models.TeammateSubuser, string, error) { + if f.subAccessErr != nil { + return nil, "", f.subAccessErr + } + return pageOneAtATime(f.subAccess, pToken) } func (f *fakeSendGridClient) GetTeammates(_ context.Context, pToken *pagination.Token, onBehalfOf sgclient.OnBehalfOf) ([]*models.Teammate, string, error) { @@ -110,12 +120,17 @@ func (f *fakeSendGridClient) GetSubuserUsernameByID(_ context.Context, subuserID // GetSpecificTeammate backs isParentScopeTeammate's dedup check: onBehalfOf // "" means "does this username exist at parent scope", answered against // globalTeammates, mirroring the real API's 404-for-missing behavior. -func (f *fakeSendGridClient) GetSpecificTeammate(_ context.Context, username sgclient.Username, _ sgclient.OnBehalfOf) (*models.TeammateScope, error) { +func (f *fakeSendGridClient) GetSpecificTeammate(_ context.Context, username sgclient.Username, onBehalfOf sgclient.OnBehalfOf) (*models.TeammateScope, error) { for _, tm := range f.globalTeammates { if tm.Username == string(username) { return &models.TeammateScope{Teammate: *tm}, nil } } + for _, tm := range f.subuserTeammates[string(onBehalfOf)] { + if tm.Username == string(username) { + return &models.TeammateScope{Teammate: *tm}, nil + } + } return nil, status.Error(codes.NotFound, "teammate does not exist") } @@ -263,3 +278,55 @@ func TestTeammateBuilder_List_TeammateRestrictedToMultipleSubusers(t *testing.T) require.Empty(t, sub2Resources, "restricted-1 must not be re-emitted under a second subuser with a conflicting ParentResourceId") } + +func TestTeammateBuilder_Grants_SubuserAccessForbidden(t *testing.T) { + client := &fakeSendGridClient{ + subusers: []models.Subuser{ + {Id: 1, Username: "sub1", Email: "sub1@example.com"}, + }, + subuserTeammates: map[string][]*models.Teammate{ + "sub1": { + {Username: "local-1", Email: "local-1@example.com"}, + }, + }, + subAccessErr: status.Error(codes.PermissionDenied, "403 Forbidden"), + } + + sub1ResourceID, err := rs.NewResourceID(subuserResourceType, 1) + require.NoError(t, err) + + resource, err := teammateResource(&models.Teammate{Username: "local-1", Email: "local-1@example.com"}, sub1ResourceID, "sub1") + require.NoError(t, err) + + tb := newTeammateBuilder(client) + grants, results, err := tb.Grants(context.Background(), resource, rs.SyncOpAttrs{PageToken: pagination.Token{}}) + + require.NoError(t, err, "a 403 from subuser_access must not abort Grants for a subuser-only teammate") + require.Empty(t, grants) + require.Equal(t, "", results.NextPageToken) +} + +func TestTeammateBuilder_Grants_SubuserAccessOtherErrorPropagates(t *testing.T) { + client := &fakeSendGridClient{ + subusers: []models.Subuser{ + {Id: 1, Username: "sub1", Email: "sub1@example.com"}, + }, + subuserTeammates: map[string][]*models.Teammate{ + "sub1": { + {Username: "local-1", Email: "local-1@example.com"}, + }, + }, + subAccessErr: status.Error(codes.Unavailable, "upstream unavailable"), + } + + sub1ResourceID, err := rs.NewResourceID(subuserResourceType, 1) + require.NoError(t, err) + + resource, err := teammateResource(&models.Teammate{Username: "local-1", Email: "local-1@example.com"}, sub1ResourceID, "sub1") + require.NoError(t, err) + + tb := newTeammateBuilder(client) + _, _, err = tb.Grants(context.Background(), resource, rs.SyncOpAttrs{PageToken: pagination.Token{}}) + + require.Error(t, err, "only PermissionDenied should be tolerated, other errors must still propagate") +} From a92ac02fc1c60f84133196a527535b7ff3850009 Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Fri, 7 Aug 2026 11:03:18 -0300 Subject: [PATCH 2/2] fix(sendgrid): disable cache for parent-scope teammate lookups too (CXP-860) uhttp's cache key ignores the on-behalf-of header, so a subuser-scoped write (no-cache read, unconditional write) could poison the shared cache entry for a later parent-scope read on the same path+query, causing isParentScopeTeammate to wrongly conclude a teammate exists at parent scope and suppress it from the sync. Co-Authored-By: Claude Sonnet 5 --- pkg/connector/client/client.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/connector/client/client.go b/pkg/connector/client/client.go index ce5b5838..10a722a9 100644 --- a/pkg/connector/client/client.go +++ b/pkg/connector/client/client.go @@ -405,20 +405,14 @@ func getTokenValue(pToken *pagination.Token) (int, error) { return value, nil } -// onBehalfOfOpts builds the optional on-behalf-of header request option. -// Returns nil (no extra options) when onBehalfOf is empty, so the same call -// path works for parent-scope and subuser-scoped requests. WithNoCache is -// required here: uhttp's cache key doesn't include on-behalf-of, so without -// it every subuser would be served the parent (or another subuser's) cached -// response for the same URL. +// onBehalfOfOpts always disables caching: uhttp's cache key ignores the +// on-behalf-of header, so parent- and subuser-scoped calls would otherwise collide. func onBehalfOfOpts(onBehalfOf OnBehalfOf) []uhttp.RequestOption { - if onBehalfOf == "" { - return nil - } - return []uhttp.RequestOption{ - uhttp.WithHeader(OnBehalfOfHeaderName, string(onBehalfOf)), - uhttp.WithNoCache(), + opts := []uhttp.RequestOption{uhttp.WithNoCache()} + if onBehalfOf != "" { + opts = append(opts, uhttp.WithHeader(OnBehalfOfHeaderName, string(onBehalfOf))) } + return opts } func (h *SendGridClient) doRequest(