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 intention of this change is to always respond in the metadata-rich
version of the header that indicates things like the required scope and
the URL of the resource_metadata endpoint, which was previously hidden
and only visible if clients used a non-standard HTTP request header.
semantically it's probably the preferable version of the header by now
anyways, because:
* all APIs accept some kind of Bearer token, not all of them accept Basic auth
* Even API tokens can now be passed as Bearer tokens
Practically the Basic auth header also caused unintended browser pop-ups when the frontend
code didn't include the correct request header to avoid the Basic auth offer, this now can't
happen anymore, since the Basic auth version of the header is only returned, if the client actively
tried to authenticate through Basic auth.
We generate those tokens with a prefix, so that we
can decide by looking at a token, whether it's an API Token
or a different kind of token, so that we can decide which
code path to choose for validating the token.
The usage of access tokens as Bearer token has the usability advantage,
that you can paste them as plaintext into tools that expect you
to specify the token as a header.
Also the Basic auth approach for our old tokens usually rather caused
issues, such as browsers prompting for credentials in surprising situations.
If we were to deprecate basic authentication one day, this change today could've
been the first step towards that.
We even had a spec testing behaviour of a string that needed
escaping, but due to the fact that we constructed the relatively large
header content in the spec, we never noticed that escaping was indeed
missing in that case.
This one is defined as optional by RFC 6750, which defines
the usage of bearer tokens. It allows a client to know, which
scopes are required to access a given resource when using Bearer tokens.
The current implementation opens a small window of
opportunity to authenticate through a previously issued
access token even after a user account was locked.
If access tokens had a longer lifetime, this could become
a large window of opportunity.
This was initiated by Rubocop complaining
about the method complexity.
Rewriting it with no changes.
Adding additional test cases to specify expected behaviour,
also making edge cases visible that are otherwise covered by
separate feature specs in a different location.
We introduced the ability to disable doorkeeper applications in
the past, but apparently we didn't check that an application whose
token we validate is also enabled.
Now we make sure that only tokens of enabled applications are accepted.
This is intended to be a more natural and readable way of manipulating the
identity_url of a user. It also means that if we ever decide to change
the way how the identity_url is built or how providers are attached to
a user, it should be easier to switch the majority of tests relying on it
over to the new schema.
So far we are only accepting JWTs for requests to
APIv3 and our scopes are not more fine-grained than that.
However, warden strategies are generally responsible for validating
the scopes of a token (e.g. compare DoorkeeperOAuth) to the scope
of the currently accessed API.
Previously we would have "fallen out" of the
JWT strategy, even though the token was clearly intended
to be handled by this strategy. For my configuration this
led to the final error message being generated by the Basic auth
strategy, leading to a wrong error message.
Backported into 15.4 to avoid conflicts.
Previously we went into a context for all the things
that could be correct about a request. While this helps
in distinguishing from the bad scenario, it also forces
to indent by one context per thing that can go wrong.
This also means that certain error cases were artifically
nested inside a "good" case, even though there was no
immediate dependency between the one thing being good, but
not the other.
Now the happy path has no extra indentation/context and
all the things that can go wrong, have one context inside
the main context for the happy path. This is hopefully clearer.
Backported into 15.4 to avoid conflicts.
Instead of relying on raised exceptions
for lots of our control flow, we are now
using a failed operation to represent these.
We are using the Failure result for all previously
considered exceptions, because all of them were kind of
expectable error conditions.
JWT parsing is rather involved, because we need to fetch
proper certificates first. We will need to parse JWTs in
a different context than authorization as well,
so it makes sense to have the parsing centralized.
This also allowed to add specs for this previously
not (unit) tested piece of code.
https://community.openproject.org/work_packages/55643
- Add new warden authentication stategy to handle jwt issued by configured OIDC.
- Modify exisiting doorkeeper_oauth stategy to ignore jwts.
- Fill in WWW-Autheticate header with auth failure information.
- Make keycloak docker dev setup use postgres as a database.
Updates the copyright to 2021 for all files that have a copyright. Files in our source code without the copyright header still do not receive one automatically. Additionally, backlisted files are also excluded.
Previously the copyright of chiliproject which references redmine stated a copyright of redmine up to and including 2017 which is not true for the code we have in here. Because of that I changed that to 2013
Instead of warden responding with 401 "unauthorized", use our own
error response that correctly sets the `WWWW-Authenticat` header.
We tripped into the default 401 error that does not output any headers
if we're not returning any users.
This was caused by another issue: The `session` object may be
present, even though no session id exists. Checking `session&.id`
instead always yields the anonymous user.
This will ensure consumers of APIV3 to always get a JSON / HAL response
as well.
https://community.openproject.com/wp/32486
Jan Sandbrink