tenant-permission tests #1694
tenant-permission tests #1694
Conversation
dlpzx
changed the title
tests/test_tenant_unauthorized.py
Outdated
| CHECK_PERMS = ['archiveOrganization'] | ||
|
|
||
|
|
||
| @fixture(scope='function') |
There was a problem hiding this comment.
nit: an alternative and maybe more flexible version of this doesn't require a fixture...
@pytest.mark.parametrize('field_name,resolver', ...)
def test_generic(field_name, resolver, db, client, org1, groupNoTenantPermissions, userNoTenantPermissions):
local = MagicMock()
local.context = RequestContext(db, userNoTenantPermissions.username, [groupNoTenantPermissions.groupUri], userNoTenantPermissions)
with patch('dataall.base.context._request_storage', local):
# execute test command
tests/test_tenant_unauthorized.py
Outdated
| (f'{_type.name}.{field.name}', field.resolver) | ||
| for _type in bootstrap().types | ||
| for field in _type.fields | ||
| if field.resolver and _type.name in ['Mutation', 'Query'] and field.name in CHECK_PERMS |
There was a problem hiding this comment.
I think we should inverse the for-loop and make pytest fail in case any of the listed CHECK_PERMS queries are missing. i.e...
ALL_RESOLVERS = {
f'{_type.name}.{field.name}': field.resolver
for _type in bootstrap().types
for field in _type.fields
if field.resolver
}
CHECK_PERMS = [
'Mutation.archiveOrganization',
'Mutation.createOrganization',
]
@pytest.mark.parametrize('field_name,resolver', [(perm, ALL_RESOLVERS[perm]) for perm in CHECK_PERMS])There was a problem hiding this comment.
I implemented something similar in 75d30c1 adding an assertion to check that the resolver does not exist but continue for other tests
dlpzx
force-pushed
the
feat/tenant-permission-tests
branch
from
e24bf54
to
75d30c1
Compare
### Feature or Bugfix - Refactoring ### Detail Moved business logic of Worksheets to service layer. Needed #1694 ### Relates #1694 ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
|
@petrkalos I have intentionally left the commented mutations and opened the PR to review. I think that the commented mutations will require some changes and I don't want to make the PR too big. If you think we should tackle all at once I am happy to keep working on this PR |
| 'Mutation.createNetwork', | ||
| 'Mutation.deleteNetwork', |
There was a problem hiding this comment.
is this to say Operations like createNetwork and deleteNetwork do not have MANAGE_XXXX checks on them? They do have MANAGE_ENVIRONMENTS ... and I think a handful of tohers on this list are simialr
Or are these mutations that would need some additional work on the backend app logic side to conform to this pattern?
Feature or Bugfix
Detail
Tests that verify that MANAGE_ORGANIZATION permissions are used on all write APIs for organizations
Relates
Security
Please answer the questions below briefly where applicable, or write
N/A. Based onOWASP 10.
fetching data from storage outside the application (e.g. a database, an S3 bucket)?
evalor similar functions are used?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.