diff --git a/apps/onboarding/tests.py b/apps/onboarding/tests.py index 4f6a0305..0b4e3619 100644 --- a/apps/onboarding/tests.py +++ b/apps/onboarding/tests.py @@ -84,3 +84,70 @@ def test_oauth_callback_replays_verifier(self, client, workspace, connection_lin mock_provider.exchange_code.assert_called_once() _, kwargs = mock_provider.exchange_code.call_args assert kwargs["code_verifier"] == verifier + + +@pytest.mark.django_db +class TestConnectionLinkPageTokens: + """A Page must be driven by its own token, and a Page we had to skip must + not leave the client on a success page for an account never connected.""" + + def _callback(self, client, workspace, connection_link, pages): + nonce = "nonce-pages" + state = _sign_connection_link_state(workspace.id, "facebook", connection_link.token, nonce) + session = client.session + session[CONNECTION_LINK_OAUTH_SESSION_KEY] = { + "nonce": nonce, + "workspace_id": str(workspace.id), + "platform": "facebook", + "token": connection_link.token, + } + session.save() + + provider = MagicMock() + provider.exchange_code.return_value = OAuthTokens(access_token="USER-TOKEN", refresh_token="", expires_in=None) + provider.get_profile.return_value = AccountProfile(platform_id="me-1", name="Owner") + provider.get_user_pages.return_value = pages + + url = reverse("onboarding:oauth_callback", kwargs={"platform": "facebook"}) + with ( + patch("apps.onboarding.views._get_provider_for_platform", return_value=provider), + patch("apps.social_accounts.views.subscribe_account_webhooks_task"), + ): + response = client.get(url, {"code": "auth-code", "state": state}) + return response + + def test_a_page_is_connected_with_its_own_token(self, client, workspace, connection_link): + from apps.social_accounts.models import SocialAccount + + response = self._callback( + client, + workspace, + connection_link, + [{"id": "page-1", "name": "Page One", "access_token": "PAGE-TOKEN"}], + ) + + assert response.status_code == 302 + account = SocialAccount.objects.get(account_platform_id="page-1") + assert account.oauth_access_token == "PAGE-TOKEN" + assert "connection_link_error" not in client.session + + def test_a_page_without_its_own_token_is_reported_not_silently_dropped(self, client, workspace, connection_link): + from apps.social_accounts.models import SocialAccount + + response = self._callback( + client, + workspace, + connection_link, + [ + {"id": "page-1", "name": "Page One", "access_token": "PAGE-TOKEN"}, + {"id": "page-2", "name": "Page Two"}, + ], + ) + + assert response.status_code == 302 + assert SocialAccount.objects.filter(account_platform_id="page-1").exists() + # Never connected with the user token as a stand-in. + assert not SocialAccount.objects.filter(account_platform_id="page-2").exists() + error = client.session["connection_link_error"] + assert "Page Two" in error + assert "Page One" not in error diff --git a/apps/onboarding/views.py b/apps/onboarding/views.py index 91a75743..6fd29fb9 100644 --- a/apps/onboarding/views.py +++ b/apps/onboarding/views.py @@ -35,6 +35,7 @@ _get_configured_platforms, _normalize_mastodon_instance_url, _resolve_mastodon_extra_creds, + resolve_page_account_token, ) from .models import ConnectionLink, ConnectionLinkUsage, OnboardingChecklist @@ -424,7 +425,23 @@ def connection_oauth_callback(request, platform): if pages: from providers.types import AccountProfile + skipped: list[str] = [] + for page in pages: + access_token = resolve_page_account_token(page, platform, tokens.access_token) + if not access_token: + # Silently dropping these would leave the client on a + # success page for accounts that were never connected. + name = page.get("name") or page["id"] + skipped.append(name) + logger.warning( + "Connection link %s: %s provided no account token for %s; skipping.", + link.id, + platform, + name, + ) + continue + page_profile = AccountProfile( platform_id=page["id"], name=page["name"], @@ -436,7 +453,7 @@ def connection_oauth_callback(request, platform): workspace_id=workspace_id, platform=platform, profile=page_profile, - access_token=page.get("access_token", tokens.access_token), + access_token=access_token, refresh_token=tokens.refresh_token, expires_in=tokens.expires_in, # Instagram-via-Facebook receives its webhooks through @@ -447,6 +464,13 @@ def connection_oauth_callback(request, platform): connection_link=link, social_account=account, ) + + if skipped: + names = ", ".join(skipped) + request.session["connection_link_error"] = ( + f"Could not connect {names}: the platform did not provide an account token. " + "Check that you granted access to those accounts, then try again." + ) return redirect("onboarding:connection_page", token=token) # Standard single-account flow diff --git a/apps/social_accounts/management/commands/diagnose_facebook.py b/apps/social_accounts/management/commands/diagnose_facebook.py index c526f1eb..8658ba77 100644 --- a/apps/social_accounts/management/commands/diagnose_facebook.py +++ b/apps/social_accounts/management/commands/diagnose_facebook.py @@ -97,13 +97,13 @@ def check(name, ok, detail): self._check_local_state(account, check) try: - from apps.social_accounts.views import _apply_analytics_scope_flag, _get_provider_for_platform + from apps.social_accounts.provider_factory import _get_provider_for_platform, apply_analytics_scope_flag provider = _get_provider_for_platform("facebook", account.workspace.organization_id) # Mirror what the OAuth flow does, or required_scopes reports # read_insights as expected on a deployment where analytics is # deliberately off — and a healthy token gets reported as broken. - _apply_analytics_scope_flag(provider, "facebook") + apply_analytics_scope_flag(provider, "facebook") except Exception as exc: check("provider", False, f"could not build the Facebook provider: {exc}") return report diff --git a/apps/social_accounts/migrations/0018_socialaccount_missing_scopes.py b/apps/social_accounts/migrations/0018_socialaccount_missing_scopes.py new file mode 100644 index 00000000..210a2df6 --- /dev/null +++ b/apps/social_accounts/migrations/0018_socialaccount_missing_scopes.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.15 on 2026-09-10 12:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('social_accounts', '0017_seed_missing_analytics_platform_config'), + ] + + operations = [ + migrations.AddField( + model_name='socialaccount', + name='missing_scopes', + field=models.JSONField(blank=True, default=list), + ), + ] diff --git a/apps/social_accounts/models.py b/apps/social_accounts/models.py index 64b4b428..b19f0fc4 100644 --- a/apps/social_accounts/models.py +++ b/apps/social_accounts/models.py @@ -70,6 +70,14 @@ class ConnectionStatus(models.TextChoices): # a reconnect, and by the user pressing "Try again". webhook_retry_count = models.PositiveSmallIntegerField(default=0) + # Scopes we asked for that the grant came back without. Meta drops + # unapproved or declined permissions silently rather than failing the + # grant, so without this the account looks healthy and only breaks later at + # publish or insights time with an opaque platform error. Empty means + # "everything we asked for was granted", or that the platform gives us no + # way to ask. + missing_scopes = models.JSONField(default=list, blank=True) + # Connection health connection_status = models.CharField( max_length=20, @@ -247,6 +255,21 @@ def field_config(self) -> dict: """Return field configuration for this platform.""" return {**self.PLATFORM_FIELD_DEFAULTS, **self.PLATFORM_FIELD_CONFIG.get(self.platform, {})} + @property + def keeps_platform_grant_on_disconnect(self) -> bool: + """True when disconnecting here cannot revoke the platform's grant. + + The Facebook-Page flows share one grant across every Page and Instagram + account that person connected, so the only endpoint that would revoke + it takes all of them down at once — see + ``FacebookProvider.revoke_token``. Instagram Login is excluded: its + token belongs to the one account, so disconnect does revoke it. + """ + return self.platform in { + PlatformCredential.Platform.FACEBOOK, + PlatformCredential.Platform.INSTAGRAM, + } + def supports_first_comment(self) -> bool: """Whether this account can have a first comment posted by the worker. diff --git a/apps/social_accounts/provider_factory.py b/apps/social_accounts/provider_factory.py index dd22d9a5..9148099d 100644 --- a/apps/social_accounts/provider_factory.py +++ b/apps/social_accounts/provider_factory.py @@ -19,3 +19,18 @@ def _get_provider_for_platform(platform: str, org_id, **extra_credentials): credentials = {**credentials, **extra_credentials} return get_provider(platform, credentials) + + +def apply_analytics_scope_flag(provider, platform: str) -> None: + """Set ``provider.include_analytics_scopes`` from AnalyticsPlatformConfig. + + Providers add their analytics-only scopes (e.g. ``read_insights``) to the + OAuth scope list only when this flag is True, so a self-hoster whose Meta or + Google app has not been approved for them can still connect for publishing. + + Anything reasoning about *what we asked for* must apply this first — the + unconditional ``required_scopes`` is not what OAuth requested. + """ + from apps.social_accounts.models import AnalyticsPlatformConfig + + provider.include_analytics_scopes = platform in AnalyticsPlatformConfig.enabled_platforms() diff --git a/apps/social_accounts/tests/test_diagnose_facebook.py b/apps/social_accounts/tests/test_diagnose_facebook.py index 7c8df684..a7068006 100644 --- a/apps/social_accounts/tests/test_diagnose_facebook.py +++ b/apps/social_accounts/tests/test_diagnose_facebook.py @@ -49,7 +49,7 @@ def _provider(*, scopes, subscribed_fields, app_id="app-1"): def _run(account): out = StringIO() - with patch("apps.social_accounts.views._get_provider_for_platform") as factory: + with patch("apps.social_accounts.provider_factory._get_provider_for_platform") as factory: factory.return_value = account["provider"] # Non-zero exit is the documented signal that a check failed; the # report is still what we assert on. diff --git a/apps/social_accounts/tests/test_webhook_subscription.py b/apps/social_accounts/tests/test_webhook_subscription.py index 5d51ee26..90866a4a 100644 --- a/apps/social_accounts/tests/test_webhook_subscription.py +++ b/apps/social_accounts/tests/test_webhook_subscription.py @@ -624,3 +624,172 @@ def test_reconnecting_refunds_the_automatic_retry_budget(workspace): ) assert account.webhook_retry_count == 0 + + +# --------------------------------------------------------------- token scoping + + +def test_a_page_must_use_its_own_token(): + from apps.social_accounts.views import resolve_page_account_token + + page = {"id": "page-1", "access_token": "PAGE-TOKEN"} + assert resolve_page_account_token(page, "facebook", "USER-TOKEN") == "PAGE-TOKEN" + + +def test_a_page_without_its_own_token_is_not_connectable(): + """Substituting the user token publishes under the wrong identity, and made + the account eligible for a revoke that severs its siblings.""" + from apps.social_accounts.views import resolve_page_account_token + + assert resolve_page_account_token({"id": "page-1"}, "facebook", "USER-TOKEN") == "" + + +def test_instagram_via_facebook_may_fall_back_to_the_user_token(): + """Its calls address the IG user, so the user token is the right credential.""" + from apps.social_accounts.views import resolve_page_account_token + + assert resolve_page_account_token({"id": "ig-1"}, "instagram", "USER-TOKEN") == "USER-TOKEN" + + +def test_both_connect_flows_share_one_resolver(): + """select_account and the connection-link flow diverging is what let a user + token reach a Page in the first place.""" + import inspect + + from apps.onboarding import views as onboarding_views + from apps.social_accounts import views as accounts_views + + for module in (onboarding_views, accounts_views): + assert "resolve_page_account_token(" in inspect.getsource(module) + + +def test_disconnect_never_revokes_the_whole_facebook_user(): + """DELETE /me/permissions revokes the app for the entire person, so one + workspace disconnecting one Page would take down every other account.""" + from unittest.mock import MagicMock as _Mock + + from providers.facebook import FacebookProvider + from providers.instagram import InstagramProvider + + for cls in (FacebookProvider, InstagramProvider): + provider = cls({"client_id": "id", "client_secret": "secret"}) + provider._request = _Mock() + + assert provider.revoke_token("any-token") is False + provider._request.assert_not_called() + + +def test_only_the_page_flows_warn_that_disconnect_keeps_the_grant(workspace): + """Instagram Login's token belongs to the one account, so it really does + revoke on disconnect and must not carry the warning.""" + for platform, expected in ( + ("facebook", True), + ("instagram", True), + ("instagram_login", False), + ("bluesky", False), + ): + account = SocialAccount(workspace=workspace, platform=platform, account_platform_id="x", account_name="x") + assert account.keeps_platform_grant_on_disconnect is expected, platform + + +# ------------------------------------------------------------- missing scopes + + +def test_a_partial_grant_is_recorded_on_the_account(workspace): + """The whole Meta saga started with a scope silently absent from a grant.""" + from apps.social_accounts.webhooks import record_missing_scopes + + account = _account(workspace) + provider = MagicMock() + provider.required_scopes = ["pages_show_list", "pages_manage_posts", "read_insights"] + provider.get_granted_scopes.return_value = {"pages_show_list", "pages_manage_posts"} + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == ["read_insights"] + + account.refresh_from_db() + assert account.missing_scopes == ["read_insights"] + + +def test_a_complete_grant_records_nothing(workspace): + from apps.social_accounts.webhooks import record_missing_scopes + + account = _account(workspace, missing_scopes=["read_insights"]) + provider = MagicMock() + provider.required_scopes = ["pages_show_list"] + provider.get_granted_scopes.return_value = {"pages_show_list", "extra_scope"} + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == [] + + account.refresh_from_db() + # A fresh, complete grant must clear a stale warning. + assert account.missing_scopes == [] + + +def test_an_unanswerable_platform_leaves_the_field_alone(workspace): + """None means unknown. Treating it as "nothing granted" would flag every + scope on every platform that cannot be asked.""" + from apps.social_accounts.webhooks import record_missing_scopes + + account = _account(workspace, missing_scopes=["previously_noted"]) + provider = MagicMock() + provider.required_scopes = ["pages_show_list"] + provider.get_granted_scopes.return_value = None + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == [] + + account.refresh_from_db() + assert account.missing_scopes == ["previously_noted"] + + +def test_a_readback_failure_does_not_break_the_connect_flow(workspace): + from apps.social_accounts.webhooks import record_missing_scopes + + account = _account(workspace) + provider = MagicMock() + provider.get_granted_scopes.side_effect = RuntimeError("boom") + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == [] + + +def test_scopes_omitted_by_design_are_not_reported_missing(workspace): + """Connect drops the analytics-only scopes when analytics is off for the + platform, so comparing against the unconditional list would demand a + reconnect for something we deliberately never asked for.""" + from apps.social_accounts.models import AnalyticsPlatformConfig + from apps.social_accounts.webhooks import record_missing_scopes + + AnalyticsPlatformConfig.objects.update_or_create(platform="facebook", defaults={"is_enabled": False}) + account = _account(workspace, platform="facebook") + + from providers.facebook import FacebookProvider + + provider = FacebookProvider({"client_id": "id", "client_secret": "secret"}) + provider.get_granted_scopes = MagicMock(return_value=set(provider.required_scopes) - {"read_insights"}) + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == [] + + account.refresh_from_db() + assert account.missing_scopes == [] + + +def test_a_scope_missing_while_analytics_is_on_is_still_reported(workspace): + from apps.social_accounts.models import AnalyticsPlatformConfig + from apps.social_accounts.webhooks import record_missing_scopes + + AnalyticsPlatformConfig.objects.update_or_create(platform="facebook", defaults={"is_enabled": True}) + account = _account(workspace, platform="facebook") + + from providers.facebook import FacebookProvider + + provider = FacebookProvider({"client_id": "id", "client_secret": "secret"}) + provider.include_analytics_scopes = True + granted = set(provider.required_scopes) - {"read_insights"} + provider.get_granted_scopes = MagicMock(return_value=granted) + + with patch("apps.social_accounts.webhooks._get_provider_for_platform", return_value=provider): + assert record_missing_scopes(account) == ["read_insights"] diff --git a/apps/social_accounts/views.py b/apps/social_accounts/views.py index d589820e..67f09090 100644 --- a/apps/social_accounts/views.py +++ b/apps/social_accounts/views.py @@ -26,7 +26,7 @@ from .models import MastodonAppRegistration, PlatformVisibility, SocialAccount from .oauth_aliases import from_url_slug, redirect_uri_from_request, to_url_slug from .oauth_pkce import issue_pkce_verifier, pkce_kwargs -from .provider_factory import _get_provider_for_platform +from .provider_factory import _get_provider_for_platform, apply_analytics_scope_flag from .webhooks import ( subscribe_account_webhooks, subscribe_account_webhooks_task, @@ -47,26 +47,6 @@ def _get_visible_platform_choices(): return PlatformVisibility.visible_choices() -def _apply_analytics_scope_flag(provider, platform): - """Set ``provider.include_analytics_scopes`` based on AnalyticsPlatformConfig. - - Providers add their analytics-only scopes (e.g. ``read_insights``, - ``yt-analytics.readonly``) to the OAuth scope list only when this flag is - True. If the platform is disabled in ``AnalyticsPlatformConfig`` (analytics - not yet rolled out for it), we omit those scopes so a self-hoster whose - Facebook / TikTok / Google app hasn't been approved for them can still - connect accounts for publishing. - - A no-op for ``instagram`` and ``instagram_login``, which both request their - insights scope unconditionally — see ``SocialProvider.analytics_only_scopes`` - for why deferring it there did more harm than good. - """ - from apps.social_accounts.models import AnalyticsPlatformConfig - - enabled = AnalyticsPlatformConfig.enabled_platforms() - provider.include_analytics_scopes = platform in enabled - - def _get_configured_platforms(org_id): """Return set of platform names that have credentials configured.""" from providers import PROVIDER_REGISTRY @@ -261,7 +241,7 @@ def connect_platform(request, workspace_id): # Standard OAuth flow provider = _get_provider_for_platform(platform, request.org.id) - _apply_analytics_scope_flag(provider, platform) + apply_analytics_scope_flag(provider, platform) nonce = secrets.token_urlsafe(32) state = _sign_state(workspace_id, platform, request.user.id, nonce) @@ -480,9 +460,7 @@ def select_account(request): for page in page_data["pages"]: if page["id"] in selected_ids: - access_token = page.get("access_token") - if not access_token and platform == "instagram": - access_token = user_tokens["access_token"] + access_token = resolve_page_account_token(page, platform, user_tokens.get("access_token", "")) if not access_token: messages.error( request, @@ -740,7 +718,7 @@ def reconnect(request, workspace_id, account_id): # Standard OAuth reconnect provider = _get_provider_for_platform(platform, request.org.id) - _apply_analytics_scope_flag(provider, platform) + apply_analytics_scope_flag(provider, platform) nonce = secrets.token_urlsafe(32) state = _sign_state(workspace_id, platform, request.user.id, nonce) code_verifier = issue_pkce_verifier(provider) @@ -875,6 +853,26 @@ def disconnect(request, workspace_id, account_id): # ------------------------------------------------------------------ +def resolve_page_account_token(page: dict, platform: str, user_access_token: str) -> str: + """Pick the token a Page-backed account must be driven by. + + A Facebook Page needs its *own* Page token: a user token publishes under + the wrong identity, and — because the only revoke endpoint that accepts it + revokes the app for the whole person — makes a per-account disconnect able + to sever every other connection they have. + + Instagram-via-Facebook is the one exception: its calls address the IG user, + so the user token is the correct credential when the Page dict carries none. + + Returns "" when no usable token exists, which callers must treat as "cannot + connect this account" rather than substituting one. + """ + token = page.get("access_token") + if not token and platform == PlatformCredential.Platform.INSTAGRAM: + token = user_access_token + return token or "" + + def _create_or_update_account( *, workspace_id, diff --git a/apps/social_accounts/webhooks.py b/apps/social_accounts/webhooks.py index ebf55d57..34e93767 100644 --- a/apps/social_accounts/webhooks.py +++ b/apps/social_accounts/webhooks.py @@ -18,7 +18,7 @@ from .error_messages import WEBHOOK_REJECTED_MESSAGE, WEBHOOK_UNAVAILABLE_MESSAGE, classify_webhook_failure from .models import SocialAccount -from .provider_factory import _get_provider_for_platform +from .provider_factory import _get_provider_for_platform, apply_analytics_scope_flag logger = logging.getLogger(__name__) @@ -203,6 +203,7 @@ def subscribe_account_webhooks_task(account_id): except SocialAccount.DoesNotExist: logger.info("Account %s gone before webhooks could be subscribed.", account_id) return + record_missing_scopes(account) subscribe_account_webhooks(account) @@ -244,3 +245,38 @@ def unsubscribe_account_webhooks(account) -> bool: account.platform, ) return False + + +def record_missing_scopes(account) -> list[str]: + """Note which requested scopes the grant came back without. + + Runs on the same post-connect task as the webhook subscription so the + OAuth redirect stays free of blocking round trips. Best-effort: a platform + that cannot answer leaves the field untouched rather than claiming nothing + was granted. + """ + try: + provider = _get_provider_for_platform(account.platform, account.workspace.organization_id) + # Match what OAuth actually asked for. Connect omits the analytics-only + # scopes when the platform's analytics is switched off, so comparing + # against the unconditional list would report a scope as missing that we + # deliberately never requested — and send the user to reconnect for it. + apply_analytics_scope_flag(provider, account.platform) + granted = provider.get_granted_scopes(account.oauth_access_token) + except Exception: + logger.exception("Could not read granted scopes for %s (%s)", account.id, account.platform) + return [] + + if granted is None: + return [] + + missing = sorted(set(provider.required_scopes) - granted) + SocialAccount.objects.filter(pk=account.pk).update(missing_scopes=missing) + if missing: + logger.warning( + "Account %s (%s) connected without %s; those features will fail.", + account.id, + account.platform, + ", ".join(missing), + ) + return missing diff --git a/providers/base.py b/providers/base.py index 9b068a0f..dd1d9a12 100644 --- a/providers/base.py +++ b/providers/base.py @@ -262,6 +262,20 @@ def debug_token(self, access_token: str) -> dict: # Token management # ------------------------------------------------------------------ + def get_granted_scopes(self, access_token: str) -> set[str] | None: + """Which of the requested scopes the platform actually granted. + + Meta silently drops permissions it has not approved, or that the user + declined, rather than failing the grant — so a connection reports + healthy and only breaks later, at publish or insights time, with an + opaque platform error. Asking up front turns that into something we can + name while the user is still looking at the screen. + + Returns ``None`` when the platform offers no way to ask, which callers + must treat as "unknown", never as "nothing granted". + """ + return None + def revoke_token(self, access_token: str) -> bool: """Revoke an OAuth token. Returns True if successful.""" return False diff --git a/providers/facebook.py b/providers/facebook.py index b9cc8350..7364ac98 100644 --- a/providers/facebook.py +++ b/providers/facebook.py @@ -7,10 +7,11 @@ from urllib.parse import urlencode, urlparse from .base import SocialProvider -from .exceptions import APIError, OAuthError, PublishError +from .exceptions import APIError, OAuthError, ProviderError, PublishError from .meta_comments import parse_graph_time from .meta_insights import fetch_insights_safe, parse_insights_response from .meta_messaging import build_send_payload, resolve_recipient_id +from .meta_oauth import facebook_login_params from .types import ( AccountMetrics, AccountProfile, @@ -161,13 +162,12 @@ def rate_limits(self) -> RateLimitConfig: # ------------------------------------------------------------------ def get_auth_url(self, redirect_uri: str, state: str, code_verifier: str | None = None) -> str: - params = { - "client_id": self.credentials["client_id"], - "redirect_uri": redirect_uri, - "state": state, - "scope": ",".join(self.required_scopes), - "response_type": "code", - } + params = facebook_login_params( + client_id=self.credentials["client_id"], + redirect_uri=redirect_uri, + state=state, + scopes=self.required_scopes, + ) return f"{OAUTH_URL}?{urlencode(params)}" def exchange_code(self, code: str, redirect_uri: str, code_verifier: str | None = None) -> OAuthTokens: @@ -1081,14 +1081,56 @@ def debug_token(self, access_token: str) -> dict: ) return resp.json().get("data", {}) - def revoke_token(self, access_token: str) -> bool: + def get_granted_scopes(self, access_token: str) -> set[str] | None: + """Read the grant back by inspecting the token itself. + + Not ``/me/permissions``: these accounts hold a *Page* token, and ``/me`` + resolves to whatever the token identifies — the Page, which has no + ``permissions`` edge. ``/debug_token`` reports the scopes carried by any + token, Page ones included, and is the documented way to ask. + + Needs an app access token to make the call, so a provider built without + app credentials reports "unknown" rather than guessing. + """ + client_id = self.credentials.get("client_id") + client_secret = self.credentials.get("client_secret") + if not client_id or not client_secret: + return None + try: - self._request( - "DELETE", - f"{BASE_URL}/me/permissions", - access_token=access_token, + resp = self._request( + "GET", + f"{BASE_URL}/debug_token", + params={ + "input_token": access_token, + # Meta's documented app-token form. Never log this. + "access_token": f"{client_id}|{client_secret}", + }, ) - return True - except APIError: - logger.warning("Failed to revoke Facebook token") - return False + except ProviderError: + logger.warning("Could not read granted permissions for %s", self.platform_name) + return None + + data = resp.json().get("data") or {} + scopes = data.get("scopes") + if scopes is None: + # A token Meta declines to describe is unknown, not unscoped. + return None + return set(scopes) + + def revoke_token(self, access_token: str) -> bool: + """Intentionally does nothing. Disconnecting cannot revoke the grant. + + The only endpoint that would revoke it, ``DELETE /me/permissions``, + revokes this app for the *whole* Facebook user. One workspace + disconnecting one Page would sever every other Page and Instagram + account that person connected anywhere else, so it must never run from + a per-account action. + + Disconnect therefore drops our stored token, removes the webhook + subscription, and deletes posts that targeted only this account. + Removing the app's access outright is the user's own action in Facebook + → Settings → Apps and Websites — which is also what makes the next + connect show the full permission dialog instead of "continue sharing?". + """ + return False diff --git a/providers/instagram.py b/providers/instagram.py index 5f972aba..033d0259 100644 --- a/providers/instagram.py +++ b/providers/instagram.py @@ -12,13 +12,14 @@ from urllib.parse import urlencode from .base import SocialProvider -from .exceptions import APIError, OAuthError, PublishError +from .exceptions import APIError, OAuthError, ProviderError, PublishError from .meta_comments import ( fetch_instagram_comments, find_own_instagram_comment, resolve_comment_reply_target, ) from .meta_insights import fetch_insights_safe +from .meta_oauth import facebook_login_params from .types import ( AccountMetrics, AccountProfile, @@ -151,13 +152,12 @@ def rate_limits(self) -> RateLimitConfig: # ------------------------------------------------------------------ def get_auth_url(self, redirect_uri: str, state: str, code_verifier: str | None = None) -> str: - params = { - "client_id": self.credentials["client_id"], - "redirect_uri": redirect_uri, - "state": state, - "scope": ",".join(self.required_scopes), - "response_type": "code", - } + params = facebook_login_params( + client_id=self.credentials["client_id"], + redirect_uri=redirect_uri, + state=state, + scopes=self.required_scopes, + ) return f"{OAUTH_URL}?{urlencode(params)}" def exchange_code(self, code: str, redirect_uri: str, code_verifier: str | None = None) -> OAuthTokens: @@ -595,6 +595,53 @@ def reply_to_comment(self, access_token: str, comment_id: str, text: str, extra: data = resp.json() return ReplyResult(platform_message_id=data.get("id", ""), extra=data) + def get_granted_scopes(self, access_token: str) -> set[str] | None: + """Read the grant back by inspecting the token itself. + + Not ``/me/permissions``: these accounts hold a *Page* token, and ``/me`` + resolves to whatever the token identifies — the Page, which has no + ``permissions`` edge. ``/debug_token`` reports the scopes carried by any + token, Page ones included, and is the documented way to ask. + + Needs an app access token to make the call, so a provider built without + app credentials reports "unknown" rather than guessing. + """ + client_id = self.credentials.get("client_id") + client_secret = self.credentials.get("client_secret") + if not client_id or not client_secret: + return None + + try: + resp = self._request( + "GET", + f"{BASE_URL}/debug_token", + params={ + "input_token": access_token, + # Meta's documented app-token form. Never log this. + "access_token": f"{client_id}|{client_secret}", + }, + ) + except ProviderError: + logger.warning("Could not read granted permissions for %s", self.platform_name) + return None + + data = resp.json().get("data") or {} + scopes = data.get("scopes") + if scopes is None: + # A token Meta declines to describe is unknown, not unscoped. + return None + return set(scopes) + + def revoke_token(self, access_token: str) -> bool: + """Deliberately a no-op — see ``FacebookProvider.revoke_token``. + + This connection shares the Facebook user's grant, so revoking it would + also sever every sibling connection. Left explicit rather than + inherited so the next reader does not "fix" it by adding + DELETE /me/permissions. + """ + return False + # ------------------------------------------------------------------ # Webhooks # ------------------------------------------------------------------ diff --git a/providers/instagram_login.py b/providers/instagram_login.py index 26ae9e66..1273a12f 100644 --- a/providers/instagram_login.py +++ b/providers/instagram_login.py @@ -19,7 +19,7 @@ from urllib.parse import urlencode from .base import SocialProvider -from .exceptions import APIError, OAuthError, PublishError +from .exceptions import APIError, OAuthError, ProviderError, PublishError from .meta_comments import ( fetch_instagram_comments, find_own_instagram_comment, @@ -706,6 +706,13 @@ def _get_media_fields(self, access_token: str, media_id: str) -> dict: # ------------------------------------------------------------------ def revoke_token(self, access_token: str) -> bool: + """Revoke this app's grant on the connected Instagram account. + + Safe to do per-account here, unlike the Facebook-Page providers: this + flow's token belongs to the Instagram account itself, so revoking it + severs nothing else. It also restores the full permission dialog on the + next connect, rather than the abbreviated "continue sharing?" prompt. + """ try: self._request( "DELETE", @@ -713,6 +720,9 @@ def revoke_token(self, access_token: str) -> bool: access_token=access_token, ) return True - except APIError: + except ProviderError: + # Not just APIError: a 429 raises RateLimitError and an expired + # grant raises TokenExpiredError, neither of which subclasses it. + # Disconnect must proceed regardless of why revocation failed. logger.warning("Failed to revoke Instagram token") return False diff --git a/providers/meta_oauth.py b/providers/meta_oauth.py new file mode 100644 index 00000000..0011fc8f --- /dev/null +++ b/providers/meta_oauth.py @@ -0,0 +1,31 @@ +"""Shared Login-dialog parameters for the Facebook-hosted Meta providers. + +``FacebookProvider`` and ``InstagramProvider`` both drive the same dialog at +facebook.com; ``InstagramLoginProvider`` drives Instagram's own, which has a +different vocabulary and deliberately does not use any of this. +""" + +from __future__ import annotations + +# Lets us re-ask for a permission the user previously *declined*. Without it +# Meta skips the dialog entirely on a reconnect, so a declined scope could +# never be requested again and the connection silently lacked an ability it +# advertises. +# +# It does NOT force the full permission list for someone who already granted +# everything — they still see the short "continue sharing?" confirmation. Only +# the user removing the app (Facebook → Settings → Apps and Websites) restores +# the first-time dialog. +FACEBOOK_LOGIN_EXTRA_PARAMS: dict[str, str] = {"auth_type": "rerequest"} + + +def facebook_login_params(*, client_id: str, redirect_uri: str, state: str, scopes: list[str]) -> dict[str, str]: + """Build the query parameters for the facebook.com Login dialog.""" + return { + "client_id": client_id, + "redirect_uri": redirect_uri, + "state": state, + "scope": ",".join(scopes), + "response_type": "code", + **FACEBOOK_LOGIN_EXTRA_PARAMS, + } diff --git a/templates/calendar/partials/publish_sent.html b/templates/calendar/partials/publish_sent.html index dc782541..661340d3 100644 --- a/templates/calendar/partials/publish_sent.html +++ b/templates/calendar/partials/publish_sent.html @@ -65,8 +65,10 @@ {% if pp.status == "failed" and pp.publish_error %}
{{ pp.publish_error }}
{% endif %} - {# Published, but its first comment never landed — amber, not red: - the post itself went out fine. #} + {% comment %} + Published, but its first comment never landed — amber, not red: + the post itself went out fine. + {% endcomment %} {% if pp.first_comment_status == "failed" %}First comment failed
{% endif %} diff --git a/templates/social_accounts/partials/_account_card.html b/templates/social_accounts/partials/_account_card.html index c1d15b1f..1bb9c166 100644 --- a/templates/social_accounts/partials/_account_card.html +++ b/templates/social_accounts/partials/_account_card.html @@ -54,7 +54,20 @@Disconnect {{ account.account_name|default:account.account_handle }}? Historical data will be preserved.
+Disconnect {{ account.account_name|default:account.account_handle }}? Historical data will be preserved.
+ {% comment %} + Disconnecting drops our copy of the credentials; it cannot + revoke the grant, because the only endpoint that would do so + revokes this app for the whole person and would sever their + other connected accounts. Say so rather than let "Disconnect" + imply a revocation it never performs. + {% endcomment %} + {% if account.keeps_platform_grant_on_disconnect %} ++ This removes the account from BrightBean only. To revoke our access entirely, + remove BrightBean in Facebook › Settings › Apps and Websites. +
+ {% endif %}+ Some features are unavailable. + This account was connected without {{ account.missing_scopes|join:", " }}. + Anything relying on them will fail until the permission is granted — reconnect to try again. +
+{{ account.last_error }}
diff --git a/tests/providers/test_facebook.py b/tests/providers/test_facebook.py index 57f2d088..9e6c7cdd 100644 --- a/tests/providers/test_facebook.py +++ b/tests/providers/test_facebook.py @@ -1,4 +1,4 @@ -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from unittest.mock import MagicMock, call import httpx @@ -890,9 +890,13 @@ def test_fetch_post_comments_uses_field_expansion_and_does_not_pass_caller_since """`since` on /feed filters by POST time, so passing the caller's `since` would hide every new comment on an older post.""" provider = FacebookProvider({"client_id": "id", "client_secret": "secret", "page_id": "page-1"}) - provider._request = MagicMock(return_value=_feed_response([_comment()])) + # Both dates relative to now. Fixed ones silently invert this test once they + # drift past FACEBOOK_FEED_WINDOW_DAYS: the 30-day window floor then lands + # *after* the caller's `since`, and the comment falls outside the lookback. + recent = (datetime.now(UTC) - timedelta(days=1)).strftime("%Y-%m-%dT%H:%M:%S+0000") + provider._request = MagicMock(return_value=_feed_response([_comment(created=recent)])) - since = datetime(2026, 8, 7, 8, 0, tzinfo=UTC) + since = datetime.now(UTC) - timedelta(days=2) messages = provider._fetch_post_comments("page-token", since=since) assert [m.platform_message_id for m in messages] == ["comment-1"] diff --git a/tests/providers/test_meta_replies.py b/tests/providers/test_meta_replies.py index 1112bdc3..3f2c61ad 100644 --- a/tests/providers/test_meta_replies.py +++ b/tests/providers/test_meta_replies.py @@ -6,6 +6,7 @@ """ from unittest.mock import MagicMock +from urllib.parse import parse_qs, urlparse import pytest @@ -463,3 +464,87 @@ def test_instagram_login_without_its_own_id_keeps_every_message(): provider._fetch_media_comments = MagicMock(return_value=[]) assert len(provider.get_messages("token")) == 1 + + +# --------------------------------------------------- re-requesting permissions + + +def test_facebook_auth_url_rerequests_declined_permissions(): + """Without auth_type=rerequest Meta skips the dialog on a reconnect. + + A user who declined a scope could then never be asked again, and the + connection silently lacks the ability it advertises. + """ + provider = FacebookProvider(CREDS) + url = provider.get_auth_url("https://studio.example/cb", "state-1") + + assert parse_qs(urlparse(url).query)["auth_type"] == ["rerequest"] + + +def test_instagram_auth_url_rerequests_declined_permissions(): + provider = InstagramProvider(CREDS) + url = provider.get_auth_url("https://studio.example/cb", "state-1") + + assert parse_qs(urlparse(url).query)["auth_type"] == ["rerequest"] + + +def test_instagram_login_auth_url_carries_no_facebook_only_params(): + """auth_type belongs to the facebook.com dialog, not Instagram's own. + + Sending parameters the Instagram-hosted dialog does not document risks it + rejecting the request outright, so the shared helper must not reach here. + """ + provider = InstagramLoginProvider(CREDS) + query = parse_qs(urlparse(provider.get_auth_url("https://studio.example/cb", "state-1")).query) + + assert "auth_type" not in query + + +# ------------------------------------------------------ granted-scope readback + + +def test_granted_scopes_inspect_the_token_not_me_permissions(): + """These accounts hold a *Page* token, so /me resolves to the Page, which + has no permissions edge. debug_token reports any token's scopes.""" + provider = FacebookProvider(CREDS) + provider._request = MagicMock( + return_value=_resp({"data": {"is_valid": True, "scopes": ["pages_show_list", "pages_manage_posts"]}}) + ) + + assert provider.get_granted_scopes("page-token") == {"pages_show_list", "pages_manage_posts"} + provider._request.assert_called_once_with( + "GET", + "https://graph.facebook.com/v25.0/debug_token", + params={"input_token": "page-token", "access_token": "id|secret"}, + ) + + +def test_granted_scopes_need_app_credentials_to_ask(): + """debug_token requires an app token; without one the answer is unknown.""" + provider = FacebookProvider({"client_id": "id"}) + + assert provider.get_granted_scopes("token") is None + + +def test_a_token_meta_will_not_describe_is_unknown_not_unscoped(): + """An empty set would report every scope as missing and demand a reconnect.""" + provider = FacebookProvider(CREDS) + provider._request = MagicMock(return_value=_resp({"data": {"is_valid": False}})) + + assert provider.get_granted_scopes("token") is None + + +def test_a_failed_readback_is_unknown_not_empty(): + from providers.exceptions import RateLimitError + + provider = FacebookProvider(CREDS) + provider._request = MagicMock(side_effect=RateLimitError("slow down", platform="facebook")) + + assert provider.get_granted_scopes("token") is None + + +def test_providers_that_cannot_be_asked_report_unknown(): + from providers.bluesky import BlueskyProvider + + assert BlueskyProvider(CREDS).get_granted_scopes("token") is None + assert InstagramLoginProvider(CREDS).get_granted_scopes("token") is None