Change http response code for missing-legalhold-consent.

[fisx]

412 is used for "need to add some clients" in message posting; 403 makes client implementations more straight-forward.

It's probably worth mentioning that this only affects users about to get in contact with LH devices. For those users, we have a new safety check in place that if the user has old clients or is not in a team cleared for LH, she will get a new error response. 403 in that case is better, since it makes it clear to the client that this won't work on retry either. 412 is used for notifying the client that they need to pick up some newly added clients listening in on the conversation, so this will probably break in less nice ways.

tl;dr: only affects users in touch with LH teams that shouldn't, and makes their UX strictly better. :)

Checklist

• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• If end-points have been added or changed: the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
• If a schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
• Section Unreleased of CHANGELOG.md contains the following bits of information:
  • A line with the title and number of the PR in one or more suitable sub-sections.
  • If /a: measures to be taken by instance operators.
  • If /a: list of cassandra migrations.
  • If public end-points have been changed or added: does nginz need upgrade?
  • If internal end-points have been added or changed: which services have to be deployed in a specific order?

[pcapriotti]

Looks good, and it does make more sense to return 403 instead of 412. Minor suggestion below.

[pcapriotti]

To avoid this duplication, you could turn this into an ErrorDescription and move it to the ErrorDescription module in wire-api, where it can then be used from both brig and galley. Also, it makes it easier to add it to swagger, if desired, or even later turn it into a statically checked response with MultiVerb.

I've done this for a bunch of errors as part of #1657. This is a typical example: 8e73769. It can be a bit of work though, depending on how often the error is thrown, so feel free to ignore this.

[fisx]

thanks, i'll look into it.

[fisx]

(why is nothing ever easy? :-))

[fisx]

I've solved it like this: 83abb9c

I've made you the commit author @pcapriotti, let me know if that's not ok and I'll change it back.

[pcapriotti]

Sure, that's fine.

[pcapriotti]

FYI: #1693 contains the change I was suggesting, since it was useful for another refactoring.

[fisx]

            User cannot fetch prekeys of LH users if consent is missing:                                                                      FAIL
              Exception: Assertions failed:
               1: 403 =/= 500
               2: Right (TestErrorLabel {fromTestErrorLabel = "internal-server-error"}) =/= Right (TestErrorLabel {fromTestErrorLabel = "missing-legalhold-consent"})
              
              Response was:
              
              Response {responseStatus = Status {statusCode = 500, statusMessage = "Internal Server Error"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Fri, 30 Jul 2021 15:15:03 GMT"),("Server","Warp/3.3.13"),("Content-Encoding","gzip"),("Content-Type","application/json")], responseBody = Just "{\"code\":500,\"message\":\"Internal Server Error\",\"label\":\"internal-server-error\"}", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}
              CallStack (from HasCallStack):
                error, called at src/Bilge/Assert.hs:89:5 in bilge-0.22.0-KwPXBVYHFlF3OFbmYHfwv3:Bilge.Assert
                <!!, called at src/Bilge/Assert.hs:107:19 in bilge-0.22.0-KwPXBVYHFlF3OFbmYHfwv3:Bilge.Assert
                !!!, called at test/integration/API/Teams/LegalHold.hs:1176:9 in main:API.Teams.LegalHold

seems like i need to do some more integration test fixing...