Find Facebook Pages a login only reaches through a Business Portfolio - #301
Open
paulocastellano wants to merge 27 commits into
Open
Find Facebook Pages a login only reaches through a Business Portfolio#301paulocastellano wants to merge 27 commits into
paulocastellano wants to merge 27 commits into
Conversation
/me/accounts silently omits Pages that live under Meta's newer "New Pages Experience" / Business Portfolio model, even when the token's granular scopes show the Page was explicitly granted - confirmed via Meta's own Access Token Debugger against a live account whose Page returned zero results from /me/accounts but resolved fine when queried directly by ID. Falls back through Business Manager's owned_pages/client_pages (via the existing business_management scope) when /me/accounts comes back empty, so Pages under that model are still found.
Same root cause and fix as the FacebookController fetchPages() fallback - the Page/Instagram-linked-Page lookup goes through the same /me/accounts call and is subject to the same Meta-side gap.
Both controllers walked /me/accounts and, when it came back empty, the Business Portfolio edges behind it. ManagedPages now owns that walk for the Facebook and Instagram-via-Facebook flows alike. Three behaviour changes come with it: The portfolio edges are read on every connect, not only when /me/accounts is empty, and merged by Page id. Someone holding one Page by a classic role and the rest through a portfolio was auto-connected to that single Page and never offered the others. /me/businesses is read only once /me/permissions confirms the login granted business_management, and a failure anywhere along the portfolio walk leaves the /me/accounts list standing. It used to escape into the callback's catch, turning "no pages" into "could not connect" for every login without the scope. Pages the login cannot get an access_token for are dropped. Connecting one produces an account that cannot publish.
Covers the merge with /me/accounts, the access_token filter, the business_management gate, and a failing portfolio edge leaving the /me/accounts list intact — plus the connect flow end to end on both Meta platforms. The Instagram-via-Facebook request count moves from five to six for the /me/permissions check.
The /me/permissions check saved a rejected /me/businesses call on logins without business_management, at the cost of running a path nobody had verified against a live account. The edges already fail soft, so the check bought log tidiness and nothing else.
3 tasks
The multi-Page selection flow behind a portfolio — the case #292 asked a maintainer to check — plus pages spread across two portfolios, a paginated edge, a portfolio entry with no id, and a portfolio page merging with one /me/accounts already returned.
Http::fake only stubs the URLs it is given; anything else goes out over the network. These files stubbed /me/accounts and left /me — and now /me/businesses — unstubbed, so the suite was issuing live requests to graph.facebook.com on every run. They came back 400 and the code under test swallowed them, so nothing ever went red while the assertions were measuring Meta's answer instead of the fixture's. Every Graph call the connect flow makes is stubbed now, and the files prevent stray requests so a missing one fails loudly. Inertia's SSR endpoint is allowed through; it is not what these tests are about.
GraphPaginator throws so no caller reads a failed fetch as an empty list and auto-connects whatever arrived first. Swallowing that exception on the portfolio edges gave the invariant away: a 429 on owned_pages left the merged list holding only the /me/accounts page, and the callback connected it with no picker. The exception now carries whether the failure was transient, classified by GraphError, which already owns Meta's rate-limit and transient code table. A denied permission reads as "this login reaches no portfolio pages"; a throttle, a 5xx or a truncated walk is raised. The walk also stops at MAX_PORTFOLIOS and logs what it skipped. Each portfolio costs two more paginated edges inside a synchronous OAuth callback, and nothing bounded that loop.
Concurrent Instagram lookups. The Instagram description ran one request per Page, in sequence, at a 15s timeout each. That list used to be the Pages someone holds a role on — a handful. It is now the union with every portfolio's owned_pages and client_pages, so a portfolio holding hundreds of Pages serialised the OAuth callback past any gateway timeout, for exactly the accounts the portfolio walk exists to reach. Meta's ids= batching is no help: each Page carries its own access token and one call takes one token. The lookups run in concurrent rounds. A Page without a token is not a Page you don't have. Meta lets someone decline pages_read_engagement on its per-permission toggles and still lists the Page, without an access_token. Dropping it inside the walk left the caller saying "no Pages found, you need to be an admin of at least one" to an admin. ManagedPages returns everything Meta listed and publishable() separates what can be posted to, so the callbacks can tell the two apart and say which one happened. Stored scopes are what Meta granted. The scope list was written to the account's scopes column straight from the request, claiming access the login may have refused — business_management above all, which needs Advanced Access and is declined by default without it. It now comes from /me/permissions, falling back to the request when Meta cannot be asked.
PublishToSocialPlatform::failForMissingScopes() blocks a post when a platform's required publish scope is absent from the account's scopes column, so writing that column from /me/permissions can dead-end an account. Meta does not document that the endpoint echoes scope strings verbatim, and the edge is paginated, so a scope it never mentions is unknown rather than refused and stays. Only declined and expired drop.
Raise instead of truncating. The ceiling logged a warning and returned whatever fit, which is the one thing this module refuses to do everywhere else: if the walk cannot finish, the real list is unknown, and a truncated list holding exactly one Page would have been auto-connected without ever showing the picker. It now raises, and the ceiling rises to GraphPaginator::MAX_PAGES' 100 since the walk no longer pays for it serially. Read the edges concurrently. Up to two paginated edges per portfolio ran back to back inside the OAuth callback. They run in rounds now; a URL that does not come back cleanly still goes through GraphPaginator, which owns the single place that logs a Graph failure and decides whether it is a rejection or an unknown. Prefer the record that carries a token. Merging kept whichever copy of a Page id arrived first, and /me/accounts always arrives first — so a Page listed there without a token buried the portfolio copy that had one, and the login was told its permission was missing for a Page it could reach. Describe only the Instagram accounts that survive. The per-Page lookup ran before filterConnectableIdentities discarded them, spending a BUC-rate-limited call on every Page only to throw the answer away. The filter reads instagram_business_account.id straight off the raw Page, so it needs no lookup to run first. Two Instagram tests mocked Socialite without usingGraphVersion, so the callback threw, the generic catch answered, and asserting only success=false passed on the error path instead of the one under test.
The concurrency test drove 30 portfolios with every edge empty, so the merge across rounds was never exercised with data in it.
GraphPaginator refuses to follow a paging.next that points off the host the walk started from, so a tampered response cannot carry the access token somewhere else. Reading the first page of each edge through the pool and handing its paging.next straight back to GraphPaginator made that URL the *start* of a new walk, which is the one URL the guard trusts implicitly — so the first hop went unchecked. The host is compared before the hand-off now, and a mismatch re-walks the edge from the beginning so GraphPaginator's own guard is what refuses it, with its logging.
optional() was written for "this edge may be forbidden" and answers a rejection with an empty list. Following paging.next through it gave a rejected cursor the same answer: page one of a 250-Page portfolio came back and the rest was dropped, and a single connectable Page in that fragment would have been auto-connected with no picker. The same hole sat on /me/businesses, where GraphPaginator is all-or-nothing — a failure on page two threw away the portfolios page one had already listed, degrading the connect back to /me/accounts alone in silence. The exception now carries how many pages arrived. Only a rejection on the very first request reads as "this edge is not readable"; anything after that is a fragment and raises. Cursors skip optional() entirely. A login Meta reports as refusing business_management also stops walking the edges at all. The controllers already read /me/permissions for the scopes column, so the answer costs nothing, and the walk was otherwise spending a request on a certain 403 — and logging it at error level — on every successful connect by such a login. An Instagram account with an empty Name connected as display_name null: data_get's default only fires on an absent key, and describeRound always writes the key.
A card whose Page moved behind a portfolio is the reconnect shape of the bug this branch fixes, and nothing covered it: the walk has to find the Page, and filterConnectableIdentities has to keep the original card rather than offering the portfolio's other Pages.
The portfolio edges are additive, but every failure in them was raised and the callback's generic catch turned it into "error connecting" — so one throttled edge among sixty denied a login the Pages /me/accounts had already returned, and each retry burned more of the quota that caused it. Only /me/accounts failing is fatal now; everything else marks the walk incomplete and keeps what arrived. What the raise was protecting is kept where it belongs: a lone Page is only taken without asking when the walk saw everything, or when a reconnect has already pinned which Page is wanted. Otherwise the picker opens, and the login can see for itself that its Page is not there. The ceiling stops pretending. It compared the count after walking every page of /me/businesses — up to ten thousand ids — so the runaway it existed to bound had already happened. One request, one page, and more portfolios than that is an incomplete walk rather than a failed one. A pooled edge that fails is classified where it lands instead of being re-fetched, halving the cost of the common client_pages rejection, and GraphPaginator logs a confirmed rejection at warning: it is Meta answering the question, not something going wrong. A login that declined the permission its platform needs to publish is refused at connect. Meta issues a Page token off pages_show_list, so declining pages_manage_posts still produced a green account whose every scheduled post was then hard-failed by failForMissingScopes. Test fakes address Graph through the config rather than a literal host, which is what CLAUDE.md asks for and what the newer tests already did.
Marking the walk incomplete stopped the auto-connect, but every dead end after it still gave a definitive answer. A login whose only Pages sit behind a throttled portfolio was told "no Facebook Pages found, you need to be an admin of at least one" — the exact sentence this branch exists to stop showing to admins, now arriving for a different reason. The already-connected and missing-permission answers were equally sure of themselves. When the walk could not see everything and there is nothing to offer, it says so and asks for a retry.
inertia.ssr.enabled defaulted to true and nothing in phpunit.xml turned it off, so every test rendering an Inertia page issued a real request to the SSR endpoint. The project does not use SSR, so those calls only ever failed and fell back to client rendering — quietly, on every run. Defaulting it off is what the project already assumed, and it retires the allowStrayRequests hole the Meta connect tests were carrying to work around it. INERTIA_SSR_ENABLED still turns it back on.
Routing every short listing to "try again in a moment" swallowed network_taken: a workspace that already holds its one Facebook account was told to retry, forever, whenever a portfolio edge was throttled. That answer comes from our own rows and does not depend on how far the walk got. all_connected and page_not_found do, and still yield. Also: an off-host cursor now stops the edge instead of re-reading page one, which cost a request and could follow an on-host cursor on the retry, quietly undoing the guard. Cursor follow-ups are budgeted, since they cannot be pooled and were the one unbounded serial path left. The exception's fetched count lost its last reader two commits ago and is gone. Comments trimmed throughout.
MAX_CONTINUATIONS counted edges, not requests: each one then handed off to GraphPaginator, which follows up to a hundred more pages by itself. The budget the docblock promised was fifty times larger than it claimed. Cursors are now followed one budgeted request at a time, so the count means what it says, and pages already read survive a cut-off instead of being thrown away with the exception. A refused /me/businesses is no longer read as "this login has no portfolios". For a single edge a rejection answers the question; for the index of edges it means we could not look — and answering complete there auto-connected the one /me/accounts page while hiding every portfolio Page, which is this branch's own bug wearing a different hat. SSR goes back to its shipped default. Turning it off in config to quiet the test suite would have disabled it wherever it is actually started — docker/Dockerfile builds the bundle. phpunit.xml carries the switch now, next to PULSE, TELESCOPE and NIGHTWATCH, and CLAUDE.md records why.
The Facebook and Instagram-via-Facebook callbacks ran the same twenty-five lines: the profile touch Meta's review wants, the granted-scope read and the publish-scope refusal, the page walk, and the answer for a walk with nothing to offer. They only matched because both were edited side by side, every round, which is a guarantee nobody should be making by hand. graphApi() moves to SocialController and reads the host by platform value, so it serves every network rather than the two that had copied it, and graphVersion() derives from it instead of reading config a second time. select() stays as it is. The two differ in the middle — different identity keys, different connect shapes — and folding them would be abstraction for its own sake.
Nothing in the repo starts an SSR process, so the shipped default was describing a setup that does not exist. With it off the test env needs no override of its own, and CLAUDE.md records that turning it on means starting the process, not just flipping the env.
Last round I made a refused /me/businesses mark the walk incomplete, on the argument that refusing the index means "we could not look". That was wrong in the case that matters most: an app without Advanced Access for business_management gets that refusal on every single connect, so every login on such an install lost auto-connect and every login without Pages was told to retry forever. Self-hosted in Live mode is exactly that. The rule that holds everywhere: a Page this login cannot enumerate is a Page it cannot get a token for, so it was never connectable, and the list of connectable Pages is complete. Only an unknown — a throttle, a hiccup, a budget or a ceiling — leaves the walk unable to vouch for itself. Index and edge now answer the same way, which is also what makes the two readable together. The per-request budgets were each bounded while their sum was not: ten pooled rounds plus twenty-five cursor requests can outlive nginx's fastcgi_read_timeout of 120s. The walk now carries a deadline and returns what it has. touchProfile exists only because Meta's review wants the call. It had no timeout and no guard, so a hung /me could stall the callback to the gateway timeout or fail a connect outright, over a response nobody reads.
It still said any failure marks the walk incomplete, which stopped being true when a refusal became an answer. A docblock describing an invariant the code no longer holds is how this branch got two of its bugs.
meta_page_walk_seconds bounded the portfolio half of the walk and nothing else. /me/accounts could paginate a hundred pages at fifteen seconds each, and the Instagram lookups pooled in rounds that were themselves serial — a portfolio with three hundred linked Pages is fifteen rounds, after the walk had already spent its own budget. Both honour the deadline now. The lookups skip rather than drop: the Page still connects, only its handle and avatar arrive empty. The first request is always made; the budget bounds what comes after it. A Graph body that is valid JSON but not an object — a proxy answering "throttled" — reached GraphError::isTransient, whose parameter is ?array, and under strict_types raised a TypeError. That is an Error, so it walked past both callbacks' catch(\Exception) and 500'd the popup instead of showing a message. Refusing a login before any listing has happened no longer borrows the wording for "we found your Pages but not the permission to post to them". composer run dev no longer starts an SSR process for SSR that is off, and CLAUDE.md no longer claims nothing in the repo starts one, which composer.json contradicted.
The Page node reference is explicit: access_token is "only returned if the User making the request has a role (other than Live Contributor) on the Page". Being an admin of the portfolio that owns a Page lists it but does not grant that role, so the walk can surface Pages this login will never get a token for. The popup told those users to reconnect and accept every permission, which cannot produce a Page role and so could never work. It now names the role as well. I rejected this in review on the grounds that a portfolio Page had been published to successfully in the wild. That proved a token comes back when the login holds a role, not that one always does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carries the two commits from #292 by @StoriaJames, who found the bug and reproduced it live, and adds the review round on top. Opened here because the fork is organization-owned, so GitHub does not let maintainers push to that branch.
The bug
/me/accountsis documented as "a list of Pages you have a role on" — a classic Page role. Someone who administers a Page through a Business Portfolio assignment, the norm under the New Pages Experience, holds no classic role, so the edge answers200withdata: []and the popup says "No Facebook Pages found. You need to be an admin of at least one page."@StoriaJames confirmed it against a live account with Meta's Access Token Debugger: the token's granular scopes showed
pages_show_list,pages_read_engagementandpages_manage_postsall granted scoped to the Page's id,/me/accountsstill came back empty, and querying the Page node directly by id worked.The empty list is also what the popup message proves:
GraphPaginatorthrows on any HTTP failure, and that surfaces aserror_connecting. Users sawno_facebook_pages, so Graph really did answer 200 with nothing in it.The fix
business_managementjoins the Facebook scopes — the Instagram-via-Facebook flow already requested it on the same Meta app — andApp\Services\Social\Meta\ManagedPagesnow owns the Page lookup for both flows:/me/accountsfirst, then the portfolio'sowned_pagesandclient_pagesbehind/me/businesses, merged by Page id.Three things differ from #292 as it stands:
/me/accountsis empty. Someone holding one Page by a classic role and the rest through a portfolio was auto-connected to that single Page and never offered the others — the multi-Page case Fix Facebook/Instagram Page fetch missing New Pages Experience pages #292 left open in its test plan./me/accountslist standing. Previously the/me/businessescall sat outside thetry, so its exception escaped into the callback'scatchand turned "no pages" into "could not connect" for every login without the scope. Copilot flagged the same thing on Fix Facebook/Instagram Page fetch missing New Pages Experience pages #292.access_tokenfor are dropped. Connecting one produces an account that cannot publish./me/accountsitself stays strict: a failure there still aborts rather than reading as an empty list, soGraphPaginator's guarantee is intact.Before merging
business_managementneeds Advanced Access on the Meta app. The Instagram-via-Facebook flow has requested it on this same app for a while, so it is very likely already approved — worth confirming in the Meta dashboard, because without it/me/permissionswill not list the scope and the portfolio walk simply never runs. Nothing breaks; the bug just stays.Tests
Six unit tests on
ManagedPages(merge with/me/accounts, theaccess_tokenfilter, thebusiness_managementgate, a failing portfolio edge, a failing/me/accountsstill aborting), two feature tests on the Facebook connect flow, one on Instagram-via-Facebook. Full suite: 3892 passed, 1 skipped.