-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[uss_qualifier] expand the authentication validation scenario to include constraint reference endpoints #690
Conversation
9db0eca
to
a26fbdc
Compare
a26fbdc
to
9bb0ab2
Compare
- `utm.constraint_management` | ||
- `utm.constraint_processing` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making these required makes this scenario unusable in jurisdictions that don't use these features (which is all of them I'm aware of right now). Probably the most important CI configuration currently is utm_implementation_us as it parallels a test configuration in active use for actual regulatory compliance in the US, and this causes the entire scenario to be skipped in that configuration. We should probably add a validation criterion to that configuration (and probably others) that the count of skipped actions doesn't exceed the number of actions we expect to be skipped -- that would catch newly-skipped scenarios like this.
Each of these scopes should be optional (and then the validation for them not performed if not provided), or else split this scenario into strategic coordination (existing), constraint management, and constraint processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, that's an oversight, thank you for pointing it out. I'll:
- make the relevant scopes (
utm.constraint_management
,utm.constraint_processing
) optional and make sureutm_implementation_us
properly runs - possibly make the required scope for any endpoint group optional
- add a validation criterion to
utm_implementation_us
@@ -317,6 +318,146 @@ it is in violation of **[astm.f3548.v21.DSS0210,A2-7-2,7](../../../../../require | |||
If the DSS does not allow searching for operational intents when valid credentials are presented, | |||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. | |||
|
|||
### Constraint reference endpoints authentication test step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not the highest priority for adjustment, but It seems like the "steps" in this scenario should actually be "cases". A step is supposed to be a single logical action (do one thing), reflecting what a manual checkout test card would look like ("(step 1) Ok, now we're going to X -- did things Q and P happen? (step 2) Now we're going Y -- did things R and S happen?"). I would imagine a good test case would be "test authentication of all the constraint management endpoints", and then each testing of each constraint management endpoint would be a test step (or, testing of each constraint management endpoint in a particular way would be a test step).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense. it might be possible to easily do this as part of this PR, as I need to handle the optional utm.constraint_processing
scopes, but I'm not entirely sure yet.
(Replacing the current steps with a test case having a single step would be easy in any case: I might keep splitting each test case into proper steps for later)
I created #742 to keep track of it in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the current priorities I'll leave the splitting into separate test cases for later.
0e0cf17
to
ed0ff98
Compare
Updated the PR: the various endpoint groups are now only optionally tested when the relevant scopes are available. The scenario now runs as part of the
|
ed0ff98
to
db7385b
Compare
db7385b
to
427263f
Compare
@BenjaminPelletier this PR ends up mixing two or three things now, some of them have been split:
(Will reopen once they are merged and integrated) |
427263f
to
dfa2049
Compare
dfa2049
to
ed3c9a9
Compare
Ready for review: locally, using the f3548_self_contained configuration, all endpoints are tested:
|
ed3c9a9
to
857279e
Compare
857279e
to
b465580
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments, otherwise LGTM.
#### 🛑 Create constraint reference with valid credentials check | ||
|
||
If the DSS does not allow the creation of a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be:
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. | |
it is in violation of **[astm.f3548.v21.DSS0005,3](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
#### 🛑 Get constraint reference with valid credentials check | ||
|
||
If the DSS does not allow fetching a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. | |
it is in violation of **[astm.f3548.v21.DSS0005,3](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: no checking of the get response format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Added to the validator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
monitoring/uss_qualifier/scenarios/astm/utm/dss/authentication/authentication_validation.md
Outdated
Show resolved
Hide resolved
#### 🛑 Delete constraint reference with valid credentials check | ||
|
||
If the DSS does not allow the deletion of a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. | |
it is in violation of **[astm.f3548.v21.DSS0005,3](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, no checking of delete response format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
monitoring/uss_qualifier/scenarios/astm/utm/dss/authentication/authentication_validation.md
Outdated
Show resolved
Hide resolved
monitoring/uss_qualifier/scenarios/astm/utm/dss/authentication/cr_api_validator.py
Show resolved
Hide resolved
f435b47
to
09b67b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix the missed requirements? Otherwise LGTM thanks!
#### 🛑 Delete constraint reference with valid credentials check | ||
|
||
If the DSS does not allow the deletion of a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
#### 🛑 Get constraint reference with valid credentials check | ||
|
||
If the DSS does not allow fetching a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
#### 🛑 Create constraint reference with valid credentials check | ||
|
||
If the DSS does not allow the creation of a constraint reference when valid credentials are presented, | ||
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../requirements/astm/f3548/v21.md)**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1?
Thanks for catching them, they did not make it into the previous commit. Should be correct now. |
This is heavily inspired from the existing validation logic for operational intent references.
Some additional updates:
utm_implementation_us
configuration to make the expected number of skipped scenarios explicit, which should allow us to detect if scenarios suddenly start being skipped when we don't expect it.Note: this also moves the instantiations of the authentication validator classes just before each validator is run, as they depend on the current time.