[uss_qualifier/scenarios/netrid/misbehavior] Add checks for SP too large area search (NET0250) #873
Conversation
|
|
||
| return mapping_by_injection_id | ||
|
|
||
| def _evaluate_and_test_invalid_requests( |
There was a problem hiding this comment.
Nitpick: I'd keep 'unauthenticated' in the function name for the sake of clarity (unless you think that this would convey something wrong about what the function actually does)
monitoring/uss_qualifier/scenarios/astm/netrid/common/misbehavior.py
Outdated
Show resolved
Hide resolved
| details=f"Queried flights on {flights_url} for USS {participant_id} with no credentials, expected a failure but got a success reply.", | ||
| summary="Did not receive expected error code for too-large area request", | ||
| severity=Severity.High, | ||
| details=f"{participant_id} was queried for flights in {geo.rect_str(rect)} with a diagonal of {diagonal_km} which is larger than the maximum allowed diagonal of {self._rid_version.max_diagonal_km}. The expected error code is 413, but instead code {uss_flights_query.status_code} was received.", |
There was a problem hiding this comment.
Nit: expected error may also be 400
There was a problem hiding this comment.
I think this would deserve a proper PR. Currently, 400 are silently accepted on various other cases. I will leave it as is for the moment and bring the topic to the Contributors meeting.
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| ## Overview | |||
|
|
|||
| In this scenario, the service provider's endpoint are accessed directly with missing or incorrect credentials. Resources that exists as well as resources that are not expected to exist are queried. | |||
| In this scenario, the service provider's endpoints are accessed directly to test missing or incorrect credentials as well as invalid requests. Resources that exists as well as resources that are not expected to exist are queried. | |||
There was a problem hiding this comment.
Nit: I don't think we actually test for incorrect credentials in this particular scenario. We could reflect that in the description, or add an issue requesting that we enrich the scenario
(also applies to v19 version)
There was a problem hiding this comment.
I removed the misleading description and created an issue to address this specifically: #874
|
|
||
| #### Area too large check | ||
|
|
||
| **[astm.f3411.v19.NET0250](../../../../requirements/astm/f3411/v19.md)** requires that a NetRID Service Provider rejects a request for a very large view area with a diagonal greater than *NetMaxDisplayAreaDiagonal*. If such a large view is requested and a 413 error code is not received or the response contains Remote ID data, then this check will fail. |
There was a problem hiding this comment.
Nit: check documentation can mention that 400 is an acceptable return code (same for v22a)
|
|
||
| self.record_note( | ||
| f"{participant_id}/{injection_id}/area_too_large_query", | ||
| f"Will attempt to search an area too large at {flights_url} - (diagonal: {diagonal_km} km)", |
There was a problem hiding this comment.
Is this note really needed? That's meant for info useful to appear in the report. And this info is already present in the check failure.
| if uss_flights_query.status_code not in (400, 413): | ||
| check.record_failed( | ||
| summary="Did not receive expected error code for too-large area request", | ||
| severity=Severity.High, |
There was a problem hiding this comment.
We should be specifying the severity through the emoji in the .md, this parameter is meant to be deprecated.
| check.record_failed( | ||
| summary="Received Remote ID data while an empty response was expected because the requested area was too large", | ||
| severity=Severity.High, | ||
| details=f"{participant_id} was queried for flights in {_rect_str(rect)} with a diagonal of {diagonal_km} which is larger than the maximum allowed diagonal of {self._rid_version.max_diagonal_km}. The Remote ID data shall be empty, instead, the following payload was received: {uss_flights_query.query.response.content}", |
There was a problem hiding this comment.
nit: Include the flights in failure message instead of full response content?
This PR adds a test step to the RID misbehavior scenario to finalize checking NET0250 started in #821.
Note that to keep the scenario steps simple and distinct, the service providers flights urls are fetched two times from the DSS during the scenario. An alternative would have been to load them in a separate step which would have required a polling step which is more expensive in terms of time than calling twice the DSS.
Merging the
unauthenticatedandinvalid search areasteps have been considered, however it would have required to either merge the checks in a single step, or check results after polling and capturing the information, which introduces a certain level of complexity, which do not seem to be required at this point. Clarity of the test scenario has been privileged. Happy to revisit this based on reviewers input.