Commit Graph

8425 Commits

Author SHA1 Message Date
Chris Hill-Scott
19ad11e383 Don’t repeat digits in security codes
People with dyslexia and dyscalculia find it difficult to transpose
codes which have consecutive, repeated digits[1].

This commits enhances the algorithm for generating codes to not repeat
the previous digit in a code.

This reduces the key space for our codes from 100,000 possibilities to
65,610 possibilities.

1. https://twitter.com/annaecook/status/1442567679710150662
2021-09-30 10:24:17 +01:00
Katie Smith
75901a02a6 Merge pull request #3334 from alphagov/new-zendesk-form
Use the new Zendesk form for all tickets
2021-09-29 14:09:11 +01:00
Katie Smith
58597653df Update how "sending to TV numbers" Zendesk tickets are created 2021-09-29 11:26:20 +01:00
Katie Smith
0c0c7f4478 Update how "letters still created status" Zendesk tickets are created 2021-09-29 11:23:28 +01:00
Katie Smith
2f66e38fb9 Update how "missing ackfile for letters" Zendesk tickets are created 2021-09-29 11:10:50 +01:00
Katie Smith
64c0a3fb9d Update how 'letters still sending' Zendesk tickets are created
These now use the new Zendesk form.
2021-09-29 11:07:37 +01:00
Katie Smith
b114dadcae Update how pending virus check Zendesk tickets are created
This updates the tickets that are created when the
`check_if_letters_still_pending_virus_check` scheduled task detects
letters in the `pending-virus-check` state.
2021-09-29 11:03:48 +01:00
Katie Smith
9ff0ca0363 Update how live broadcast Zendesk tickets are created
These now use the Notify Form in Zendesk
2021-09-29 10:59:07 +01:00
Katie Smith
329a418cc4 Bump utils to 46.1.0
This is to bring in the Zendesk changes which allow us to create tickets
using the Notify Form in Zendesk.
2021-09-24 08:16:06 +01:00
Ben Thorner
0d45168cd8 Merge pull request #3332 from alphagov/api-docs-179208442
Add new guidance on writing public APIs
2021-09-20 10:36:50 +01:00
Ben Thorner
d1826af3c7 Remove out-of-date docs on new workers
In response to: https://github.com/alphagov/notifications-api/pull/3332#issuecomment-921897641

These were so out-of-date as to be misleading. We can always refer
back to them in the version history if needed.
2021-09-17 16:53:42 +01:00
Ben Thorner
68eeb1defa Merge pull request #3325 from alphagov/prevent-empty-areas-178986763
Add validation to prevent blank area names
2021-09-17 15:20:15 +01:00
Ben Thorner
d8a0967ec0 Add validation to prevent blank area names
Now that these are used for display on gov.uk/alerts we need to
make sure the data is being set properly. We've already found an
example where it wasn't [1]. We validate external broadcasts in
two stages: with the official CAP XML schema [2] and then again
with our own, more specific schema for the converted JSON. Since
this validation is a custom requirement I've made it part of the
JSON schema. Note that jsonschema recommends avoiding metachars
like "\w" since they're not supported by all implementations [3].

I've tested the new validation manually and it works as expected
by disallowing e.g. "  " but still alowing "foo" and "foo bar".

[1]: https://www.notifications.service.gov.uk/services/120107d0-d99a-4c42-8b70-f37d2f28879b/rejected-alerts/d6e0c70e-60f6-4422-8589-2a2d159c63f2
[2]: 81a25ff1ef/app/xml_schemas/CAP-v1.2.xsd
[3]: http://json-schema.org/understanding-json-schema/reference/regular_expressions.html
2021-09-17 13:33:52 +01:00
Ben Thorner
4c7de35f68 Merge pull request #3331 from alphagov/move-govuk-alerts-179208442
Restructure govuk-alerts endpoint to be internal
2021-09-16 13:58:15 +01:00
Ben Thorner
f712bafc7f Add new guidance on writing public APIs
In response to: https://github.com/alphagov/notifications-api/pull/3305#issuecomment-896631475
2021-09-15 17:09:04 +01:00
Ben Thorner
14adddd566 Extract content about new workers into own doc
This is a bit too niche for the README, which should be focussed on
the bare minimum someone needs to know to get started with a repo.
Moving this content to its own doc is consistent with other apps [1]
and gives it more room to grow.

[1]: https://github.com/alphagov/notifications-admin/tree/master/docs
2021-09-15 16:11:54 +01:00
Ben Thorner
6a53871455 Restructure govuk-alerts endpoint to be internal
In response to: https://github.com/alphagov/notifications-api/pull/3305#pullrequestreview-726672421

Previously this was added among the public /v2 endpoints, but it's
only meant for internal use. While only the govuk-alerts app would
be able to access it, the location and /v2 URL suggested otherwise.
This restructures the endpoint so it resembles other internal ones.
2021-09-15 15:36:17 +01:00
Leo Hemsted
d61ec50fb1 Merge pull request #3329 from alphagov/contact-list-tz-fix
return contact list created_at in UTC, not BST
2021-09-15 15:09:59 +01:00
Ben Thorner
12640e5380 Fix misleading test name for error scenario 2021-09-15 11:02:51 +01:00
Ben Thorner
35430e9a9f Refactor custom validation into own function
This sets a pattern for adding another in the next commits.
2021-09-15 11:02:50 +01:00
Leo Hemsted
33ca817e17 return contact list created_at in UTC, not BST
make sure timestamps returned from the api are always consistent.

The only place in models where we're serializing a BST timestamp is on
the Notification.serialize_for_csv method now, which at least is a bit
different as this is user-facing (it also returns a formatted
human-readable notification_status for example).
2021-09-14 12:41:52 +01:00
Ben Thorner
81a25ff1ef Merge pull request #3327 from alphagov/bump-utils-46-0-0
Bump utils to 46.0.0
2021-09-14 11:22:10 +01:00
Ben Thorner
c53ee6de94 Merge pull request #3323 from alphagov/add-permission-local-dev
Make it easy to develop with broadcast services
2021-09-14 11:22:00 +01:00
Ben Thorner
4d05b064fd Remove redundant DB fixtures in conftest.py
These were only there to ensure a DB session existed for the test
and are now included implicitly as one of the dependencies of the
"sample_user" fixture.
2021-09-14 09:37:32 +01:00
Ben Thorner
c9ea85938a Refactor "create_user" to actually create a user
This switches a number of fixtures to use "sample_user", which is
equivalent to calling the previous "create_user" function when it
used to default the email to "notify@...".
2021-09-14 09:29:28 +01:00
Ben Thorner
922fd2f333 Support testing commands and add first test
We have a lot of commands and it's important we test the ones that
are meant to be used in the future to ensure they work when they're
needed. Testing Flask commands is usually easy as written in their
docs [1], but I had to make some changes to the way we decorate the
command functions so they can work with test DB objects - I couldn't
find any example of someone else encountering the same problem.

[1]: https://flask.palletsprojects.com/en/2.0.x/testing/#testing-cli-commands
2021-09-14 09:29:23 +01:00
Sakis
b945d2c1fd Merge pull request #3328 from alphagov/drop-p1-for-broadcasts
Don't raise P1 for broadcasts
2021-09-10 10:28:20 +03:00
sakisv
df739d6b94 Fix flake8 2021-09-10 10:17:28 +03:00
sakisv
9faa3d34e1 Fix tests
Specifically, no longer test for a p1 zendesk when sending an alert
and drop misleading "p1" from test name when cancelling an alert.

We're no longer creating a P1 from the code, but we _do_ create a
zendesk ticket when sending out an alert.

When cancelling, what we want to test is that we don't create a second
ticket when the alert is cancelled.
2021-09-10 10:14:28 +03:00
sakisv
65c21f694c Don't raise P1 for broadcasts
This is happening on the AWS side now as part of
alphagov/notifications-broadcasts-infra#267 - but we still want to keep
the zendesk ticket as it contains useful context _and_ provides
visibility to the team.
2021-09-09 16:44:19 +03:00
Ben Thorner
11e1a597da Bump utils to 46.0.0
This brings in some new polygon simplication code [1] so we need to
tweak any tests that rely on the exact number of polygons after this
operation.

[1]: https://github.com/alphagov/notifications-utils/pull/890
2021-09-08 14:36:13 +01:00
Ben Thorner
04d8678c27 Make it easy to develop with broadcast services
Previously I had to handcraft some SQL to give myself access to a
broadcast service I created locally. I've done this enough times
that I think it's worth automating.
2021-09-08 09:45:11 +01:00
Ben Thorner
187e58d8ae Merge pull request #3321 from alphagov/remove-remaining-areas-migration-178986763
Remove remaining areas migration scaffolding
2021-09-08 09:40:53 +01:00
Ben Thorner
8e810259af Merge pull request #3324 from alphagov/remove-migration-command-178986763
Remove redundant command to migrate area data
2021-09-08 09:40:47 +01:00
Ben Thorner
54808104a6 Stop defaulting simple_polygons to empty array
This is now done by the Admin app [1].

[1]: baf20e0075
2021-09-07 17:16:24 +01:00
Ben Thorner
59d0ab4f65 Stop defaulting "ids" to an empty array
This is so we can distinguish custom broadcasts in the Admin app
[1]. I've also extended the POST test for custom broadcasts to
check we're correctly reading data for "names", as this wasn't
being tested previously.

[1]: 411fda81c0
2021-09-07 17:16:22 +01:00
Ben Thorner
dd41cf854c Remove support for old "areas" sub-field
All broadcasts with this field have now been migrated to use "ids".

This also removes a few lines that were missed in previous PRs:

- Added by mistake: fd7ebbebb0 (diff-045554136e1462693a6cbb6328b2e056a81e8b348e94575edd8f72b78c5da96eR115)
- Missed removal: ec1171f85c (diff-045554136e1462693a6cbb6328b2e056a81e8b348e94575edd8f72b78c5da96eR110)
2021-09-07 17:14:57 +01:00
Ben Thorner
406e7e9aa1 Remove redundant command to migrate area data
The migration has been done.
2021-09-07 13:32:01 +01:00
Ben Thorner
b2b921ef21 Merge pull request #3322 from alphagov/migrate-custom-broadcasts-178986763
Add command to migrate custom broadcast areas
2021-09-07 12:20:53 +01:00
Ben Thorner
b639031b9f Add command to migrate custom broadcast areas
Unlike broadcasts created in the Admin app, these are only expected
to have "names" and "simple_polygons" in their areas column [1].

The migration command in the Admin app [2] isn't suitable for these
broadcasts as it would try to aggregate their areas, etc.

I've put a conditional on "areas" being present (in the areas column)
so this command doesn't pick up any new custom broadcasts.

[1]: 023a06d5fbo
[2]: https://github.com/alphagov/notifications-admin/pull/4011
2021-09-06 18:15:32 +01:00
Ben Thorner
6af39c4d3b Remove redundant force_overrride feature
This is no longer needed as all areas data has now been migrated.
2021-09-06 09:53:39 +01:00
Ben Thorner
3779146cc5 Merge pull request #3317 from alphagov/remove-areas-2-support-178986763
Remove support for "areas_2" field
2021-09-03 11:16:40 +01:00
Ben Thorner
d50c563f08 Remove support for "areas_2" field
The Admin app was only using this temporarily and is now using the
"areas" field instead [1], so we can delete this one.

[1]: https://github.com/alphagov/notifications-admin/pull/4006
2021-09-01 17:42:15 +01:00
Ben Thorner
43ddfe0560 Remove old "simple_polygons" fields in schemas
These were missed in [1].

[1]: https://github.com/alphagov/notifications-api/pull/3314
2021-09-01 17:23:29 +01:00
Ben Thorner
be7272b44f Merge pull request #3314 from alphagov/next-stage-areas-migration-178986763
Switch "areas" field to "areas_2" format
2021-09-01 16:16:30 +01:00
Chris Hill-Scott
6a2cd7e9bf Merge pull request #3307 from alphagov/remove-email-address-validate-password-reset
Don’t update `email_access_validated_at` on password reset
2021-09-01 10:06:48 +01:00
Chris Hill-Scott
2c7e4657ce Don’t update email_access_validated_at on password reset
As of https://github.com/alphagov/notifications-admin/pull/4000/files
the admin app is doing this, so we don’t need to do it here as well.
2021-09-01 09:54:54 +01:00
Chris Hill-Scott
43f010ea1b Merge pull request #3315 from alphagov/no-0-weight
Don’t call random.choices with zero weighting
2021-08-31 12:06:03 +01:00
Chris Hill-Scott
6900505b05 Don’t call random.choices with zero weighting
As of 041d8b48a2
it’s not valid to call `random.choices` without giving at least one of
the options a positive weighting.

This makes sense, because giving a zero weighting is effectively saying
‘theres’s only one choice, but don’t choose it’.

In our codebase this is applicable where there’s only one international
provider, which we want to use even when it’s been de-prioritised for
domestic SMS.

This doesn’t cause a problem now, but will if we upgrade to Python
versions greater than 3.9.0.
2021-08-31 11:08:27 +01:00
Ben Thorner
bf0bf4e31c Favour new "areas" format for PagerDuty alerts
Broadcasts created via the API [1] and the Admin app [2] should
both now have this field set. It's also more informative to show
this, and broadcasts created via the API don't have IDs anyway.

There's a small risk that an old broadcast that gets approved won't
have this data, but it's for information only and we intend to
backfill all old broadcasts in the near future.

[1]: 023a06d5fb
[2]: 7dbe3afa19
2021-08-27 14:22:12 +01:00