Harden the Google Analytics account fetch

Three defects, all found reviewing the rebased branch.

The connection probe hit /v1alpha/accounts, which any analytics.readonly
grant can call, while the driver's first request is the account's
accessBindings — that additionally needs Administrator on the account
and the manage.users.readonly scope. An Editor connecting, or a user
declining the second scope on Google's granular consent screen, probed
green and then 403'd on every campaign fetch, leaving the source
permanently "Connected" with no rows. The probe now targets the same
accessBindings collection the driver reads.

A single unreadable property aborted the whole account. A property the
token cannot see, or one deleted between the list and the read, threw
away every binding already collected; 49 of 50 readable properties are
still worth reviewing, so 403 and 404 now skip that property. Anything
else still fails the fetch.

Fan-out errors named no resource: the account call, the property list
and each per-property call all returned the same "unexpected status"
string, so a 403 on one subproperty out of forty was unattributable.
Errors now carry the account or property ID.

The cassette gains a subproperty parented to another property (only
reachable through the ancestor filter, so it pins the hierarchy walk
that the filter change claimed) and a property returning 403.

Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>

pkg/accessreview/drivers/google_analytics.go

@@ -23,6 +23,7 @@ package drivers
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -101,7 +102,7 @@ func (d *GoogleAnalyticsDriver) ListAccounts(ctx context.Context) ([]AccountReco
 
 	// Account-level bindings.
 	if err := d.collectBindings(ctx, members, "v1alpha", "accounts", url.PathEscape(d.accountID), "accessBindings"); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("cannot list google analytics access bindings for account %q: %w", d.accountID, err)
 	}
 
 	// Property-level bindings, one loop per property beneath the account.
@@ -111,9 +112,21 @@ func (d *GoogleAnalyticsDriver) ListAccounts(ctx context.Context) ([]AccountReco
 	}
 
 	for _, propertyID := range propertyIDs {
-		if err := d.collectBindings(ctx, members, "v1alpha", "properties", url.PathEscape(propertyID), "accessBindings"); err != nil {
-			return nil, err
+		err := d.collectBindings(ctx, members, "v1alpha", "properties", url.PathEscape(propertyID), "accessBindings")
+		if err == nil {
+			continue
 		}
+
+		// A property the token cannot read, or one deleted between the list
+		// and the read, must not discard the bindings already collected: an
+		// account whose properties are 49/50 readable is still worth
+		// reviewing. Anything else invalidates the whole fetch.
+		if e, ok := errors.AsType[*googleAnalyticsStatusError](err); ok &&
+			(e.status == http.StatusForbidden || e.status == http.StatusNotFound) {
+			continue
+		}
+
+		return nil, fmt.Errorf("cannot list google analytics access bindings for property %q: %w", propertyID, err)
 	}
 
 	return googleAnalyticsRecords(members), nil
@@ -187,6 +200,17 @@ func (d *GoogleAnalyticsDriver) listProperties(ctx context.Context) ([]string, e
 	return nil, fmt.Errorf("cannot list all google analytics properties: %w", ErrPaginationLimitReached)
 }
 
+// googleAnalyticsStatusError carries the HTTP status of a failed Admin API call
+// so callers can tell a per-resource permission problem apart from a failure
+// that invalidates the whole fetch.
+type googleAnalyticsStatusError struct {
+	status int
+}
+
+func (e *googleAnalyticsStatusError) Error() string {
+	return fmt.Sprintf("unexpected status %d", e.status)
+}
+
 func (d *GoogleAnalyticsDriver) getJSON(ctx context.Context, endpoint string, out any) error {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
 	if err != nil {
@@ -205,7 +229,7 @@ func (d *GoogleAnalyticsDriver) getJSON(ctx context.Context, endpoint string, ou
 	}()
 
 	if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
-		return fmt.Errorf("cannot fetch google analytics resource: unexpected status %d", httpResp.StatusCode)
+		return &googleAnalyticsStatusError{status: httpResp.StatusCode}
 	}
 
 	if err := json.NewDecoder(httpResp.Body).Decode(out); err != nil {
@@ -217,6 +241,7 @@ func (d *GoogleAnalyticsDriver) getJSON(ctx context.Context, endpoint string, ou
 
 // googleAnalyticsURL builds a v1alpha Admin API URL from path segments, adding
 // the shared pageSize, an optional page token, and any extra query values.
+// Keys present in extra replace the default rather than adding to it.
 func googleAnalyticsURL(pageToken string, extra url.Values, segments ...string) (string, error) {
 	joined, err := url.JoinPath("https://"+googleAnalyticsAPIHost, segments...)
 	if err != nil {
@@ -232,6 +257,8 @@ func googleAnalyticsURL(pageToken string, extra url.Values, segments ...string)
 	q.Set("pageSize", strconv.Itoa(googleAnalyticsPageSize))
 
 	for k, vs := range extra {
+		q.Del(k)
+
 		for _, v := range vs {
 			q.Add(k, v)
 		}
@@ -246,6 +273,19 @@ func googleAnalyticsURL(pageToken string, extra url.Values, segments ...string)
 	return parsed.String(), nil
 }
 
+// GoogleAnalyticsAccountBindingsProbeURL builds a single-item account-level
+// accessBindings request for accountID. The connection probe uses it so the
+// check exercises the permission the driver actually needs — Administrator on
+// the account, granted through analytics.manage.users.readonly — instead of the
+// accounts list, which any analytics.readonly grant can call.
+func GoogleAnalyticsAccountBindingsProbeURL(accountID string) (string, error) {
+	return googleAnalyticsURL(
+		"",
+		url.Values{"pageSize": {"1"}},
+		"v1alpha", "accounts", url.PathEscape(accountID), "accessBindings",
+	)
+}
+
 // addGoogleAnalyticsBinding folds one access binding into the per-email member
 // map, deduplicating roles and setting the admin flag when the admin role is
 // present.