- 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.
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.
"eeek" now matches the semantic identifier pattern, so
find_by_id_or_identifier! raises RecordNotFound with
model: "WorkPackage", producing the WP-specific error
message instead of the generic 404.
Extract FinderMethods module that transparently resolves both numeric and
semantic identifiers (e.g. "PROJ-42") using FriendlyId's Object#friendly_id?
for dispatch. The module is included in both the WorkPackage class and
extended onto every relation, so scoped queries like
WorkPackage.visible(user).find("PROJ-42") work seamlessly.
- Override find to resolve semantic IDs via identifier column + alias table
- Override exists? with the same resolution chain
- Refactor find_by_id_or_identifier to use friendly_id? instead of semantic_id?
- Update API route to accept string IDs (type: Integer → type: String)
- Update controller and ViewComponent finders to use find_by_id_or_identifier
- Pass display_id from Rails views to Angular custom elements
* Add rel=nofollow to user-generated links to deter SEO spam
Links in user-generated content (work package descriptions, comments,
wiki pages) previously carried rel="noopener noreferrer" but not
nofollow. Search engines therefore passed PageRank through them, making
OpenProject community instances attractive targets for spammers posting
links for SEO gain.
Adding nofollow removes this incentive without any visible impact on
legitimate users.
* Fix missing nofollow in AutolinkCustomProtocolsFilter
The class-level visible scope now verifies the user retains access to
the associated remindable, consistent with the existing instance method.
Updated specs to reflect the corrected authorization behavior.
The setting previously used "numeric" and "alphanumeric" as its allowed
values. Rename them to "classic" and "semantic" to better align with the
product terminology for the work package identifier modes.
Includes a migration to update any stored setting values in the database,
updated constants and helper methods on Setting::WorkPackageIdentifier,
and all corresponding references across models, components, forms,
frontend controllers, locales, and specs.
Remove the early return in identifier_alphanumeric_format so users see
all validation errors at once (e.g. "123!" reports both
must_start_with_letter and no_special_characters). Simplify the special
characters regex since the leading-letter check is already separate.
Update API specs to use test identifiers that trigger only one
validation error, matching the new non-short-circuiting behavior.
Oliver Günther