Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docs for user permissions #2128

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a197ecd
Add docs for user permissions
fraxachun Jun 2, 2024
91c6d1e
Link from content scope chapters
fraxachun Jun 4, 2024
7d883a3
Merge remote-tracking branch 'origin/main' into user-permission-docs
fraxachun Jun 4, 2024
7dc2ba7
Update docs/docs/content-scope/content-website.md
fraxachun Jun 13, 2024
7108bf8
Update docs/docs/content-scope/data-driven-application.md
fraxachun Jun 13, 2024
c2784dc
Remove wrongly copied text
fraxachun Jun 13, 2024
ebab3b4
Update docs/docs/user-permissions/index.md
fraxachun Jun 13, 2024
9154cfc
Update docs/docs/user-permissions/index.md
fraxachun Jun 13, 2024
28df049
Fix wrong guards
fraxachun Jun 13, 2024
4b70c01
Update docs/docs/user-permissions/admin.md
fraxachun Jun 13, 2024
4361ac6
Add demo code
fraxachun Jun 13, 2024
d737402
Merge remote-tracking branch 'origin/main' into user-permission-docs
fraxachun Jun 13, 2024
36ae826
Apply suggestions from code review
johnnyomair Jun 24, 2024
3139432
Apply suggestions from code review
johnnyomair Jun 24, 2024
7764240
Multiple small improvements
thomasdax98 Jun 25, 2024
b80c68e
Merge remote-tracking branch 'origin/main' into user-permission-docs
fraxachun Oct 18, 2024
10a517d
Fix merge errors
fraxachun Oct 18, 2024
b66e736
Add sentence
fraxachun Oct 18, 2024
0e85f2d
Update to current api
fraxachun Oct 18, 2024
68766b7
Use admin panel
fraxachun Oct 18, 2024
07cbbe9
Move section
fraxachun Oct 18, 2024
12ee25e
Add part
fraxachun Oct 18, 2024
73dbd31
Add text
fraxachun Oct 18, 2024
773e977
fix
fraxachun Oct 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 0 additions & 32 deletions docs/docs/auth/index.md

This file was deleted.

2 changes: 1 addition & 1 deletion docs/docs/content-scope/content-website.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ You can then use `useContentScope()` to access the currently selected scope, whi

COMET's user permission feature will automatically validate `scope` arguments of GraphQL operations and check if a user has access to the entity's scope. The column must be named `scope` for this to work.

You additionally need `@ScopedEntity` (at entity level) for nested entities. And, for operations without a `scope` argument, you must add `@AffectedEntity` at resolver level.
For nested entities or operations without a `scope` argument please refer to the [documentation of the User Permissions](/docs/user-permissions/access-control) system which describes how to decorate the resolvers/controllers properly.
fraxachun marked this conversation as resolved.
Show resolved Hide resolved
47 changes: 1 addition & 46 deletions docs/docs/content-scope/data-driven-application.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,49 +44,4 @@ const variables = {

### API: User permissions

Data-driven applications don't benefit much from the COMET scope system. But for user permissions it has its value.

First, an overview of user permissions:

- Every user has permissions that give them access to resolvers (e.g., "products") - not covered here
- Every user has access to scopes

(Both are defined by rule in `AccessControlService` or can be overridden manually per user in the Admin)

- Every entity belongs to a scope

(The user permission feature checks for every request if the entity scope and the user's allowed scopes match.)

#### @ScopedEntity

Use this decorator at entity level to return the scope of an entity. You might have to load multiple relations for nested data.

```ts
@ScopedEntity(async (product: Product) => {
return {
dealer: product.dealer.id,
};
})
@Entity()
export class Product extends BaseEntity<Product, "id"> {}
```

#### @AffectedEntity

Use this decorator at the operation level to specify which entity (and thus scope) is affected by the operation.

```ts
@Query(Product)
@AffectedEntity(Product)
async product(@Args("id", { type: () => ID }) id: string): Promise<Product> {
//...
}
```

```ts
@Query([Product])
@AffectedEntity(Dealer, { idArg: "dealer" })
async products(@Args("dealer", { type: () => ID }) dealer: string): Promise<Product[]> {
// Note: you can trust "dealer" being in a valid scope, but you need to make sure that your business code restricts this query to the given dealer
}
```
In data-driven applications, the scope system is primarily used for controlling permissions. For additional information refer to the [User Permissions chapter](/docs/user-permissions/access-control).
fraxachun marked this conversation as resolved.
Show resolved Hide resolved
File renamed without changes.
6 changes: 6 additions & 0 deletions docs/docs/logging/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
title: Logging
sidebar_position: 8
---

COMET DXP can be used to create both, content websites and data-driven applications. The usage of the content scope differs considerably between these two use cases.
fraxachun marked this conversation as resolved.
Show resolved Hide resolved
147 changes: 147 additions & 0 deletions docs/docs/user-permissions/access-control.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
---
title: Access Control in the API
sidebar_position: 2
---

:::note

The term **operation** stands for the locations in which COMET DXP invokes permission checks:

- Queries/Mutations in GraphQL-resolvers
- Routes in REST-controllers

Normally you want to decorate the methods of these classes, however, decorating the whole class is also possible.
:::

After activating the module, COMET DXP checks every operation for the required permissions and scopes. Therefore it is necessary to decorate the operations to let the system know what to check. COMET DXP then checks if the current user possesses the permission defined in the decorator.

Additionally, the scope of the data in operation will be checked against the scope of the users. To achieve this, the system has to know the scope of the data that is being handled right now.

:::note
You might also want to check the permissions on field resolvers. Therefore you have to add `guards` to `fieldResolverEnhancers` in the configuration of the GraphQL-module. Please be aware that field resolvers are only checked for permissions but not for scopes.
thomasdax98 marked this conversation as resolved.
Show resolved Hide resolved
:::

## Permission check

**@RequiredPermission**

This decorator is mandatory for all operations. The first parameter of type `string | string[] | "disablePermissionCheck"` configures which permission is necessary to access the decorated operation.

The core of COMET DXP already defines a list of permissions (e.g. `pageTree`, `dam`, `cronJobs`, `userPermissions`). Permissions are defined as plain strings, in the most basic case they represent the main items of the menu bar in the administration panel.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We possibly should provide more information when to use a Comet-internal permission in projects, e.g., pageTree for pages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know what exactly to write about that.


However, if you need a more fine-grained access control you might want to concatenate strings, e.g. `newsRead` or `newsCreate`. Only create as many permissions as really necessary.

:::info
Future version will support a dot-like notation (e.g. `news` will subsume `news.read` and `news.write`).
:::

## Scope check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the sections about @AffectedEntity and ScopedEntity be in the content scope section?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this section back to the Content-Scope chapter: 07cbbe9


:::caution
COMET DXP validates the data relevant for the operation, but cannot check if the validated data is finally used. You are responsible for applying the validated data in your operations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds confusing to me. I guess what it should tell me is "Be careful which data you use and how you use it in your operation to not accidentally bypass the scope check".

What do you think about:

Suggested change
COMET DXP validates the data relevant for the operation, but cannot check if the validated data is finally used. You are responsible for applying the validated data in your operations.
COMET DXP validates the scope of the data relevant for the operation, but it cannot check how you use the data / what you do with the data within the operation. It's your job to use the data in a responsible way and without bypassing the scope check in your code. This means, for example:
- configuring the scope check for all data used in your operation (not just for one entity)
- not loading additional data from the DB without considering the scope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand these examples.

:::

To evaluate the scope there a two technically very distinctive ways depending on the type of operation:

### Operations that create entities or query lists

If an operation does not handle existing entities the scope has to be passed as an argument. COMET DXP expects the argument to be named `scope` to be able to validate it. Do not forget to apply the `scope` argument in your operation.
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved

### Operations that handle specific entities

**@AffectedEntity**

COMET DXP needs information on which entities are being handled in the operation ("affected entity"). Therefore, every operation of this kind needs to be marked with this decorator.
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved

Use this decorator at the **operation level** to specify which entity (and thus scope) is affected by the operation.

:::info
By default COMET DXP tries to load the affected entity by id with the value of the submitted id-argument. However, the name of the argument can be altered by the `idArg` setting.
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved
:::

```ts
@Query(Product)
@AffectedEntity(Product)
async product(@Args("id", { type: () => ID }) id: string): Promise<Product> {
//...
}
```

```ts
@Query([Product])
@AffectedEntity(Dealer, { idArg: "dealerId" })
async products(@Args("dealerId", { type: () => ID }) dealerId: string): Promise<Product[]> {
// Note: you can trust "dealerId" being in a valid scope, but you need to make sure that your business code restricts this query to the given dealer
}
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also mention that you can have multiple AffectedEntity decorators:

Suggested change
It's possible to add multiple `@AffectedEntity` decorators to one operation if multiple entities are affected:
\```ts
@Query(Product)
@AffectedEntity(Product)
@AffectedEntity(Dealer, { idArg: "dealerId" })
async product(@Args("id", { type: () => ID }) id: string, @Args("dealerId", { type: () => ID }) dealerId: string): Promise<Product> {
//...
}
\```

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**@ScopedEntity**

Retrieving the affected entity alone is not sufficient, COMET DXP also needs to know the scope of the entity. The simplest case is when the entity has a field named `scope`. If this is true, this decorator is not necessary.

If the scope is stored in a different field or the entity has a relation to another entity that stores the scope, additional information is required. This is where this decorator comes into play.

Use this decorator at the **entity level** to return the scope of an entity.

```ts
@ScopedEntity(async (product: Product) => {
return {
dealer: product.dealer.id,
};
})
@Entity()
export class Product extends BaseEntity<Product, "id"> {}
```

:::info
You might have to load multiple relations for nested data.
:::

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For documents that get their scope from a `PageTreeNode`, you can pass the helper service `PageTreeNodeDocumentEntityScopeService` provided by the library:
\```ts
@ScopedEntity(PageTreeNodeDocumentEntityScopeService)
export class PredefinedPage extends BaseEntity<PredefinedPage, "id"> implements DocumentInterface {
\```

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## Disable permission/scope checks

**skipScopeCheck**

The scope check can be disabled by adding `{skipScopeCheck: true}` as the second argument of the `@RequiredPermission` decorator.

:::caution
Use this option only when you are sure that checking the scope is not necessary (e.g. the current entity does not have a scope). Do not add it just because it seems cumbersome at the moment to add the correct `AffectedEntity`/`ScopedEntity` decorators.
:::

:::note
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note doesn't seem right here. Was this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because in many cases skipScopeCheck is disabled only for this reason. But we could think about a more prominent location for this advice since it covers a fairly extensive design guideline.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, maybe add a sentence that skipScopeCheck shouldn't be used in combination with @GetCurrentUser()?

But we could think about a more prominent location for this advice since it covers a fairly extensive design guideline.

This could be something for a follow-up PR regarding general API design.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to avoid using the `getCurrentUser` decorator. Instead, you should explicitly send all the data needed in an operation. In the following example, this requires adding `userId` as a scope part as well as passing the data throughout the client. Nevertheless, this leads to a cleaner API design.
thomasdax98 marked this conversation as resolved.
Show resolved Hide resolved

```diff
- @RequiredPermission("products", {skipScopeCheck: true})
+ @RequiredPermission("products")
+ @AffectedEntity(User)
- async myProducts(@GetCurrentUser() currentUser: CurrentUser): Promise<Product[]> {
+ async productsForUser(@Args("userId", { type: () => ID }) userId: string): Promise<Product[]> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work, I suppose. We either need

Suggested change
+ @AffectedEntity(User)
- async myProducts(@GetCurrentUser() currentUser: CurrentUser): Promise<Product[]> {
+ async productsForUser(@Args("userId", { type: () => ID }) userId: string): Promise<Product[]> {
+ @AffectedEntity(User)
- async myProducts(@GetCurrentUser() currentUser: CurrentUser): Promise<Product[]> {
+ async productsForUser(@Args("id", { type: () => ID }) id: string): Promise<Product[]> {

or

Suggested change
+ @AffectedEntity(User)
- async myProducts(@GetCurrentUser() currentUser: CurrentUser): Promise<Product[]> {
+ async productsForUser(@Args("userId", { type: () => ID }) userId: string): Promise<Product[]> {
+ @AffectedEntity(User, { idArg: "userId" })
- async myProducts(@GetCurrentUser() currentUser: CurrentUser): Promise<Product[]> {
+ async productsForUser(@Args("userId", { type: () => ID }) userId: string): Promise<Product[]> {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//...
}
```

:::

**@PublicApi** (COMET DXP v6)

`@PublicApi()` can be used to expose a single handler (query, mutation or route) or a whole class (resolver or controller) publicly.

:::caution

Using the decorator at class level causes later added handlers to be automatically public. Prefer using the decorator for single handlers only.
thomasdax98 marked this conversation as resolved.
Show resolved Hide resolved

:::

**@DisableGlobalAuthGuard** (COMET DXP v7)
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved

`@DisableGlobalAuthGuard()` disables the global auth guard (`CometAuthGuard`). This may be used if a different authentication method is desired (e.g., basic authentication) for a specific handler or class. It should be used in combination with a custom guard. The custom guard may leverage `@PublicApi` as well to expose handlers publicly.
fraxachun marked this conversation as resolved.
Show resolved Hide resolved

e.g.:

```typescript
@DisableGlobalGuard()
@UseGuards(MyCustomGuard)
async handlerThatUsesACustomGuard(): {
...
}
```
48 changes: 48 additions & 0 deletions docs/docs/user-permissions/admin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
title: Permissions in Admin
sidebar_position: 3
---

:::caution
Please be aware that there is no access control possible in the admin panel. You have to check every permission and scope in the API. The following functions only assist for showing the correct user interface.
:::

### CurrentUserProvider

The `CurrentUserProvider` loads the current User from the API and provides the following Hooks:
fraxachun marked this conversation as resolved.
Show resolved Hide resolved

**useCurrentUser**

You can use this hook to access the current user.

:::note
The current user object provides `allowedContentScopes` which may be used for the content scope selector.
:::

:::info
Try not to use the permissions field of the current user object directly as this is subject to change in future versions.
:::

**useUserPermissionCheck**

This hook provides a function that behaves like the `isAllowed` function in the API.

```ts
const isAllowed = useUserPermissionCheck();
if (isAllowed("pageTree")) {
// ...
}
```

:::info
Since this function also checks the content scope, it requires the `ContentScopeProvider` in the rendering tree.
:::

### MasterMenuData
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved

The `MasterMenuData` data type provides a unified format for

- the `menu` prop in `MasterMenu`
- the `menu` prop in `MasterMenuRoutes`

Regarding user permissions, `MasterMenuData` also provides a `requiredPermission` field. Both of the mentioned components use this field to filter the data.
33 changes: 33 additions & 0 deletions docs/docs/user-permissions/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
title: User Permissions
sidebar_position: 6
---

While COMET DXP does not provide authentication it handles authorization by providing a User Permissions system.
fraxachun marked this conversation as resolved.
Show resolved Hide resolved

## Key concepts

The user permissions system

- streamlines authorization throughout the application
- not only checks operations but also the handled data
- relies on external user handling
- allows assigning permissions by code as well as in the administration panel
- offers an administration panel that works out of the box

COMET DXP checks authentication in two dimensions:

**Permissions** are used for access control to specific resolvers or controllers.

**Content Scopes** (also referred to as scopes) are used to control which data is allowed to be handled (see [documentation about content scopes](/docs/content-scope)).

Users in COMET DXP possess permissions and scopes. Every operation is assigned to one or more permissions and handles data that is bound to a scope. The system then just tries to match if the requested permissions and scopes are covered by the user's permissions and scopes.
fraxachun marked this conversation as resolved.
Show resolved Hide resolved

There are no roles as they can easily be represented as a combination of permissions. Furthermore, the ability to check scopes is much more powerful than just having assigned a role.
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved

## Important types

- `User` is provided by COMET DXP as an interface so that it's possible to enhance the type by Typescript augmentation. By default, a ` User` object contains the fields `id`, `name` and `email`.
johnnyomair marked this conversation as resolved.
Show resolved Hide resolved
- `CurrentUser` is used as a GraphQL-type and is returned by GetCurrentUser(). It's not customizable and enhances the default `User` type with the current permissions and scopes.
thomasdax98 marked this conversation as resolved.
Show resolved Hide resolved
- `ContentScope` is provided as an interface and should be augmented in the application.
- There is no custom type for permissions, they are reflected as plain strings.
Loading
Loading