-
Notifications
You must be signed in to change notification settings - Fork 0
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
Custom audits #697
Merged
Merged
Custom audits #697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We'll be adding other audits, so it's clearer to have them in separate source files in an audit-specific directory.
The logging level for "root" is DEBUG in base.py, which makes pytest tests run with -s print out lots of unhelpful debug messages. Set the logging level in test.py to be INFO instead of DEBUG.
They are far from complete.
This means that the writers group associated with the RC of the workspace should be granted can_compute access. This indicator can be used in the audit.
These are determined based on the start and end dates.
This makes it easier to test the audits - we don't have to figure out the start and end dates for each upload cycle created in the audit tests. We can just use a parameter to set them appropriately.
Instead of running a sub-method based on temporality of the upload cycle, define sub-methods based on the group type. This will make the logic clearer. Add the sub-method for auditing the RC member group.
The field tracks the date when QC was completed. This will also be used in audits.
Previously I had started with the RC member group, but an upload workspace should never be shared directly with the member group; that will happen via sharing with the auth domain. Also start redoing tests such that the tests are split into different classes by UploadWorkspace stage. This is a little clearer to program and test, because different sharing is expected at different stages. The audits themselves are still set up to run sub-methods by group type, though.
Note that this is before the combined workspace is shared; to add those checks, we'll need to add a flag on the upload cycle.
This field tracks if the data in the combined workspace is completed and ready to be shared with the consortium.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #697 +/- ##
==========================================
+ Coverage 94.89% 97.93% +3.03%
==========================================
Files 130 149 +19
Lines 6588 15900 +9312
==========================================
+ Hits 6252 15572 +9320
+ Misses 336 328 -8 ☔ View full report in Codecov by Sentry. |
They should still have read access by being in the members group in the auth domain.
This one is easy - it should always be "read" access.
This is also easy - DCC admins should always be an owner.
Any other group not covered in the audit should never have direct access.
There are no tests for this view yet, since I'm not done writing the audit class itself. However, we can use this to check the audit against prod data.
The audit will just ignore these groups.
Add a foreign key in RC to the ManagedGroup model to link a non-member group (similar to what we've done for member and uploader groups). Show the non-member group on the detail page.
These tests call the "run_audit" method instead of the method that audits a specific workspace and group. The run_audit flow handles selecting the proper groups.
Right now this button is just a link to a page that resolves the audit. Also add or make some modifications to templates (still not final).
This is a better indicating that the access and role that are shown are the current access/role, not the access/role that they should be.
Like the workspace sharing, create auth domain memberships such as they would be at the previous step of the cycle.
Remove the previous migrations added into this branch and regenerate them, with all changes in a single migration.
Instead of using the date for sharing with GREGOR_ALL, use the date when the workspace was shared with its auth domain. This is how we're actually sharing the workspace - GREGOR_ALL is part of the auth domain and the workspace is then shared with its auth domain.
For groups that should only ever have read/write permission, change the audit results to errors when the group has OWNER permission. This is more sensible than having a "needs action" result, since this case should never occur.
When run_upload_workspace_audit detects an error, it now prints out a link to the page to audit sharing/auth domain membership for all UploadWorkspaces.
We'll add the new management command as a cron job after testing the audits out for a bit. |
Accidentally had an old data_access_audit in the template, so some records were not getting rendered.
I accidentally put a test in the wrong class; move it to the UploadCycleDetailTest class, and fix the code so that it creates the object instead of relying on the test class to have it.
RC uploaders should not have access until the upload cycle starts, and should lose access after QC is complete. Note that RC uploaders typically will have access via being in the member group. This commit likely breaks auditing for RC members groups, since we had to split the notes into separate notes for members vs. uploaders due to different access at different stages.
The RC members group should not be added to the auth domain until after the upload cycle starts.
RC non-members should only have access to the workspace after an upload cycle starts.
Mostly these affected the timing of the upload cycle and when to grant writer access to RC uploaders.
2 weeks and 4 weeks, respectively, are more in line with approximate timeframes for past upload cycles.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To enable some of these audits to be done programmatically, I added more tracking in ACM:
date_ready_for_compute
for an UploadCycle indicates that the data model is finalized and compute can proceed in associated UploadWorkspacesdate_qc_completed
field for UploadWorkspaces indicates if QC in that workspace is completedate_completed
field for ConsortiumCombinedDataWorkspaces indicates if the combined workspace is complete and ready to shareStill to do for UploadWorkspace sharing:
To do for UploadWorkspace auth domain membership auditing:
Other to dos:
is_ready_for_compute
date_qc_completed
for UploadWorkspacesdate_completed
for Combined workspacesCloses #688