Feature/manage databases #250
Conversation
michaeljcollinsuk
force-pushed
the
feature/manage-databases
branch
from
fe49773 to
9a301b2
Compare
michaeljcollinsuk
force-pushed
the
feature/manage-databases
branch
from
beecc7d to
9c5e9d4
Compare
There was a problem hiding this comment.
Have tested locally. Assigned myself as a super user and could assign permissions, then see the data in quicksight. Then revoked permissions and could no longer see table.
A couple of questions.
Will a user who doesn't have grantable permissions be able to see the database access tab? If not, I could see the tab as a non-superuser.
If they should be able to see it, you may want to hide the grant permissions button from those who shouldn't be able to grant permissions.
For granting/revoking lf opt in permissions, for safety, it may be worth implementing something similar to what's in the following PR. You can list the opt ins of a principal and use the database name to decide what can be revoked or added. There may be a better way to code it but it at least gives us a bit of security when performing opt in changes.
Other than that, fantastic work!
Yes, currently they will be able to see the tab and also see all the databases - is this what you saw. I was thinking of adding a tab so you can switch between dbs/tables you have access to and all dbs. But didnt get round to it.
Agreed - another thing I wanted to add but didnt get round to/forgot to do
Is this specifically about removing the hybrid mode opt-in? I'm a little unsure about this so might be better to discuss. But my thinking is that it shouldnt be necessary as when updating permissions, the opt in is deleted, and then reapplied. And when the access is revoked we always know that we need to revoke the opt-in also. |
Methods to grant access to databases and tables. Also update glue service to add method to return target table data.
A user should only be able to grant access to another user that they themselves have. If the user is a superuser, they can grant any access.
This reverts commit 333b0a6.
Dont save the model if the API call fails. Also use the values from the selected accesslevel model when making API call.
Move logic away from model save methods, and on to methods on the model that are called by the access forms.
Use DB transactions to keep the database state in sync with lake formation permissions. Add validation to check that selected grantable are valid based on standard permissions.
Refactors code to remove need for duplicate access levels with grantable flag.
Remove the grantable field as no longer necessary
When a user is granted access to a table, grant the user DESCRIBE access to the database with grantable permission. This is because having access to describe the database only allows you to see the name of the DB.
Only show the actions column to manage/revoke table access if the user is either a superuser, or has grantable permissions on the table.
michaeljcollinsuk
force-pushed
the
feature/manage-databases
branch
from
090cca8 to
680dc11
Compare
|
Merging despite the failed scan. See bug ticket ministryofjustice/analytical-platform#5251 |
Adds ability for users to grant access to databases. This initial implementation allows:
To run locally:
python manage.py migratemake serve-ssoor use the debugger in VS codeLimitations:
TODO/next steps: