Unfortunately the "Custom Actions" usage of `ajv.compile()` requires that
the `script-src` CSP directive contain `'unsafe-eval'`, otherwise the
whole feature breaks.
Using `'unsafe-eval'` defeats much of the point of CSP, so it should be
removed as soon as possible. Bug 1530607 is filed to track.
The latest client release still supports Python 2, and given the rarity
of client releases, it's unlikely that a new release will be made any
time soon - so requiring Python 3 on `master` will have little effect.
Previously the frontend would calculate the access token expiry timestamp
in milliseconds and pass it to the `/login/` API via an `ExpiresAt` header.
The backend would then convert both the Id Token's `exp` and current time
to milliseconds, when calculating the earliest expiry. The result then
had to be converted back to seconds for use with Django's session
`.set_expiry()`.
It is instead much simpler to leave everything in seconds, since none of
the Auth0-provided inputs were in milliseconds to start with, so there is
no loss of precision, just fewer conversions required. Timestamps are also
more commonly in seconds, so use of seconds is less surprising.
After this is deployed there will initially be users who have old frontend
pages open that are still sending the expiry as milliseconds. In order to
be able to differentiate between new and old clients, the header has been
renamed to `Access-Token-Expires-At` (which also makes it clearer as to
what the expiry is for, given there is also an Id Token expiry), and a
temporary fall-back added to the backend that can be removed after a few
days has passed.
Since `test_get_username_from_userinfo` is a little too narrowly-scoped
and would be better as an API test. It has been combined with two other
tests in `test_auth.py` to give a more representative workflow test.
Since:
* They don't need to use the slower `transactional_db` fixture that has
advanced transaction-inspecting support.
* They don't need to add a request finalizer, since the `db` fixture
cleans up the User during test teardown automatically.
* `User` does not need to be imported locally.
This speeds up `test_auth.py` by 4x.
Since the name/description references the pre-auth0 implementation that
was removed in #3144. The `test_user` fixture has also been removed,
since it is not required for the test to run (the error referenced in
the comment no longer occurs).
The latest policy used in the report-only header has been working well
on production (the violation reports logged to New Relic are only from
scripts injected by browser addons), so we're ready to start enforcing
the policy by using the real `Content-Security-Policy` header name.
NB: When features are added in the future, PR authors and reviewers will
need to remember to update the policy if needed (for example to add domains
to the `connect-src` directive). The CSP header is not enabled when using
`webpack-dev-server` (it would break dev source maps and react-hot-loader)
so if in doubt test locally (using `yarn build` and serving via Django
runserver) or on prototype first.
See:
https://developer.mozilla.org/en-US/docs/Web/HTTP/CSPhttps://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy
Notably this adds support for Django 2.1, unblocking upgrading from 2.0.
The custom job details API `NumberInFilter` has been removed, since it
wasn't working after the breaking changes in django-filter, and is not
really needed now that the job details API has pagination enabled, so
has a default `LIMIT 2001` applied to queries.
Changes:
https://github.com/carltongibson/django-filter/blob/master/CHANGES.rst#version-20-2018-7-13
Occasionally failing build/test runs can fail in such a way that results
in a significant amount of log spam and therefore log files that are
hundreds of MB in size each. This can cause log parsing backlogs,
particularly when many jobs on the same push fail in such a way.
The log parser now checks the `Content-Length` of log files prior to
streaming them, and skips the download/parse if it exceeds the set
threshold. The frontend has been adjusted to display an appropriate
message explaining why the parsed log is not available.
The threshold has been set to 5MB, since:
* the 99th percentile of download size on New Relic was ~2.8MB:
https://insights.newrelic.com/accounts/677903/dashboards/339080
* `Content-Length` is the size of the log prior to decompression, and
the chronic logspam cases have been known to have compression ratios
of 20-50x, which would translate to an uncompressed size limit of
up to 250MB (which is already much larger than buildbot's former 50MB
uncompressed size limit).
The id token payload contains an `exp` property, which is an integer
representing the number of seconds past the epoch at which the id token
expires.
However the mocked value in our authentication tests was the string `'500'`,
which is neither the correct data type, nor a timestamp. This meant that
during tests only, the `min(accesstoken_exp_in_ms, idtoken_exp_in_ms)`
in `AuthBackend.authenticate()` was comparing an int and a string, which
under Python 3 results in:
`TypeError: '<' not supported between instances of 'str' and 'int'`
A later bug/PR will refactor the auth backend to fix issues unrelated to
Python 3 compatibility and add more test coverage.
Since otherwise report submissions will fail with an HTTP 403 on browsers
that send the Django session cookie but not the CSRF token.
The collector has also been rewritten to be a standard Django view rather
than a django-rest-framework APIView, since the latter is more of a
hindrance than a help for this use-case (particularly now we're wanting
to disable CSRF checks).
Makes the following changes to the initial header added in #4678:
1) Adds a `frame-src` directive
Whilst the Auth0 domain is already whitelisted in `connect-src` allowing
initial logins to work, Auth0.js renewals are performed in an iframe, so
need both the auth0 domain and `'self'` (for the `/login.html` callback)
to be permitted via `frame-src`.
2) Adds https://taskcluster-artifacts.net to `connect-src`
Since some requests to `queue.taskcluster.net` redirect to it (eg for the
"Add new jobs" feature), and for redirects both the original and new domain
need whitelisting.
3) Adds `'report-sample'` to `script-src` and `style-src`, which makes
the browser send JS/CSS samples for any violations of the "inline" rules,
making it easier to debug collected CSP violation reports.
This adds a `Content-Security-Policy-Report-Only` header for static assets
served by WhiteNoise (such as our frontend), which includes a first pass
at a possible policy that should work for Treeherder.
The header also includes a `report-uri` directive, which points at a newly
added API for collecting CSP violation reports. Reports are logged as
warnings (so will appear in Papertrail) and sent to New Relic as a custom
event. This will allow us to see whether the policy would block valid
requests, so we can refine it prior to converting to the real (ie blocks
things) `Content-Security-Policy` header.
The addition of `ng-csp` to `perf.html` is to enable AngularJS's ngCSP
feature, which turns off use of `eval()` and automatic stylesheet
injection, so that the policy directives `unsafe-eval` and
`unsafe-inline` don't have to be used. This requires us to then manually
import the AngularJS stylesheet to include the styles that would have
previously been injected:
https://docs.angularjs.org/api/ng/directive/ngCsp
See:
https://developer.mozilla.org/en-US/docs/Web/HTTP/CSPhttps://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policyhttps://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy-Report-Only
Ed Morley
Renovate Bot
dependabot[bot]
Sarah Clements
dependabot[bot]
Sarah Clements
ionutgoldan