Skip to content

Commit

Permalink
Organization restricted datastore (#4892)
Browse files Browse the repository at this point in the history
* add db column to datastore

* backend changes

* ignore forbidden orgs when reporting datasets

* Update app/models/binary/DataSetService.scala

Co-authored-by: Florian M <[email protected]>

* add readAccessQ

* bump schema version

Co-authored-by: Florian M <[email protected]>
  • Loading branch information
youri-k and fm3 authored Oct 29, 2020
1 parent 730958a commit 5c06706
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
### Added
- The total length of skeletons can now be measured using the dropdown in the tree list tab. Also, the frontend API received the methods `api.tracing.measureTreeLength` and `api.tracing.measureAllTrees`. [#4898](https://github.com/scalableminds/webknossos/pull/4898)
- Introduced an indeterminate visibility state for groups in the tree tab if not all but only some of the group's children are visible. Before, the visibility of those groups was shown as not visible which made it hard to find the visible trees. [#4897](https://github.com/scalableminds/webknossos/pull/4897)
- Dataset uploads on a specific Datastore can now be restricted to a single organization. [#4892](https://github.com/scalableminds/webknossos/pull/4892)

### Changed
- In the tree tab, all groups but the root group are now collapsed instead of expanded when opening a tracing. [#4897](https://github.com/scalableminds/webknossos/pull/4897)
Expand Down
3 changes: 2 additions & 1 deletion MIGRATIONS.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md).
-

### Postgres Evolutions:
- [056-add-layer-specific-view-configs.sql](conf/evolutions/056-add-layer-specific-view-configs.sql)
- [057-add-layer-specific-view-configs.sql](conf/evolutions/056-add-layer-specific-view-configs.sql)
- [058-add-onlyAllowedOrganization.sql](conf/evolutions/057-add-onlyAllowedOrganization.sql)
1 change: 1 addition & 0 deletions app/controllers/WKDataStoreController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class WKDataStoreController @Inject()(dataSetService: DataSetService,
_ <- bool2Fox(dataSetService.isProperDataSetName(uploadInfo.name)) ?~> "dataSet.name.invalid"
_ <- dataSetService
.assertNewDataSetName(uploadInfo.name, organization._id)(GlobalAccessContext) ?~> "dataSet.name.alreadyTaken"
_ <- bool2Fox(dataStore.onlyAllowedOrganization.forall(_ == organization._id)) ?~> "dataSet.upload.Datastore.restricted"
} yield Ok
}
}
Expand Down
4 changes: 4 additions & 0 deletions app/models/binary/DataSetService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ class DataSetService @Inject()(organizationDAO: OrganizationDAO,
.findOneByName(orgaTuple._1)
.futureBox
.flatMap {
case Full(organization) if dataStore.onlyAllowedOrganization.exists(_ != organization._id) =>
logger.info(
s"Ignoring ${orgaTuple._2.length} reported datasets for forbidden organization ${orgaTuple._1} from organization-specific datastore ${dataStore.name}")
Fox.successful(List.empty)
case Full(organization) =>
for {
foundDatasets <- dataSetDAO.findAllByNamesAndOrganization(orgaTuple._2.map(_.id.name), organization._id)
Expand Down
15 changes: 11 additions & 4 deletions app/models/binary/DataStore.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import play.api.libs.json.{Format, JsObject, Json}
import play.api.mvc.{Request, Result, Results, WrappedRequest}
import slick.jdbc.PostgresProfile.api._
import slick.lifted.Rep
import utils.{SQLClient, SQLDAO}
import utils.{ObjectId, SQLClient, SQLDAO}

import scala.concurrent.{ExecutionContext, Future}

Expand All @@ -23,6 +23,7 @@ case class DataStore(
isForeign: Boolean = false,
isConnector: Boolean = false,
allowsUpload: Boolean = true,
onlyAllowedOrganization: Option[ObjectId] = None
)

object DataStore {
Expand All @@ -45,7 +46,8 @@ object DataStore {
isDeleted = false,
isForeign.getOrElse(false),
isConnector.getOrElse(false),
allowsUpload.getOrElse(true)
allowsUpload.getOrElse(true),
None
)

def fromUpdateForm(name: String,
Expand Down Expand Up @@ -91,6 +93,9 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext
def idColumn(x: Datastores): Rep[String] = x.name
def isDeletedColumn(x: Datastores): Rep[Boolean] = x.isdeleted

override def readAccessQ(requestingUserId: ObjectId): String =
s"(onlyAllowedOrganization is null) OR (onlyAllowedOrganization in (select _organization from webknossos.users_ where _id = '$requestingUserId'))"

def parse(r: DatastoresRow): Fox[DataStore] =
Fox.successful(
DataStore(
Expand All @@ -102,7 +107,8 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext
r.isdeleted,
r.isforeign,
r.isconnector,
r.allowsupload
r.allowsupload,
r.onlyallowedorganization.map(ObjectId(_))
))

def findOneByName(name: String)(implicit ctx: DBAccessContext): Fox[DataStore] =
Expand All @@ -116,7 +122,8 @@ class DataStoreDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext

override def findAll(implicit ctx: DBAccessContext): Fox[List[DataStore]] =
for {
r <- run(sql"select #${columns} from webknossos.datastores_ order by name".as[DatastoresRow])
accessQuery <- readAccessQuery
r <- run(sql"select #$columns from webknossos.datastores_ where #$accessQuery order by name".as[DatastoresRow])
parsed <- Fox.combined(r.toList.map(parse))
} yield parsed

Expand Down
13 changes: 13 additions & 0 deletions conf/evolutions/058-add-onlyAllowedOrganization.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- https://github.com/scalableminds/webknossos/pull/4892

START TRANSACTION;

DROP VIEW webknossos.dataStores_;

ALTER TABLE webknossos.dataStores ADD COLUMN onlyAllowedOrganization CHAR(24);

CREATE VIEW webknossos.dataStores_ AS SELECT * FROM webknossos.dataStores WHERE NOT isDeleted;

UPDATE webknossos.releaseInformation SET schemaVersion = 58;

COMMIT TRANSACTION;
11 changes: 11 additions & 0 deletions conf/evolutions/reversions/058-add-onlyAllowedOrganization.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
START TRANSACTION;

DROP VIEW webknossos.dataStores_;

ALTER TABLE webknossos.dataStores DROP onlyAllowedOrganization;

CREATE VIEW webknossos.dataStores_ AS SELECT * FROM webknossos.dataStores WHERE NOT isDeleted;

UPDATE webknossos.releaseInformation SET schemaVersion = 57;

COMMIT TRANSACTION;
2 changes: 2 additions & 0 deletions conf/messages
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ dataSet.initialTeams.teamsNotEmpty=Dataset already has allowed teams
dataset.initialTeams.invalidTeams=Can only assign teams of user
dataset.initialTeams.timeout=Timeout while setting initial teams. Was the request sent manually?
dataset.delete.disabled=Dataset deletion is disabled for this webKnossos instance
dataSet.upload.Datastore.restricted=Your organization is not allowed to upload datasets to this datastore. Please choose another datastore.
dataSet.upload.validation.failed=Failed to validate Dataset information for upload.

dataSource.notFound=Datasource not found on datastore server

Expand Down
6 changes: 3 additions & 3 deletions test/db/dataStores.csv
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
name,url,publicUrl,key,isScratch,isDeleted,isForeign,isConnector,allowsUpload
'localhost','http://localhost:9000','http://localhost:9000','something-secure',f,f,f,f,t
'connect','http://localhost:8000','http://localhost:8000','secret-key',f,f,f,t,f
name,url,publicUrl,key,isScratch,isDeleted,isForeign,isConnector,allowsUpload,onlyAllowedOrganization
'localhost','http://localhost:9000','http://localhost:9000','something-secure',f,f,f,f,t,
'connect','http://localhost:8000','http://localhost:8000','secret-key',f,f,f,t,f,
5 changes: 3 additions & 2 deletions tools/postgres/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ START TRANSACTION;
CREATE TABLE webknossos.releaseInformation (
schemaVersion BIGINT NOT NULL
);
INSERT INTO webknossos.releaseInformation(schemaVersion) values(57);
INSERT INTO webknossos.releaseInformation(schemaVersion) values(58);
COMMIT TRANSACTION;

CREATE TABLE webknossos.analytics(
Expand Down Expand Up @@ -160,7 +160,8 @@ CREATE TABLE webknossos.dataStores(
isDeleted BOOLEAN NOT NULL DEFAULT false,
isForeign BOOLEAN NOT NULL DEFAULT false,
isConnector BOOLEAN NOT NULL DEFAULT false,
allowsUpload BOOLEAN NOT NULL DEFAULT true
allowsUpload BOOLEAN NOT NULL DEFAULT true,
onlyAllowedOrganization CHAR(24)
);

CREATE TABLE webknossos.tracingStores(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class DataSourceController @Inject()(
val resumableUploadInformation = ResumableUploadInformation(chunkSize, totalChunkCount)
for {
_ <- if (!uploadService.isKnownUpload(uploadId))
webKnossosServer.validateDataSourceUpload(id) ?~> "dataSet.name.alreadyTaken"
webKnossosServer.validateDataSourceUpload(id) ?~> "dataSet.upload.validation.failed"
else Fox.successful(())
chunkFile <- request.body.file("file") ?~> "zip.file.notFound"
_ <- uploadService.handleUploadChunk(uploadId,
Expand Down

0 comments on commit 5c06706

Please sign in to comment.