We found another scenario where signing out of the db can cause a 500.
If the user archives their trial mode service, current_service.active = false, then signs out, the current user was being signed out client side first, meaning current_user is now an Anonymous user, next the call to the API is made to log out user on db, all calls to NotifyApiClient `check_inactive_service`, which is only authorised if user is platform_admin, but an AnonymousUser does not have that attribute, so a 500 is raise.
Seemed a bit cleaner to change the User.signout method to rather than the `check_inactive_service` method for current_user.is_authenticated.
I ended up creating a new test user and logged_in_client, which isn't really great. But I tried adding a current_session_id to the active user in the test, but that broke all other tests.
I tried setting current_session_id in all the users being tested but that didn't work either. I'd like to come back to fixing the tests and reducing the number of conftest methods in another PR. For now this fixes the bug.
Anytime a user clicks "sign out" we should be signing them out server side as well. This can be accomplished by setting the Users.current_session_id = null.
I found that the method User.logged_in_elsewhere doesn't need to check if the current_session_id is None. The current_session_ids in the cookie and db (redis or postgres) then the user should be forced to log in again.
previously it assumed that invalid_pages would always exist, however it
might be `None` if the error isn't on a specific page. Errors on
specific pages include a page not being A4 or content being outside the
boundary. Errors not on specific pages include the file not being a pdf,
or containing too many pages
Cookies using the hostname as a domain will be set
with a prefix of `.` by browsers, it seems to
ensure all subdomains are included in the scope.
When deleting the `seen_cookie_message` cookie we
want to set its domain without a `.` prefix, to
match the domain set by the original code.
Leaving the `domain` attribute out from the cookie
string will ensure the `.` prefix is not set.
make sure everything is using the `nl2br` formatter that properly wraps
it in markdown to keep everything sanitised nicely. Also write a couple
of tests
We have been clearing all the Google Analytics
cookies on each page request.
It is now possible for a user to consent to having
Google Analytics cookies so this should have been
checking for that before deleting them.
This makes that change, with tests for those
scenarios.
Removes the following fields from the tracker
config:
- `name`, which was erroring due to it
including a `.`
- `displayFeaturesTask` which seems to be
deprecated
Also refactors the `create` command to put all
fields into the options parameter, as shown in the
developer docs:
https://developers.google.com/analytics/devguides/collection/analyticsjs/creating-trackers
https://jestjs.io/docs/en/configuration#testurl-string
Affects all DOM APIs that return information about
the URL, for example window.location.
Why:
We now have tests for setting/deleting cookies.
Tough-cookie, the library JSDOM uses for cookie
handling cookies doesn't allow setting cookies
with `domain=localhost`. This is correct by
RFC6265, the standard it follows, as domains must
have 2 or more `.`s in them.
The only way to set a cookie on `localhost` is to
leave out the `domain` attribute.
The code we are testing sets and deletes cookies
set on specific domains so using `localhost` is
out.
We also cannot just set/delete cookies on the
domains used as cookies are required to match the
domain of the current page.
The solution we are left with is to set the
current page to one from production and make sure
each cookie is set relative to that domain.
Note: this introduces `testURL` in isolation to be
sure it doesn't break any existing tests.
service contact blocks contain new lines - and jinja2 normally ignores
newlines (as in it keeps them as new lines) - but we need to turn them
into `<br>` tags so that we can show the formatting that the user has
added. We were previously just doing `{{ block | nl2br | safe }}`. nl2br
turns the new lines into `<br>` tags, and then `safe` tells jinja that
it doesn't need to escape the html.
this causes issues if the user adds `<script>alert(1)</script>` to their
contact block (or some other evil xss hack), where that will get let
through due to the safe flag
To solve this, use `Markup(html='escape')` to sanitise any html, and
then convert new lines to <br>.
bump utils
another xss
we all run 3.6 locally, we test against 3.6 both locally and on
concourse, and the latest version of openpyxl (required by
pyexcel-xlsx) doesn't support 3.5 anymore
