The Paginator.paginate class method bypassed the instance, discarding the
resolved_anchor state the controller reads after .call; it had no callers, so
drop it and keep the single new(...).call entry point. Extract the activity
anchor side effect into record_resolved_anchor so the intent is explicit at the
call site, and pin the server contract with a request spec asserting an
unresolvable activity anchor omits the resolved-comment value.
The work package activity tab computed a per-journal sequence_version on
every render — a ROW_NUMBER() window function over a LATERAL join — only to
stamp the legacy data-anchor-activity-id that #activity-N deep links rely on.
Nothing mints those links anymore; copy and share links use
#comment-<journal id>, which needs no extra query.
The activity number is now resolved on demand. Only a request carrying
?anchor=activity-N runs the window function, mapping the number to a journal
id the paginator exposes as resolved_anchor. The view hands that to the
client, which rewrites #activity-N to the canonical #comment-<id> and scrolls
using the comment anchor already present in the DOM. Default renders no longer
touch the window function.
References WP #68063.
We have a built-in bruteforce protection for built-in users. When users
are being created from LDAP on-the-fly, these limits cannot apply, as we
do not have a user object yet.
Instead, we can provide a more generous throttler to block attempts
- our APIv3 relies on having a non-null user
- if all auth strategies fail, the anonymous user is set to User.current
- bcf api is mounted in the root api - path is /api/bcf/v2_1 - that is
the reason for the need of being able to fallback to anonymous user in
the bcf api scope
This warden strategy is primarily used to allow APIv3 requests
from the browser, which only authenticates using its session cookie.
Since this is susceptible to cross-site-request-forgery, prevention of
CSRF must take place. This was so far only ensured through the usage of
the X-Requested-With header. When a client sent along this header, the
server could know that a CORS-preflight request must have been made and
thus the browser most certainly has validated that the request is valid
according to CORS rules.
However, the header itself is a non-standard header and while some JavaScript
frameworks add it to requests, not all of them do. For us this was practically
visible on the API docs hosted under `/api/docs`.
The solution is to expect the browser to send the Sec-Fetch-Site header with a value
of same-origin. This header can't be set through JavaScript, but only by the browser
and the value "same-origin" ensures that scheme, host and port are the same for requester
and requested endpoint, thus eliminating CSRF concerns. This feature is widely supported by
all major browsers, the last of which was Safari which added support 3 years ago.
We might want to consider dropping the check for X-Requested-With entirely, since it should be
superfluous. For now it was left in place for greater compatibility.
The performance decreased because of a combination of calls that were supposed to increase performance.
We have a mechanism in place which automatically eager loads models needed in the element representers when a collection of them is rendered. This is to avoid N+1 queries of course. But, if eager loading is combined with e.g., a LIMIT, which we do because we paginate, rails automatically falls back to issuing two instead of just one SQL statement. Which makes sense as otherwise LEFT JOINS might mess with the result set.
But Rails does so in a somewhat simple fashion. It uses the first query to get the DISTINCT ids. The second is used to load the values (without a limit). But instead of removing all WHERE statements in the second SQL statement and then apply just the one for the ids, it keeps the original WHERE statement and applies the one for the ids on top. The problem with that is that the database trips on that (I didn't check the why) and uses a less than optimal query plan.
That was the problem here as well. The first query remained reasonable quick (300ms) but the second one took 25s.
The fix is to split the two statements by hand in the representer whenever eager loading is defined. The first query has all the filters but no eager loading and fetches the ids. The second takes the ids, and with eager loading included loads the data. Et voila, second query takes 10ms.
That at least works for relations, work_packages and projects. But there are other representers that also seem to have custom behaviour. I'm looking into whether they can be easily adapted.
Another update to the mcp gem switched some error
behaviour back to how we originally expected it to be.
This should finally be in line with the MCP specification
again.
Verify that semantic work package identifiers (e.g. "TESTPROJ-1")
are resolved end-to-end through the controller and API layers,
using with_settings/with_flag helpers instead of allow mocks.
Klaus Zanders