Only users that have the new `viewHiddenGroups` permissions will be able to see these groups.
You might want this when you want to give certain users special permissions, but don't want to make your authorization scheme public to regular users.
Co-authored-by: luceos <daniel+github@klabbers.email>
There are two more API integration tests that explicitly add the
"Authorization" header right now:
- `Flarum\Tests\integration\api\authentication\WithApiKeyTest`
- `Flarum\Tests\integration\api\csrf_protection\RequireCsrfTokenTest`
These two specifically test authentication, so in those cases the
explicitness seems desirable.
Test the request, not a controller (implementation detail). This also
focuses on the observable behavior instead of hacking our way into the
middleware pipeline in order to observe internal behavior.
The authenticated user is now determined by looking at the API response
to compare permissions and (non-)existing JSON keys.
We decided it is better to have a less intelligent search (that does not
match search terms in titles) for some people than a bad-performing
search for everyone.
We will revisit the search performance topic in the next release cycle,
possibly with larger changes around indexing.
Refs #1738, #1741, #1764.
In flarum/core#1854, I changed the implementation of `assertCan()` to be
more aware of the user's log-in status. I came across this when unifying
our API's response status code when actors are not authenticated or not
authorized to do something.
@luceos rightfully had to tweak this again in ea84fc4, because the
behavior changed for one of the few API endpoints that checked for a
permission that even guests can have.
It turns out having this complex behavior in `assertCan()` is quite
misleading, because the name suggests a simple permission check and
nothing more.
Where we actually want to differ between HTTP 401 and 403, we can do
this using two method calls, and enforce it with our tests.
If this turns out to be problematic or extremely common, we can revisit
this and introduce a method with a different, better name in the future.
This commit restores the method's behavior in the last release, so we
also avoid another breaking change for extensions.
This test would have failed without commit ea84fc4. Next, I will revert
that commit and most of my PR #1854, so we need this test to ensure the
API continues to behave as desired.
This fixes a regression from #1843 and #1854. Now, the frontend again
shows the proper "Incorrect login details" message instead of "You
do not have permission to do that".
HTTP 401 should be used when logging in (i.e. authenticating) would make
a difference; HTTP 403 is reserved for requests that fail because the
already authenticated user is not authorized (i.e. lacking permissions)
to do something.
* Integration tests: Memoize request handler as well
This is useful to send HTTP requests (or their PSR-7 equivalents)
through the entire application's middleware stack (instead of
talking to specific controllers, which should be considered
implementation detail).
* Add tests for CSRF token check
* Integration tests: Configure vendor path
Now that this is possible, make the easy change...
* Implement middleware for CSRF token verification
This fixes a rather large oversight in Flarum's codebase, which was that
we had no explicit CSRF protection using the traditional token approach.
The JS frontend was actually sending these tokens, but the backend did
not require them.
* Accept CSRF token in request body as well
* Refactor tests to shorten HTTP requests
Multiple tests now provide JSON request bodies, and others copy cookies
from previous responses, so let's provide convenient helpers for these.
* Fixed issue with tmp/storage/views not existing, this caused tmpname to notice.
Fixed csrf test that assumed an access token allows application access, which is actually api token.
Improved return type hinting in the StartSession middleware
* Using a different setting key now, so that it won't break tests whenever you re-run them once smtp is set.
Fixed, badly, the test to create users etc caused by the prepareDatabase flushing all settings by default.
* added custom view, now needs translation
This creates a dedicated test suite for integration tests. All of them
can be run independently, and there is no order dependency - previously,
all integration tests needed the installer test to run first, and they
would fail if installation failed.
Now, the developer will have to set up a Flarum database to be used by
these tests. A setup script to make this simple will be added in the
next commit.
Small tradeoff: the installer is NOT tested in our test suite anymore,
only implicitly through the setup script. If we decide that this is a
problem, we can still set up separate, dedicated installer tests which
should probably test the web installer.
