Skip to content

Commit

Permalink
Votes consistent permissions (#1724)
Browse files Browse the repository at this point in the history
- Feature
- Bugfix

The Votes submodule is used by Dashboards, Datasets and
Redshift_Datasets in the frontend. Although in Dashboards anyone can see
and click "upvote" and in the other 2 modules it is restricted to admins
of the resource only.

In this PR we consolidate one single behavior for Votes:
- The UpVote button is visible by all users: any user can get the number
of upvotes for a data asset
- The UpVote button is visible but disabled for any user except for the
resource admin team: only users with access to the resource can vote for
it.

In this PR we mimic these 2 rules also in the backend:
- the upVote API is restricted to users with `GET_X` permissions to the
resource
- the getVote and countVotes APIs are open to all users

- [X] Deployed to AWS
- [X] Redshift Dataset owner can upvote a dataset
- [X] Programmatically a non-owner receives an error message when
executing upvote

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.
  • Loading branch information
dlpzx committed Dec 9, 2024
1 parent ab1f6e5 commit ff9a8bd
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 14 deletions.
2 changes: 1 addition & 1 deletion backend/dataall/modules/dashboards/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def __init__(self):
)
)

add_vote_type('dashboard', DashboardIndexer)
add_vote_type('dashboard', DashboardIndexer, GET_DASHBOARD)

EnvironmentResourceManager.register(DashboardRepository())
log.info('Dashboard API has been loaded')
Expand Down
2 changes: 1 addition & 1 deletion backend/dataall/modules/s3_datasets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def __init__(self):
)
)

add_vote_type('dataset', DatasetIndexer)
add_vote_type('dataset', DatasetIndexer, GET_DATASET)

TargetType('dataset', GET_DATASET, UPDATE_DATASET, MANAGE_DATASETS)

Expand Down
20 changes: 15 additions & 5 deletions backend/dataall/modules/vote/services/vote_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from dataall.base.context import get_context
from dataall.modules.catalog.indexers.base_indexer import BaseIndexer
from dataall.modules.vote.db.vote_repositories import VoteRepository
from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService

_VOTE_TYPES: Dict[str, Type[BaseIndexer]] = {}
_VOTE_TYPES: Dict[str, Dict[Type[BaseIndexer], str]] = {}


def add_vote_type(target_type: str, indexer: Type[BaseIndexer]):
_VOTE_TYPES[target_type] = indexer
def add_vote_type(target_type: str, indexer: Type[BaseIndexer], permission: str):
_VOTE_TYPES[target_type] = {'indexer': indexer, 'permission': permission}


def _session():
Expand All @@ -26,9 +27,18 @@ class VoteService:

@staticmethod
def upvote(targetUri: str, targetType: str, upvote: bool):
with _session() as session:
context = get_context()
target_type = _VOTE_TYPES[targetType]
with context.db_engine.scoped_session() as session:
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=targetUri,
permission_name=target_type.get('permission'),
)
vote = VoteRepository.upvote(session=session, targetUri=targetUri, targetType=targetType, upvote=upvote)
_VOTE_TYPES[vote.targetType].upsert(session, vote.targetUri)
target_type.get('indexer').upsert(session, vote.targetUri)
return vote

@staticmethod
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/design/components/UpVoteButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import * as PropTypes from 'prop-types';
import React from 'react';

export const UpVoteButton = (props) => {
const { upVoted, onClick, upVotes } = props;
const { upVoted, onClick, upVotes, disabled } = props;
return (
<Button
color="primary"
disabled={disabled}
startIcon={
upVoted ? (
<ThumbUpAlt fontSize="small" />
Expand All @@ -27,5 +28,6 @@ export const UpVoteButton = (props) => {
UpVoteButton.propTypes = {
upVoted: PropTypes.bool,
onClick: PropTypes.func,
upVotes: PropTypes.any
upVotes: PropTypes.any,
disabled: PropTypes.bool
};
1 change: 1 addition & 0 deletions frontend/src/modules/Dashboards/views/DashboardView.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ const DashboardView = () => {
<Box sx={{ m: -1 }}>
<UpVoteButton
upVoted={isUpVoted}
disabled={!isAdmin}
onClick={() => upVoteDashboard(dashboard.dashboardUri)}
upVotes={upVotes || 0}
/>
Expand Down
11 changes: 6 additions & 5 deletions frontend/src/modules/S3_Datasets/views/DatasetView.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,14 @@ const DatasetView = () => {

<Grid item>
<Box sx={{ m: -1 }}>
<UpVoteButton
disabled={!isAdmin}
upVoted={isUpVoted}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
{isAdmin && (
<span>
<UpVoteButton
upVoted={isUpVoted}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
<Button
color="primary"
startIcon={<ForumOutlined fontSize="small" />}
Expand Down

0 comments on commit ff9a8bd

Please sign in to comment.