-
Notifications
You must be signed in to change notification settings - Fork 138
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
Fix test for request conflict with SSNv2 #4898
Conversation
EOF | ||
|
||
diff expected stderr | ||
--output-file testuser.crt | ||
|
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.
A request can be used to generate multiple certificates. If a request is provided and it match with the identified record, is there a verification? I mean in this case the csr is the same to the one recorded, it could be OK but If the csr does not match, which one is used? Is there a conflict?
I think this command should use a different CSR, and verify if it works as expected, which for me it should generate an error but other results could be valid.
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.
While a CSR can be reused for multiple enrollments, a request record in DS is meant to store the info associated to a single enrollment, e.g. the CSR submitted for that enrollment, the enrollment status, the enrollment completion date. It also serves as historical records. We've also been using the same CSR in the existing SSN tests and the CA always creates a new request record for each enrollment.
Please see the updated PR. I've updated the PR to use a new CSR to test the conflict, then check the request record after the enrollment, and it shows that the request record has changed. I think this is definitely a bug because now we've lost the original info in it.
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.
Yes, overwrite the request is a bug. I am wondering if this behaviour is the same for RSNv3. If the request exist it gets overwritten. It should be a very rare situation but to investigate. I am expecting the same behaviour for requests and certs. Fortunately, the certs do not get overwritten.
The fix to this is related to the handling of the error in case of range overlap since there should be no other situations where this condition should occur. Maybe we could open an issue just to remember the problem.
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.
Yes, I think it will affect RSNv3 and also KRA too. I opened ticket #4899 for this issue.
The test for request ID conflict with SSNv2 has been updated to create a duplicate request record with the proper requestId attribute. With this change the CLI is no longer failing, but the CA is still updating the duplicate request record instead of creating a new one as expected.
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.
LGTM
Quality Gate passedIssues Measures |
@fmarco76 Thanks! |
The test for request conflict with SSNv2 has been updated to create a duplicate request with the proper
requestId
attribute. With this change the CLI is no longer failing, but it still does not create a new request as expected.