-
Notifications
You must be signed in to change notification settings - Fork 28
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
GWS Policy API Updates for Common Controls and Gmail #538
Conversation
864469b
to
8715985
Compare
484e88d
to
67abc58
Compare
454cbdf
to
67abc58
Compare
@rlxdev Why isn't Common Controls 1.4 included in this? I'm all for splitting big updates into multiple PRs, but it doesn't look like we have a separate issue for that. The policy API does support checking it, the logic is the same as what you have for 1.1, just trading line 289 with |
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.
Tested and works as expected. My only ask is that you open up an issue to track updating 1.4 since it's ready to be updated.
If I understand the Common Controls 1.4 baseline correctly, the criterion for meeting the requirement is to allow users to turn on 2-step verification with security key only or non-telephony OR passwordless authentication. Google has not provided the passwordless authentication setting in the Policy API yet, so the baseline can only be partially implemented. This is one of the baselines in the Policy API tracker for Google. I didn’t change the Rego code for this reason, deciding to wait for Google until they provide the passwordless authentication settings in the API results. |
This looks like a conversation we'll want to queue up for later to clarify the baseline, my understanding is that 1.4 just boiled down to "don't use SMS or voice as a second factor." That's certainly all that the Rego is currently checking and is consistent with the equivalent M365 control. But after reviewing the implementation steps, I can see where you're coming from. |
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.
Tested and works as expected, only recommendation is the naming convention and function names - follow standard naming methodology.
Approving the PR as the functionality works as expected, recommend the naming targeted in the upcoming updates.
scubagoggles/Testing/RegoTests/commoncontrols/commoncontrols_api01_test.rego
Show resolved
Hide resolved
scubagoggles/Testing/RegoTests/commoncontrols/commoncontrols_api01_test.rego
Show resolved
Hide resolved
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.
Created new report, checked each policy with updated api setting result in details section. Ran all unit tests with no issue.
🗣 Description
Google released more settings in the Policy API subsequent to the initial PR for the ScubaGoggles Policy API integration. The settings have allowed the following policies to be implemented as part of this PR:
Closes #519
🧪 Testing
Rego tests were added for the new implementations detailed above. They all include testing for the
compliant and non-compliant cases. The following test files have been added:
In addition to these tests, testing was done on each affected baseline using the scubagws Admin UI .
✅ Pre-approval checklist
✅ Pre-merge Checklist
Squash and merge
button.✅ Post-merge Checklist