[Gh-1528] Configurable stack logs display #1559
Conversation
backend/dataall/modules/shares_base/services/share_logs_service.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @TejasRGitHub thank you so much for the cool feature! I left some comments and suggestions!
|
Hi @dlpzx , Thanks a lot for the detailed review and suggestions . I have incorporated those changes and updated code. Please let me know if this looks good now |
|
@dlpzx , updated PR based on comments in the PR. The |
backend/dataall/modules/shares_base/services/share_logs_service.py
Outdated
Show resolved
Hide resolved
| default_value='enabled', | ||
| config_property='modules.shares_base.features.show_share_logs', | ||
| ) | ||
| def check_view_logs_permissions(username, groups, shareUri): |
There was a problem hiding this comment.
nit: we do not need to pass the username and groups as argument of check_view... We can get them directly from the context object as context.groups and context.username
There was a problem hiding this comment.
Hi Tejas, thanks again for all the effort put in this PR. Have a look at the summary in this comment: https://github.com/data-dot-all/dataall/pull/1559/files#r1770948390 for the changes needed :)
TejasRGitHub
force-pushed
the
gh-1528-hide-stack-logs
branch
from
e809bce
to
cc1c5e7
Compare
|
Hi @dlpzx , I have made the changes and tested the code locally and on aws data.all deployment. Please let me know if it looks good and can be merged |
|
Hi @TejasRGitHub, this is a great PR! I have left one last nit comment and resolved all other comments. I have approved it and will merge with your response. |
### Feature or Bugfix - Feature ( Docs ) ### Detail - Added in the "Deploy to AWS" section ### Relates - #1559 ### 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.
Feature or Bugfix
Detail
As described in this issue - #1528 , stack logs provide some key info.
Creating a feature config called
show_stack_logsfor s3_datasets, s3_dataset_shares, and core ( for environment stack logs ) and adding logic in frontend to hide/show the logs button.Using decorator in backend to restrict access based on config.
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)? N/A
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.