feat: add import/export of roles with permissions #1662
Conversation
|
@dpgaspar regarding builtin roles, I can see that they get inserted into the database when the Flask-AppBuilder/flask_appbuilder/security/manager.py Lines 778 to 779 in b9b3259 So as long as the user initializes the app or runs it, the builtin roles will be in the database as well and will get exported. I tried adding an entry for FAB_ROLES in config.py and running flask run followed by flask fab export-roles and the resulting file had the builtin roles included as well.
|
krsnik93
force-pushed
the
export-roles
branch
3 times, most recently
from
3926388 to
0e27c1c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1662 +/- ##
==========================================
+ Coverage 75.45% 75.83% +0.38%
==========================================
Files 54 55 +1
Lines 7870 8025 +155
==========================================
+ Hits 5938 6086 +148
- Misses 1932 1939 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
|
||
| export_result = src_cli_runner.invoke(export_roles, [f"--path={filename}"]) | ||
| self.assertEqual(export_result.exit_code, 0) | ||
| self.assertTrue(os.path.exists(filename)) |
There was a problem hiding this comment.
Would be great if we could assert the content of the file also.
nit: can you split these into 2 distinct tests? by assert the content you can add the file has a fixture
| self.assertEqual( | ||
| len(dst_app_builder.sm.get_all_roles()), | ||
| len(src_app_builder.sm.get_all_roles()), | ||
| ) |
There was a problem hiding this comment.
can you also assert the permissions on the DB after the import?
krsnik93
force-pushed
the
export-roles
branch
3 times, most recently
from
5c25d3e to
b52bf62
Compare
|
@dpgaspar I think there might be some issues with the code coverage tool as it flags lines that are certainly covered... |
There was a problem hiding this comment.
LGTM!
Would be great if you could drop a few lines here: https://flask-appbuilder.readthedocs.io/en/latest/cli.html
|
@dpgaspar do you agree about the coverage being off or should I take a look? |
|
@krsnik93 something is strange, I'm not finding your tests on: https://pipelines.actions.githubusercontent.com/EcVmXJ9QlFNYqdgzb19Iib0zlAKatWUkmR9lWHntlezNeYEMGz/_apis/pipelines/1/runs/716/signedlogcontent/13?urlExpires=2021-07-06T12%3A29%3A02.1318847Z&urlSigningMethod=HMACV1&urlSignature=FzwDirCfFm%2FHEn%2Bzk4HTwZu%2FStetzk99ZRo5YR4xl3o%3D Can you take a look, ping me if you can't figure what's wrong |
|
Hi @dpgaspar the link does not work for me, says "Uri expired" |
|
@dpgaspar you are right that |
|
@dpgaspar looking good now |
|
@krsnik93 great work! |
Description
This PR introduces two new CLI commands:
flask fab export-rolesflask fab import-rolesIn combination, they allow for quick migration of custom roles with permissions from one host to another.
I initially tried using
Model2SchemaConverterto create schemas forsqlamodels. After making a couple of changes, I was able to use it to serialize nested models. However, I was not able to de-serialize back from this result, so I resorted to creating individualmarshmallowschemas.I don't think
export_rolesandimport_rolesshould live inSecurityManagerclasses, but not sure where to put them. Would appreciate feedback.ADDITIONAL INFORMATION