Skip to content

Commit

Permalink
feat(Datasets): restrict dataset deletion only to creators and super-…
Browse files Browse the repository at this point in the history
…users (#1713)

* feat(Datasets): restrict dataset deletion only to creators and super-users

* tests: add tests for datasets deletion check

* chore: improve deletion error message

Co-authored-by: Daniel Vila Suero <daniel@recogn.ai>

* Format long strings

Co-authored-by: Daniel Vila Suero <daniel@recogn.ai>

(cherry picked from commit c5ab270)
fix: handle separately 401 and 403 api errors in UI (#1740)

Since the changes included in #1713, users without permission trying to delete datasets from UI will hit into a re-login operation.

This PR handles separate authentication errors (401) and forbidden errors (403).

We should find a better way to handle error codes and info in general.
  • Loading branch information
frascuchon committed Oct 5, 2022
1 parent 8338f00 commit 8d07774
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 9 deletions.
6 changes: 3 additions & 3 deletions frontend/plugins/vuex-orm-axios.js
Expand Up @@ -42,7 +42,7 @@ export default ({ $axios, app }) => {

$axios.onError((error) => {
const code = parseInt(error.response && error.response.status);
if (error instanceof ExpiredAuthSessionError || [401, 403].includes(code)) {
if (error instanceof ExpiredAuthSessionError || 401 === code) {
app.$auth.logout();
}

Expand Down Expand Up @@ -70,13 +70,13 @@ export default ({ $axios, app }) => {
break;
case 404:
Notification.dispatch("notify", {
message: "Warning: " + error.response.data.detail,
message: `Warning: ${messageDetail.params.detail}`,
type: "warning",
});
break;
default:
Notification.dispatch("notify", {
message: error,
message: `Error: ${messageDetail.params.detail}`,
type: "error",
});
}
Expand Down
21 changes: 17 additions & 4 deletions scripts/migrations/es_migration_25042021.py
@@ -1,9 +1,24 @@
# Copyright 2021-present, the Recognai S.L. team.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from itertools import zip_longest
from typing import Any, Dict, List, Optional

from elasticsearch import Elasticsearch
from elasticsearch.helpers import scan, bulk
from elasticsearch.helpers import bulk, scan
from pydantic import BaseSettings

from rubrix.server.tasks.commons import TaskType


Expand Down Expand Up @@ -97,9 +112,7 @@ def map_doc_2_action(
source_index_info = client.get(index=source_datasets_index, id=dataset)

target_dataset_name = f"{dataset}-{settings.task}".lower()
target_index = target_record_index_pattern.format(
target_dataset_name
)
target_index = target_record_index_pattern.format(target_dataset_name)

target_index_info = source_index_info["_source"]
target_index_info["task"] = settings.task
Expand Down
8 changes: 7 additions & 1 deletion src/rubrix/server/services/datasets.py
Expand Up @@ -141,7 +141,13 @@ def delete(self, user: User, dataset: ServiceDataset):
as_dataset_class=None,
)
if found:
self.__dao__.delete_dataset(dataset)
if user.is_superuser() or user.username == dataset.created_by:
self.__dao__.delete_dataset(dataset)
else:
raise ForbiddenOperationError(
f"You don't have the necessary permissions to delete this dataset. "
"Only dataset creators or administrators can delete datasets"
)

def update(
self,
Expand Down
15 changes: 15 additions & 0 deletions tests/datasets/test_datasets.py
Expand Up @@ -3,6 +3,7 @@
import rubrix as rb
from rubrix import TextClassificationSettings, TokenClassificationSettings
from rubrix.client import api
from rubrix.client.sdk.commons.errors import ForbiddenApiError


@pytest.mark.parametrize(
Expand Down Expand Up @@ -37,3 +38,17 @@ def test_settings_workflow(mocked_client, settings_, wrong_settings):

with pytest.raises(ValueError, match="Task type mismatch"):
rb.configure_dataset(dataset, wrong_settings)


def test_delete_dataset_by_non_creator(mocked_client):
try:
dataset = "test_delete_dataset_by_non_creator"
rb.delete(dataset)
rb.configure_dataset(
dataset, settings=TextClassificationSettings(label_schema={"A", "B", "C"})
)
mocked_client.change_current_user("mock-user")
with pytest.raises(ForbiddenApiError):
rb.delete(dataset)
finally:
mocked_client.reset_default_user()
27 changes: 26 additions & 1 deletion tests/helpers.py
Expand Up @@ -3,21 +3,46 @@
from fastapi import FastAPI
from starlette.testclient import TestClient

from rubrix._constants import API_KEY_HEADER_NAME
from rubrix._constants import API_KEY_HEADER_NAME, RUBRIX_WORKSPACE_HEADER_NAME
from rubrix.client.api import active_api
from rubrix.server.security import auth
from rubrix.server.security.auth_provider.local.settings import settings
from rubrix.server.security.auth_provider.local.users.model import UserInDB


class SecuredClient:
def __init__(self, client: TestClient):
self._client = client
self._header = {API_KEY_HEADER_NAME: settings.default_apikey}
self._current_user = None

@property
def fastpi_app(self) -> FastAPI:
return self._client.app

def change_current_user(self, username):
default_user = auth.users.__dao__.__users__["rubrix"]
new_user = UserInDB(
username=username,
hashed_password=username, # Even if required, we can ignore it
api_key=username,
workspaces=["rubrix"], # The default workspace
)

auth.users.__dao__.__users__[username] = new_user
rb_api = active_api()
rb_api._user = new_user
rb_api.set_workspace(default_user.username)
rb_api.client.token = new_user.api_key

def reset_default_user(self):
default_user = auth.users.__dao__.__users__["rubrix"]

rb_api = active_api()
rb_api._user = default_user
rb_api.client.token = default_user.api_key
rb_api.client.headers.pop(RUBRIX_WORKSPACE_HEADER_NAME)

def add_workspaces_to_rubrix_user(self, workspaces: List[str]):
rubrix_user = auth.users.__dao__.__users__["rubrix"]
rubrix_user.workspaces.extend(workspaces or [])
Expand Down

0 comments on commit 8d07774

Please sign in to comment.