Update error codes, messages for grant expiry

[jadudm]

PR in draft for viewing diffs, discussion, etc. Will update draft text shortly.

JIRA Ticket:

• BB2-3810
• BB2-4189

What Does This PR Do?

Users of the API noted that we return 400 errors in a number of contexts, and in some cases, return the same text for an error message when that error has different root causes.

This is particularly notable in the function validate_app_is_active. This PR makes the following changes:

1. Converts 400 (Bad Request) to 403 (Forbidden) errors.
2. Adds unit tests to specifically cover these pathways, and confirm that the expected error codes/messages are now coming back.
3. Introduces the use of symbolic constants for status codes from the http library instead of numeric codes (e.g. HTTPStatus.FORBIDDEN instead of 403)
4. Adds a new constant in the app.constants.py file for AccessType, so that AccessType.THIRTEEN_MONTH can be used instead of the string "THIRTEEN_MONTH" (again, symbolic constants vs. repeated use of values)
5. Updates existing unit tests to expect HTTPStatus.FORBIDDEN instead of HTTPStatus.BAD_REQUEST where appropriate.

What Should Reviewers Watch For?

All offline tests should pass. This PR extends unit tests to explicitly exercise this pathway via mocks.

External testing with synthetic data would likely require running Postman queries, modifying the database (e.g. to expire grants artificially), and confirming that the error codes come back as expected. This does not feel more rigorous than the unit testing in this case.

Validation

TBD

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

• Adds any new software dependencies
• Modifies any security controls
• Adds new transmission or storage of data
• Any other changes that could possibly affect security?

• Yes, one or more of the above security implications apply. This PR must not be merged without the ISSO or team
  security engineer's approval.

The implications of changing the error messaging has been discussed, and we have decided as a team that this work does not introduce any new security concerns.

Any Migrations?

No migrations.

[jadudm]

This relied on a default behavior before. What was the default behavior of InvalidClientError in terms of a status code?

[jimmyfagan]

If it just gets raised and not caught until the Django library code, then it's probably a 500. I'd have to force this case to confirm, but if that's the case, it would make sense to update that too.

[jadudm]

The default status code for InvalidClientError is a 401, I think. If not caught, yes: it would be a 500. I think this was updated to reflect our discussion so it would be consistent/in keeping with the other return values (e.g. 403/Forbidden).

[JamesDemeryNava]

Looks good, I need to review some of your comments.

Couple nits: Some "" are present, and can we include the ticket numbers in the PR title so that when the release is cut it's clearer what tickets were included?

[jimmyfagan]

(Late night review brought to you by working late on schoolwork and remembering right before going to sleep that I never submitted this) Really great stuff here, especially with those unit tests. I had a handful of minor comments, mostly related to code comments. In general, I think for simple sections of code we can let the code speak for itself a bit more to avoid some of these issues of having comments and code out of sync with each other.

One additional general ask is to revisit some of the AC and description on BB2-4189. I still saw curl -X POST "https://sandbox.bluebutton.cms.gov/v2/o/token/" -u "<client_id>:<client_secret>" -d "grant_type=refresh_token" return a 403, when that is a case where a 400 would actually be appropriate since the refresh_token param is missing. I imagine that might involve some additional tests too. BB2-4189 does deal more with bad inputs than BB2-3810 which is more legitimately expected cases with completely valid and correct requests, so if it makes sense to split the two tickets into separate PRs, feel free to do so and repurpose this to be more specific to 3810.