From ad2cf164c5f24319c091da0878c567a6540c9b2d Mon Sep 17 00:00:00 2001 From: Jan Schmitz Date: Thu, 10 Sep 2026 21:57:03 +0200 Subject: [PATCH 1/2] fix(social-accounts): make a partial Meta grant visible, and stop faking revoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Meta drops permissions it has not approved, or that the user declined, without failing the grant. The connection then reports Connected and only breaks later — at publish or insights time — with an opaque platform error. That is how Instagram publishing stayed broken until Meta's app review said so, rather than the app saying so at connect time. Read the grant back after connecting and record what is missing. The readback inspects the token via /debug_token rather than /me/permissions: these accounts hold a Page token, so /me resolves to the Page, which has no permissions edge. A platform that cannot be asked reports unknown, never an empty set — that would flag every scope as missing. The comparison applies the same analytics-scope flag OAuth used, so a scope deliberately omitted when a platform's analytics is off is not reported as absent. Ask again for scopes the user declined. Without auth_type=rerequest Meta skips the dialog entirely on a reconnect, so a declined scope could never be requested a second time. It does not force the full permission list for someone who granted everything — only removing the app does that. Stop pretending disconnect revokes anything. The only endpoint that would, DELETE /me/permissions, revokes this app for the whole person: one workspace disconnecting one Page would sever every other Page and Instagram account they connected anywhere. It never worked here anyway, being called with a Page token. It is now an explicit no-op, and the disconnect dialog says where to actually revoke. Underneath that sat a real defect: select_account and the connection-link flow had each written their own page-token selection and diverged. Onboarding fell back to the *user* token for a Facebook Page, which publishes under the wrong identity and made that account eligible for the revoke above. Both flows now share resolve_page_account_token, and a Page we have to skip is reported to the client instead of leaving them on a success page for an account that was never connected. Also fixes a permanently-red test: test_fetch_post_comments... pinned 2026-08-07, which had drifted past FACEBOOK_FEED_WINDOW_DAYS so the window floor landed after it and the assertion inverted. Both dates are now relative to now. Co-Authored-By: Claude Opus 5 --- apps/onboarding/tests.py | 67 +++++++ apps/onboarding/views.py | 26 ++- .../management/commands/diagnose_facebook.py | 4 +- .../0018_socialaccount_missing_scopes.py | 18 ++ apps/social_accounts/models.py | 23 +++ apps/social_accounts/provider_factory.py | 15 ++ .../tests/test_diagnose_facebook.py | 2 +- .../tests/test_webhook_subscription.py | 169 ++++++++++++++++++ apps/social_accounts/views.py | 50 +++--- apps/social_accounts/webhooks.py | 38 +++- providers/base.py | 14 ++ providers/facebook.py | 76 ++++++-- providers/instagram.py | 63 ++++++- providers/instagram_login.py | 14 +- providers/meta_oauth.py | 31 ++++ .../partials/_account_card.html | 29 ++- tests/providers/test_facebook.py | 10 +- tests/providers/test_meta_replies.py | 85 +++++++++ 18 files changed, 672 insertions(+), 62 deletions(-) create mode 100644 apps/social_accounts/migrations/0018_socialaccount_missing_scopes.py create mode 100644 providers/meta_oauth.py 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/social_accounts/partials/_account_card.html b/templates/social_accounts/partials/_account_card.html index c1d15b1f..c9298344 100644 --- a/templates/social_accounts/partials/_account_card.html +++ b/templates/social_accounts/partials/_account_card.html @@ -54,7 +54,18 @@

{{ account.account_nam
-

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.

+ {# 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. #} + {% 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 %}
{{ account.account_nam
+ {% comment %} + Meta drops permissions it has not approved, or that the user declined, + without failing the grant — so the connection reports healthy and only + breaks later, at publish or insights time, with an opaque platform error. + Naming them here turns that into something the user can act on. + {% endcomment %} + {% if account.missing_scopes %} +
+

+ 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. +

+
+ {% endif %} + {% if account.last_error and account.connection_status == "error" %}

{{ 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 From ba75b6e1ac5934be6b18e829e8e8269d62c0cc43 Mon Sep 17 00:00:00 2001 From: Jan Schmitz Date: Fri, 11 Sep 2026 11:48:10 +0200 Subject: [PATCH 2/2] fix(templates): stop multi-line {# #} comments rendering to the page Django's {# #} syntax is single-line only: the lexer matches {#.*?#} without DOTALL, so a comment that wraps onto a second line is never recognised as one and falls through as plain text. Two comments had wrapped, and were printing verbatim in the UI - the disconnect rationale inside the account card's confirmation popover, and the first-comment note in every row of the sent-posts table. Move both to {% comment %} blocks, which do span lines. Co-Authored-By: Claude Opus 5 --- templates/calendar/partials/publish_sent.html | 6 ++++-- .../social_accounts/partials/_account_card.html | 12 +++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) 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 c9298344..1bb9c166 100644 --- a/templates/social_accounts/partials/_account_card.html +++ b/templates/social_accounts/partials/_account_card.html @@ -55,11 +55,13 @@

{{ account.account_nam

Disconnect {{ account.account_name|default:account.account_handle }}? Historical data will be preserved.

- {# 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. #} + {% 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,