Soft-fail Microsoft 365 MFA when registration fetch fails

AuditLog.Read.All is not available on every tenant. Keep importing
accounts with MFA unknown and log the error instead of failing the
whole source fetch.

Signed-off-by: Sacha Al Himdani <sacha@probo.com>

pkg/accessreview/drivers/microsoft_365.go

@@ -23,6 +23,7 @@ package drivers
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -31,6 +32,7 @@ import (
 	"strings"
 	"time"
 
+	"go.gearno.de/kit/log"
 	"go.probo.inc/probo/pkg/coredata"
 )
 
@@ -39,6 +41,7 @@ import (
 // HTTP client (Bearer token).
 type Microsoft365Driver struct {
 	httpClient *http.Client
+	logger     *log.Logger
 }
 
 var _ Driver = (*Microsoft365Driver)(nil)
@@ -68,7 +71,7 @@ var adminRoleDisplayNames = map[string]bool{
 	"Authentication Administrator":            true,
 }
 
-func NewMicrosoft365Driver(httpClient *http.Client) *Microsoft365Driver {
+func NewMicrosoft365Driver(httpClient *http.Client, logger *log.Logger) *Microsoft365Driver {
 	return &Microsoft365Driver{
 		httpClient: &http.Client{
 			Transport: &retryRoundTripper{
@@ -76,6 +79,7 @@ func NewMicrosoft365Driver(httpClient *http.Client) *Microsoft365Driver {
 				maxRetries: 3,
 			},
 		},
+		logger: logger,
 	}
 }
 
@@ -157,9 +161,19 @@ func (d *Microsoft365Driver) ListAccounts(ctx context.Context) ([]AccountRecord,
 		return nil, fmt.Errorf("cannot list users: %w", err)
 	}
 
+	// MFA registration details require AuditLog.Read.All. A failure here
+	// must not abort the account fetch — leave MFA unknown so the campaign
+	// can still proceed. Context cancel/deadline still fail the fetch so a
+	// timed-out source does not commit an incomplete result as success.
 	mfaStatuses, err := d.listMFAStatuses(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("cannot list MFA statuses: %w", err)
+		if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+			return nil, fmt.Errorf("cannot list MFA statuses: %w", err)
+		}
+
+		d.logger.ErrorCtx(ctx, "cannot list microsoft 365 MFA statuses, reporting MFA unknown", log.Error(err))
+
+		mfaStatuses = map[string]coredata.MFAStatus{}
 	}
 
 	records := make([]AccountRecord, 0, len(users))

pkg/accessreview/drivers/microsoft_365_test.go

@@ -22,11 +22,15 @@ package drivers
 
 import (
 	"context"
+	"io"
+	"net/http"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.gearno.de/kit/log"
 	"go.probo.inc/probo/pkg/coredata"
 )
 
@@ -36,7 +40,7 @@ func TestMicrosoft365Driver(t *testing.T) {
 	rec := newRecorder(t, "testdata/microsoft_365", "MICROSOFT_365_TOKEN")
 	client := newVCRClient(rec, bearerAuth(os.Getenv("MICROSOFT_365_TOKEN")))
 
-	driver := NewMicrosoft365Driver(client)
+	driver := NewMicrosoft365Driver(client, log.NewLogger(log.WithName("test")))
 	records, err := driver.ListAccounts(context.Background())
 	require.NoError(t, err)
 	require.Len(t, records, 4)
@@ -83,3 +87,105 @@ func TestMicrosoft365Driver(t *testing.T) {
 	require.NotNil(t, dana.Active)
 	assert.False(t, *dana.Active)
 }
+
+func TestMicrosoft365Driver_MFAFetchFailureLeavesUnknown(t *testing.T) {
+	t.Parallel()
+
+	client := &http.Client{
+		Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
+			header := http.Header{"Content-Type": []string{"application/json"}}
+
+			switch {
+			case strings.HasSuffix(r.URL.Path, "/directoryRoles"):
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Header:     header,
+					Body:       io.NopCloser(strings.NewReader(`{"value":[]}`)),
+				}, nil
+			case strings.HasSuffix(r.URL.Path, "/users"):
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Header:     header,
+					Body: io.NopCloser(strings.NewReader(`{
+						"value":[{
+							"id":"user-1",
+							"userPrincipalName":"alice@example.com",
+							"mail":"alice@example.com",
+							"displayName":"Alice",
+							"accountEnabled":true
+						}]
+					}`)),
+				}, nil
+			case strings.Contains(r.URL.Path, "userRegistrationDetails"):
+				return &http.Response{
+					StatusCode: http.StatusForbidden,
+					Header:     header,
+					Body: io.NopCloser(strings.NewReader(`{
+						"error":{
+							"code":"Authentication_RequestFromNonPremiumTenantOrB2CTenant",
+							"message":"Tenant is not a B2C tenant and doesn't have premium license"
+						}
+					}`)),
+				}, nil
+			default:
+				return &http.Response{
+					StatusCode: http.StatusNotFound,
+					Header:     header,
+					Body:       io.NopCloser(strings.NewReader(`{}`)),
+				}, nil
+			}
+		}),
+	}
+
+	driver := NewMicrosoft365Driver(client, log.NewLogger(log.WithName("test")))
+	records, err := driver.ListAccounts(context.Background())
+	require.NoError(t, err)
+	require.Len(t, records, 1)
+	assert.Equal(t, "alice@example.com", records[0].Email)
+	assert.Equal(t, coredata.MFAStatusUnknown, records[0].MFAStatus)
+}
+
+func TestMicrosoft365Driver_MFAFetchCanceledPropagates(t *testing.T) {
+	t.Parallel()
+
+	client := &http.Client{
+		Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
+			header := http.Header{"Content-Type": []string{"application/json"}}
+
+			switch {
+			case strings.HasSuffix(r.URL.Path, "/directoryRoles"):
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Header:     header,
+					Body:       io.NopCloser(strings.NewReader(`{"value":[]}`)),
+				}, nil
+			case strings.HasSuffix(r.URL.Path, "/users"):
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Header:     header,
+					Body: io.NopCloser(strings.NewReader(`{
+						"value":[{
+							"id":"user-1",
+							"userPrincipalName":"alice@example.com",
+							"mail":"alice@example.com",
+							"displayName":"Alice",
+							"accountEnabled":true
+						}]
+					}`)),
+				}, nil
+			case strings.Contains(r.URL.Path, "userRegistrationDetails"):
+				return nil, context.Canceled
+			default:
+				return &http.Response{
+					StatusCode: http.StatusNotFound,
+					Header:     header,
+					Body:       io.NopCloser(strings.NewReader(`{}`)),
+				}, nil
+			}
+		}),
+	}
+
+	driver := NewMicrosoft365Driver(client, log.NewLogger(log.WithName("test")))
+	_, err := driver.ListAccounts(context.Background())
+	require.ErrorIs(t, err, context.Canceled)
+}

pkg/connector/provider/microsoft_365.go

@@ -48,8 +48,8 @@ func microsoft365Registration() *Registration {
 			"https://graph.microsoft.com/Directory.Read.All",
 			"https://graph.microsoft.com/RoleManagement.Read.Directory",
 		},
-		NewDriver: func(_ context.Context, c *http.Client, _ *coredata.Connector, _ *log.Logger) (drivers.Driver, error) {
-			return drivers.NewMicrosoft365Driver(c), nil
+		NewDriver: func(_ context.Context, c *http.Client, _ *coredata.Connector, logger *log.Logger) (drivers.Driver, error) {
+			return drivers.NewMicrosoft365Driver(c, logger.Named("microsoft365")), nil
 		},
 		NewNameResolver: func(_ context.Context, c *http.Client, _ *coredata.Connector, _ *log.Logger) drivers.NameResolver {
 			return drivers.NewMicrosoft365NameResolver(c)