diff --git a/docs/developer-guide/authorization/published-data.md b/docs/developer-guide/authorization/published-data.md new file mode 100644 index 000000000..47256c68d --- /dev/null +++ b/docs/developer-guide/authorization/published-data.md @@ -0,0 +1,103 @@ +# Published Data Authorization Model + +This document describes the authorization model used for published data and associated endpoints, for both the v4 endpoints (`/api/v4/publisheddata`) and the deprecated v3 endpoints (`/api/v3/publisheddata`). + +> **Note:** The ownership model described here is a temporary deviation from the SciCat authorization pattern. See [Temporary ownership model](#temporary-ownership-model) below. + +## Actions + +The following actions are defined for published data: + +- `AccessAny` +- `Create` +- `Read` +- `Update` +- `Delete` + +## Record states + +Every published data record carries a `status` field, which takes one of the following values, and which participates in authorization: + +- `private`: a draft, visible only to its creator and admins +- `public`: made public, but not yet registered with a DOI registrar +- `registered`: registered with a DOI registrar +- `amended`: registered and subsequently amended + +Records in state `public`, `registered` or `amended` are readable by everyone. Only records in state `private` may be modified or deleted by their creator. + +## Permissions + +Permissions are granted cumulatively to users based on their group association. The following permission levels are granted to users: + +### Unauthenticated + +Unauthenticated users do not have casl permissions to interact with published data records. However, all read endpoints are unguarded with the `@AllowAny` decorator, effectively giving unauthenticated users conditional read access. +Read access is limited to published data records with status `PUBLIC`, `REGISTERED` or `AMENDED` for unauthenticated users. + +### Authenticated + +An authenticated user has unrestricted create, and conditional read and update casl permissions for any published data record. +Read access is limited to published data records with status `PUBLIC`, `REGISTERED` or `AMENDED` and records with status `PRIVATE` if the user is listed under the record's `createdBy` field. +Update access is limited to `PRIVATE` records with the user listed under the record's `createdBy` field. + +### ADMIN_GROUPS + +If a user is part of a group listed in configuration as part of `ADMIN_GROUPS`, they have unrestricted create and read casl permissions, and conditional and update permissions. They are only allowed to update records with `PRIVATE` or `PUBLIC` status, regardless of ownership. Records in `REGISTERED` or `AMENDED` status cannot be updated by anyone. + +### DELETE_GROUPS + +If a user is part of a group listed in configuration as part of `DELETE_GROUPS`, they have unrestricted delete access to all published data records in the database. For the API v4 endpoint, records in `REGISTERED` or `AMENDED` status cannot be deleted by anyone. + +## Permission Matrix + +Table of the different permission classes defined in casl. For all special permission groups, the full list includes the relevant permissions passed on from generic authenticated user permissions. + +### API v3 + +| Operation | Unauthenticated | Authenticated | `ADMIN_GROUPS` | `DELETE_GROUPS` | +| - | - | - | - | - | +| `Create` | - | any | any | any | +| `Read` | public | public/owner | any | public/owner | +| `Update` | - | owner | unregistered | owner | +| `Delete` | - | - | - | any | + +### API v4 + +| Operation | Unauthenticated | Authenticated | `ADMIN_GROUPS` | `DELETE_GROUPS` | +| - | - | - | - | - | +| `Create` | - | any | any | any | +| `Read` | public | public/owner | any | public/owner | +| `Update` | - | owner | unregistered | owner | +| `Delete` | - | - | - | unregistered | + +Legend: +- owner: publishedData's `status` field must be `PRIVATE` and the `createdBy` field must match the user's username +- unregistered: publishedData's `status` field must one of `PRIVATE`, `PUBLIC` +- public: publishedData's `status` field must be one of `PUBLIC`, `REGISTERED`, `AMENDED` +- any: unrestricted access + +## Implementation Notes + +The definition is implemented in the casl module under `/src/casl/abilities/published-data.ability.ts` and accessible elsewhere via `CaslAbilityFactory.publishedDataAccess`. This one function is used to build one casl ability for endpoint and instance authorization: When a user receives permission for an action under some instance-level condition, they should implicitly pass endpoint authorization. + +The `PublishedDataAbility` module in `/src/casl/abilities/published-data.ability.ts` is written in such a way that permissions are cumulative. In case multiple rules apply, casl will chain them in a logical or, ultimately giving precedence to the broadest applicable rule. The special permission groups are sorted roughly in ascending order of privilege level. +In case there are expectations of mutual exclusivity for certain special groups (not the case for published data currently), additional rules using the `cannot` ability expression can be added after all `can` rules have been defined. For an example, see the jobs subsystem authorization docs. + +Read endpoints (`GET /`, `GET /count`, `GET /:id`) add an `$or` filter that matches published records plus the caller's own `private` records. The filter is combined with the client supplied `where` rather than replacing it. +Mutating endpoints (`PATCH /:id`, `POST /:id/register`, `POST /:id/resync`, and `DELETE /:id` on v4) build a filter on `doi` plus `createdBy` for non-admins, so a record belonging to somebody else is simply not found, and the same filter is reused for the write itself. +The lifecycle guard on `status` is applied after the record has been fetched, and is what distinguishes a `400 Bad Request` (found, but not modifiable in its current state) from a `404 Not Found` (not visible to the caller). + +Since a record's only ownership information is `createdBy`, there is no group-based rule to express here, and no `ownerGroup`/`accessGroups` conditions appear in the ability. + +## Temporary ownership model + +Unlike datasets, samples, proposals and most other SciCat entities, the `PublishedData` schema does not extend `OwnableClass`: it has no `ownerGroup` and no `accessGroups` fields. The only ownership hint available is `createdBy`. + +The `createdBy`-based rules described above were introduced as a hot fix for a security issue: on the v3 endpoints, any authenticated user could read any other user's `private` published data and could `PATCH` it, since those endpoints performed authentication but no further authorization (see [PR #2886](https://github.com/SciCatProject/backend/pull/2886)). + +This is understood to be a temporary measure and knowingly deviates from the supported, group-based SciCat pattern. In particular: + +- Only the exact creator of a record can act on it, whereas elsewhere in SciCat any member of the record's `ownerGroup` could. +- The "four eyes" workflow, in which a colleague reviews a `private` published data record before it is registered, is not possible through these endpoints. + +The intended long-term solution is to make `PublishedData` extend `OwnableClass` and to migrate existing records so that `ownerGroup` and `accessGroups` are populated, after which the rules above should be replaced by the standard group-based checks. This is tracked in [issue #2900](https://github.com/SciCatProject/backend/issues/2900). The current model will be supported until that redesign is implemented. diff --git a/docs/developer-guide/authorization/publisheddata.md b/docs/developer-guide/authorization/publisheddata.md deleted file mode 100644 index 29d6b0728..000000000 --- a/docs/developer-guide/authorization/publisheddata.md +++ /dev/null @@ -1,96 +0,0 @@ -# PublishedData Authorization Model - -This document describes the authorization model used for published data and associated endpoints, for both the v4 endpoints (`/api/v4/publisheddata`) and the deprecated v3 endpoints (`/api/v3/publisheddata`). - -> **Note:** The ownership model described here is a temporary deviation from the SciCat authorization pattern. See [Temporary ownership model](#temporary-ownership-model) below. - -## Actions - -The following actions are defined for published data: - -- `Create` -- `Read` -- `Update` -- `Delete` -- `AccessAny` - -`Create`, `Read`, `Update` and `Delete` are used for endpoint authorization, i.e. whether a user may call an endpoint at all. `AccessAny` is used for instance authorization, i.e. whether a user may act on records they do not own. - -## Record states - -Every published data record carries a `status` field, which takes one of the following values, and which participates in authorization: - -- `private`: a draft, visible only to its creator (and admins) -- `public`: made public, but not yet registered with a DOI registrar -- `registered`: registered with a DOI registrar -- `amended`: registered and subsequently amended - -Records in state `public`, `registered` or `amended` are readable by everyone. Only records in state `private` may be modified or deleted by their creator. - -## Permissions - -Permissions are granted cumulatively to users based on their group association. The following permission levels are granted to users: - -### Unauthenticated - -An unauthenticated user may read published data records that are in state `public`, `registered` or `amended`. Listing and counting endpoints are filtered accordingly, and a request for a `private` record is answered with `404 Not Found`. - -Unauthenticated users have no write access. - -### Authenticated - -In addition to the permissions granted to unauthenticated users, an authenticated user may read the `private` records for which their username matches the record's `createdBy`. - -An authenticated user may create published data records. They may update, register, resync and delete only records for which their username matches `createdBy` **and** whose status is `private`. Attempting to modify a record owned by another user is answered with `404 Not Found`; attempting to modify an own record that is no longer `private` is answered with `400 Bad Request`. - -### DELETE_GROUPS - -If a user is a member of a group listed in configuration as part of `DELETE_GROUPS`, they are permitted to call the delete endpoint. - -On the v4 endpoint, the instance-level restrictions above still apply, i.e. a non-admin may delete only their own `private` records. The deprecated v3 delete endpoint performs no instance-level check at all: any member of `DELETE_GROUPS` may delete any record in any state. - -### ADMIN_GROUPS - -If a user is a member of a group listed in configuration as part of `ADMIN_GROUPS`, they are granted `AccessAny` and therefore have unrestricted read access to all published data records, regardless of status or creator, and may update, register, resync and delete records created by any user. - -Admins are still subject to the lifecycle guard: records in state `registered` or `amended` cannot be updated or deleted, since they have already been handed over to a DOI registrar. - -## Permission Matrix - -| Operation | Unauthenticated | Authenticated | `DELETE_GROUPS` | `ADMIN_GROUPS` | -| - | - | - | - | - | -| `Create` | - | any | - | any | -| `Read` | published | published + own | published + own | any | -| `Update` | - | own private | own private | any not registered/amended | -| `Delete` | - | - | own private (v4) / any (v3) | any not registered/amended | - -Legend: -- published: record `status` must be `public`, `registered` or `amended` -- own: record `createdBy` must match the user's username -- own private: record `createdBy` must match the user's username and `status` must be `private` -- any: unrestricted access - -## Implementation Notes - -Endpoint authorization is defined in `CaslAbilityFactory.publishedDataEndpointAccess` and applied through the `PoliciesGuard` and `@CheckPolicies` decorators on the controllers. - -Instance authorization is defined in `CaslAbilityFactory.publishedDataInstanceAccess`, which grants `AccessAny` to members of `ADMIN_GROUPS` only. Both controllers use this single ability to decide whether to narrow the Mongo query: - -- Read endpoints (`GET /`, `GET /count`, `GET /:id`) add an `$or` filter that matches published records plus the caller's own `private` records. The filter is combined with the client supplied `where` rather than replacing it. -- Mutating endpoints (`PATCH /:id`, `POST /:id/register`, `POST /:id/resync`, and `DELETE /:id` on v4) build a filter on `doi` plus `createdBy` for non-admins, so a record belonging to somebody else is simply not found, and the same filter is reused for the write itself. -- The lifecycle guard on `status` is applied after the record has been fetched, and is what distinguishes a `400 Bad Request` (found, but not modifiable in its current state) from a `404 Not Found` (not visible to the caller). - -Since a record's only ownership information is `createdBy`, there is no group-based rule to express here, and no `ownerGroup`/`accessGroups` conditions appear in the ability. - -## Temporary ownership model - -Unlike datasets, samples, proposals and most other SciCat entities, the `PublishedData` schema does not extend `OwnableClass`: it has no `ownerGroup` and no `accessGroups` fields. The only ownership hint available is `createdBy`. - -The `createdBy`-based rules described above were introduced as a hot fix for a security issue: on the v3 endpoints, any authenticated user could read any other user's `private` published data and could `PATCH` it, since those endpoints performed authentication but no further authorization (see [PR #2886](https://github.com/SciCatProject/backend/pull/2886)). - -This is understood to be a temporary measure and knowingly deviates from the supported, group-based SciCat pattern. In particular: - -- Only the exact creator of a record can act on it, whereas elsewhere in SciCat any member of the record's `ownerGroup` could. -- The "four eyes" workflow, in which a colleague reviews a `private` published data record before it is registered, is not possible through these endpoints. - -The intended long-term solution is to make `PublishedData` extend `OwnableClass` and to migrate existing records so that `ownerGroup` and `accessGroups` are populated, after which the rules above should be replaced by the standard group-based checks. This is tracked in [issue #2900](https://github.com/SciCatProject/backend/issues/2900). The current model will be supported until that redesign is implemented. diff --git a/src/casl/abilities/published-data.ability.ts b/src/casl/abilities/published-data.ability.ts new file mode 100644 index 000000000..08b836945 --- /dev/null +++ b/src/casl/abilities/published-data.ability.ts @@ -0,0 +1,73 @@ +import { + AbilityBuilder, + ExtractSubjectType, + MongoAbility, + createMongoAbility, +} from "@casl/ability"; +import { Injectable } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; +import { AccessGroupsType } from "src/config/configuration"; +import { Action } from "../action.enum"; +import { + Subjects, + PossibleAbilities, + Conditions, +} from "../types/casl-subjects"; +import { JWTUser } from "src/auth/interfaces/jwt-user.interface"; +import { PublishedData } from "src/published-data/schemas/published-data.schema"; + +@Injectable() +export class PublishedDataAbility { + private accessGroups?: AccessGroupsType; + constructor(private configService: ConfigService) { + this.accessGroups = + this.configService.get("accessGroups") ?? + ({} as AccessGroupsType); + } + + buildAbility( + user: JWTUser | null, + ): MongoAbility { + const { can, build } = new AbilityBuilder( + createMongoAbility, + ); + + /** + * Unauthenticated user + */ + if (!user) { + return build({ + detectSubjectType: (item) => + item.constructor as ExtractSubjectType, + }); + } + + /** + * Authenticated user + */ + can(Action.Create, PublishedData); + can(Action.Read, PublishedData); + can(Action.Update, PublishedData); + + if (user.currentGroups.some((g) => this.accessGroups?.admin?.includes(g))) { + /** + * User belonging to ADMIN_GROUPS + */ + can(Action.AccessAny, PublishedData); + } + + if ( + user.currentGroups.some((g) => this.accessGroups?.delete?.includes(g)) + ) { + /** + * User belonging to DELETE_GROUPS + */ + can(Action.Delete, PublishedData); + } + + return build({ + detectSubjectType: (item) => + item.constructor as ExtractSubjectType, + }); + } +} diff --git a/src/casl/casl-ability.factory.spec.ts b/src/casl/casl-ability.factory.spec.ts index 48f6f6087..5e0156e60 100644 --- a/src/casl/casl-ability.factory.spec.ts +++ b/src/casl/casl-ability.factory.spec.ts @@ -17,6 +17,7 @@ import { OpensearchAbility } from "./abilities/opensearch.ability"; import { OrigDatablockAbility } from "./abilities/origdatablocks.ability"; import { PolicyAbility } from "./abilities/policies.ability"; import { ProposalAbility } from "./abilities/proposals.ability"; +import { PublishedDataAbility } from "./abilities/published-data.ability"; import { RuntimeConfigAbility } from "./abilities/runtime-config.ability"; describe("CaslAbilityFactory", () => { @@ -41,6 +42,7 @@ describe("CaslAbilityFactory", () => { new OrigDatablockAbility(configService), new PolicyAbility(configService), new ProposalAbility(configService), + new PublishedDataAbility(configService), new RuntimeConfigAbility(configService), ), ).toBeDefined(); @@ -77,6 +79,7 @@ describe("CaslAbilityFactory", () => { new OrigDatablockAbility(configService), new PolicyAbility(configService), new ProposalAbility(configService), + new PublishedDataAbility(configService), new RuntimeConfigAbility(configService), ); }; diff --git a/src/casl/casl-ability.factory.ts b/src/casl/casl-ability.factory.ts index 1a4135a34..d1ce97150 100644 --- a/src/casl/casl-ability.factory.ts +++ b/src/casl/casl-ability.factory.ts @@ -8,7 +8,6 @@ import { Injectable, InternalServerErrorException } from "@nestjs/common"; import { ConfigService } from "@nestjs/config"; import { JWTUser } from "src/auth/interfaces/jwt-user.interface"; import { AccessGroupsType } from "src/config/configuration"; -import { PublishedData } from "src/published-data/schemas/published-data.schema"; import { SampleClass } from "src/samples/schemas/sample.schema"; import { User } from "src/users/schemas/user.schema"; import { Action } from "./action.enum"; @@ -25,6 +24,7 @@ import { OpensearchAbility } from "./abilities/opensearch.ability"; import { OrigDatablockAbility } from "./abilities/origdatablocks.ability"; import { PolicyAbility } from "./abilities/policies.ability"; import { ProposalAbility } from "./abilities/proposals.ability"; +import { PublishedDataAbility } from "./abilities/published-data.ability"; import { RuntimeConfigAbility } from "./abilities/runtime-config.ability"; export type AppAbility = MongoAbility; @@ -45,6 +45,7 @@ export class CaslAbilityFactory { private origDatablockAbility: OrigDatablockAbility, private policyAbility: PolicyAbility, private proposalAbility: ProposalAbility, + private publishedDataAbility: PublishedDataAbility, private runtimeConfigAbility: RuntimeConfigAbility, ) { this.accessGroups = @@ -67,7 +68,7 @@ export class CaslAbilityFactory { origdatablocks: this.origDatablockAccess, policies: this.policyAccess, proposals: this.proposalAccess, - publisheddata: this.publishedDataEndpointAccess, + publisheddata: this.publishedDataAccess, runtimeconfig: this.runtimeConfigAccess, samples: this.samplesEndpointAccess, users: this.userEndpointAccess, @@ -131,33 +132,12 @@ export class CaslAbilityFactory { return this.proposalAbility.buildAbility(user); } - runtimeConfigAccess(user: JWTUser | null) { - return this.runtimeConfigAbility.buildAbility(user); + publishedDataAccess(user: JWTUser | null) { + return this.publishedDataAbility.buildAbility(user); } - publishedDataEndpointAccess(user: JWTUser) { - const { can, build } = new AbilityBuilder( - createMongoAbility, - ); - if (user) { - can(Action.Read, PublishedData); - can(Action.Update, PublishedData); - can(Action.Create, PublishedData); - } - - if ( - user && - user.currentGroups.some((g) => this.accessGroups?.delete.includes(g)) - ) { - /* - / user that belongs to any of the group listed in DELETE_GROUPS - */ - can(Action.Delete, PublishedData); - } - return build({ - detectSubjectType: (item) => - item.constructor as ExtractSubjectType, - }); + runtimeConfigAccess(user: JWTUser | null) { + return this.runtimeConfigAbility.buildAbility(user); } samplesEndpointAccess(user: JWTUser) { @@ -503,26 +483,4 @@ export class CaslAbilityFactory { item.constructor as ExtractSubjectType, }); } - - publishedDataInstanceAccess(user: JWTUser) { - const { can, build } = new AbilityBuilder( - createMongoAbility, - ); - - if ( - user && - user.currentGroups.some((g) => this.accessGroups?.admin.includes(g)) - ) { - // ------------------------------------- - // users belonging to any of the group listed in ADMIN_GROUPS - // ------------------------------------- - - can(Action.AccessAny, PublishedData); - } - - return build({ - detectSubjectType: (item) => - item.constructor as ExtractSubjectType, - }); - } } diff --git a/src/casl/casl.module.ts b/src/casl/casl.module.ts index cf36bdf45..04da9c692 100644 --- a/src/casl/casl.module.ts +++ b/src/casl/casl.module.ts @@ -14,6 +14,7 @@ import { OpensearchAbility } from "./abilities/opensearch.ability"; import { OrigDatablockAbility } from "./abilities/origdatablocks.ability"; import { PolicyAbility } from "./abilities/policies.ability"; import { ProposalAbility } from "./abilities/proposals.ability"; +import { PublishedDataAbility } from "./abilities/published-data.ability"; import { RuntimeConfigAbility } from "./abilities/runtime-config.ability"; @Module({ @@ -32,6 +33,7 @@ import { RuntimeConfigAbility } from "./abilities/runtime-config.ability"; OrigDatablockAbility, PolicyAbility, ProposalAbility, + PublishedDataAbility, RuntimeConfigAbility, ], exports: [CaslAbilityFactory], diff --git a/src/published-data/published-data.controller.ts b/src/published-data/published-data.controller.ts index 00ba9e87f..ff8fc148a 100644 --- a/src/published-data/published-data.controller.ts +++ b/src/published-data/published-data.controller.ts @@ -150,7 +150,7 @@ export class PublishedDataController { publishedDataFilters.where = this.applyReadAccessFilters( user, - this.caslAbilityFactory.publishedDataInstanceAccess(user as JWTUser), + this.caslAbilityFactory.publishedDataAccess(user as JWTUser), publishedDataFilters.where, ); @@ -191,7 +191,7 @@ export class PublishedDataController { filters.where = this.applyReadAccessFilters( user, - this.caslAbilityFactory.publishedDataInstanceAccess(user as JWTUser), + this.caslAbilityFactory.publishedDataAccess(user as JWTUser), filters.where, ); @@ -308,7 +308,7 @@ export class PublishedDataController { const publishedData = await this.publishedDataService.findOne( this.applyReadAccessFilters( user, - this.caslAbilityFactory.publishedDataInstanceAccess(user as JWTUser), + this.caslAbilityFactory.publishedDataAccess(user as JWTUser), idFilter, ), ); @@ -349,7 +349,7 @@ export class PublishedDataController { updatePublishedDataDto: PartialUpdatePublishedDataDto, ): Promise { const user = request.user as JWTUser; - const ability = this.caslAbilityFactory.publishedDataInstanceAccess(user); + const ability = this.caslAbilityFactory.publishedDataAccess(user); const canAccessAny = ability.can(Action.AccessAny, PublishedData); const filter = this.getMutationAccessFilters(user, ability, id); @@ -485,7 +485,7 @@ export class PublishedDataController { const user = request.user as JWTUser; const filter = this.getMutationAccessFilters( user, - this.caslAbilityFactory.publishedDataInstanceAccess(user), + this.caslAbilityFactory.publishedDataAccess(user), id, ); @@ -711,7 +711,7 @@ export class PublishedDataController { const user = request.user as JWTUser; const filter = this.getMutationAccessFilters( user, - this.caslAbilityFactory.publishedDataInstanceAccess(user), + this.caslAbilityFactory.publishedDataAccess(user), id, ); diff --git a/src/published-data/published-data.v4.controller.ts b/src/published-data/published-data.v4.controller.ts index f69b5228c..307cc5d33 100644 --- a/src/published-data/published-data.v4.controller.ts +++ b/src/published-data/published-data.v4.controller.ts @@ -143,7 +143,7 @@ export class PublishedDataV4Controller { publishedDataFilters.limits = publishedDataLimits; } - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); @@ -189,7 +189,7 @@ export class PublishedDataV4Controller { ) { const jsonFilters: IPublishedDataFilters = filter?.filter ?? {}; - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); @@ -293,7 +293,7 @@ export class PublishedDataV4Controller { const filter: FilterQuery = { doi, }; - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); if (ability.cannot(Action.AccessAny, PublishedData)) { @@ -371,7 +371,7 @@ export class PublishedDataV4Controller { throw new NotFoundException(`Published data with id ${id} not found.`); } - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); @@ -470,7 +470,7 @@ export class PublishedDataV4Controller { @Req() request: Request, @Param("id") id: string, ): Promise { - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); @@ -519,7 +519,7 @@ export class PublishedDataV4Controller { throw new NotFoundException(`Published data with id ${id} not found.`); } - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, ); @@ -685,7 +685,7 @@ export class PublishedDataV4Controller { throw new NotFoundException(`Published data with id ${id} not found.`); } - const ability = this.caslAbilityFactory.publishedDataInstanceAccess( + const ability = this.caslAbilityFactory.publishedDataAccess( request.user as JWTUser, );