Address SendGrid connector review feedback
- Add SendGrid third-party logo and wire it into ThirdPartyLogo - Add SendGrid name resolver (account company name, graceful fallback) - Fix MFA detection: full-access teammates carry both 2fa_exempt and 2fa_required, so report Unknown unless exactly one is present - Re-record the driver cassette against the live API - Use a random time suffix for the migration filename Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -486,6 +486,54 @@ func (r *anthropicNameResolver) ResolveInstanceName(ctx context.Context) (string
|
||||
return resp.Name, nil
|
||||
}
|
||||
|
||||
// sendGridNameResolver resolves the SendGrid account's company name from
|
||||
// the user profile endpoint, used as the AccessSource instance label.
|
||||
type sendGridNameResolver struct {
|
||||
httpClient *http.Client
|
||||
}
|
||||
|
||||
func NewSendGridNameResolver(httpClient *http.Client) NameResolver {
|
||||
return &sendGridNameResolver{httpClient: httpClient}
|
||||
}
|
||||
|
||||
func (r *sendGridNameResolver) ResolveInstanceName(ctx context.Context) (string, error) {
|
||||
req, err := http.NewRequestWithContext(
|
||||
ctx,
|
||||
http.MethodGet,
|
||||
"https://api.sendgrid.com/v3/user/profile",
|
||||
nil,
|
||||
)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("cannot create sendgrid profile request: %w", err)
|
||||
}
|
||||
|
||||
req.Header.Set("Accept", "application/json")
|
||||
|
||||
httpResp, err := r.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("cannot execute sendgrid profile request: %w", err)
|
||||
}
|
||||
|
||||
defer func() { _ = httpResp.Body.Close() }()
|
||||
|
||||
// Best-effort: a non-2xx (revoked key, or a key without the
|
||||
// user.profile.read scope) must not make the source-name worker retry
|
||||
// forever. Give up gracefully and keep the generic source name; a dead
|
||||
// key surfaces on the next ListAccounts.
|
||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||
return "", nil
|
||||
}
|
||||
|
||||
var resp struct {
|
||||
Company string `json:"company"`
|
||||
}
|
||||
if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil {
|
||||
return "", fmt.Errorf("cannot decode sendgrid profile response: %w", err)
|
||||
}
|
||||
|
||||
return resp.Company, nil
|
||||
}
|
||||
|
||||
// sentryNameResolver resolves the Sentry organization name.
|
||||
type sentryNameResolver struct {
|
||||
httpClient *http.Client
|
||||
|
||||
@@ -212,15 +212,31 @@ func sendGridRole(userType string, isAdmin bool) string {
|
||||
}
|
||||
}
|
||||
|
||||
// sendGridMFAStatus derives a teammate's MFA status from the auto-set 2fa
|
||||
// scopes SendGrid attaches to the teammate detail. A restricted teammate
|
||||
// carries exactly one of them to reflect their real status. Full-access
|
||||
// users (the account owner and full-access teammates) are the exception:
|
||||
// their scope list is the entire catalog and therefore contains BOTH
|
||||
// 2fa_exempt and 2fa_required, which says nothing about their actual MFA.
|
||||
// Only report a definitive status when exactly one scope is present;
|
||||
// both-or-neither is ambiguous, so report Unknown rather than guessing.
|
||||
func sendGridMFAStatus(scopes []string) coredata.MFAStatus {
|
||||
var exempt, required bool
|
||||
for _, scope := range scopes {
|
||||
switch scope {
|
||||
case "2fa_exempt":
|
||||
return coredata.MFAStatusDisabled
|
||||
exempt = true
|
||||
case "2fa_required":
|
||||
return coredata.MFAStatusEnabled
|
||||
required = true
|
||||
}
|
||||
}
|
||||
|
||||
return coredata.MFAStatusUnknown
|
||||
switch {
|
||||
case required && !exempt:
|
||||
return coredata.MFAStatusEnabled
|
||||
case exempt && !required:
|
||||
return coredata.MFAStatusDisabled
|
||||
default:
|
||||
return coredata.MFAStatusUnknown
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,30 +33,22 @@ func TestSendGridDriver(t *testing.T) {
|
||||
|
||||
records, err := driver.ListAccounts(context.Background())
|
||||
require.NoError(t, err)
|
||||
require.Len(t, records, 3)
|
||||
require.Len(t, records, 1)
|
||||
|
||||
// Recorded against a live SendGrid account that has only the owner. The
|
||||
// list endpoint carries no scopes, so the driver fetches the teammate
|
||||
// detail to read them.
|
||||
owner := records[0]
|
||||
assert.Equal(t, "owner@example.com", owner.Email)
|
||||
assert.Equal(t, "Olivia Owner", owner.FullName)
|
||||
assert.Empty(t, owner.FullName)
|
||||
assert.Equal(t, "Owner", owner.Role)
|
||||
assert.True(t, owner.IsAdmin)
|
||||
assert.Equal(t, "owner-user", owner.ExternalID)
|
||||
assert.Equal(t, "owner@example.com", owner.ExternalID)
|
||||
assert.Equal(t, coredata.AccessEntryAccountTypeUser, owner.AccountType)
|
||||
assert.Equal(t, coredata.MFAStatusEnabled, owner.MFAStatus)
|
||||
|
||||
admin := records[1]
|
||||
assert.Equal(t, "admin@example.com", admin.Email)
|
||||
assert.Equal(t, "Admin", admin.Role)
|
||||
assert.True(t, admin.IsAdmin)
|
||||
assert.Equal(t, "admin-user", admin.ExternalID)
|
||||
assert.Equal(t, coredata.MFAStatusEnabled, admin.MFAStatus)
|
||||
|
||||
teammate := records[2]
|
||||
assert.Equal(t, "teammate@example.com", teammate.Email)
|
||||
assert.Equal(t, "Teammate", teammate.Role)
|
||||
assert.False(t, teammate.IsAdmin)
|
||||
assert.Equal(t, "teammate-user", teammate.ExternalID)
|
||||
assert.Equal(t, coredata.MFAStatusDisabled, teammate.MFAStatus)
|
||||
// The owner is a full-access user whose scope catalog contains BOTH
|
||||
// 2fa_exempt and 2fa_required, so the MFA signal is ambiguous and the
|
||||
// driver reports Unknown rather than guessing from scope ordering.
|
||||
assert.Equal(t, coredata.MFAStatusUnknown, owner.MFAStatus)
|
||||
}
|
||||
|
||||
func TestSendGridRole(t *testing.T) {
|
||||
@@ -127,7 +119,8 @@ func TestSendGridMFAStatus(t *testing.T) {
|
||||
}{
|
||||
{name: "required", scopes: []string{"mail.send", "2fa_required"}, want: coredata.MFAStatusEnabled},
|
||||
{name: "exempt", scopes: []string{"mail.send", "2fa_exempt"}, want: coredata.MFAStatusDisabled},
|
||||
{name: "unknown", scopes: []string{"mail.send"}, want: coredata.MFAStatusUnknown},
|
||||
{name: "both is ambiguous", scopes: []string{"2fa_exempt", "2fa_required", "mail.send"}, want: coredata.MFAStatusUnknown},
|
||||
{name: "neither", scopes: []string{"mail.send"}, want: coredata.MFAStatusUnknown},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
163
pkg/accessreview/drivers/testdata/sendgrid.yaml
vendored
163
pkg/accessreview/drivers/testdata/sendgrid.yaml
vendored
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user