-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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(gitlab): vcs gitlab groups retrieval #3716
Conversation
if group.get("require_two_factor_authentication", False) is True: | ||
return CheckResult.PASSED, conf | ||
return CheckResult.FAILED, conf | ||
if group.get("require_two_factor_authentication") is False: |
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 think that maybe require_two_factor_authentication
is not a required field (although the schema down here states it is required - from the docs: https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/api/groups.md)
Hence in case there is no require_two_factor_authentication
the schema will be filtered and will not get here
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.
Looks like the docs state that require_two_factor_authentication
is not mandatory on PUT/POST operations, since it defaults to False. But on retrieval it is always returned.
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.
You are correct! missed the difference :)
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.
☕
Description
This PR fixes an issue in vcs gitlab groups retrieval. One part is in schema validation, and the second in the logic of failing checks.
Checklist: