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.

Sign up for GitHub

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

Jump to bottom

[EPIC] Integration of ACP #1738

Open
6 of 23 tasks
fredcarle opened this issue Jul 31, 2023 · 0 comments
Open
6 of 23 tasks

[EPIC] Integration of ACP #1738

fredcarle opened this issue Jul 31, 2023 · 0 comments
Assignees
@shahzadlone
Labels
area/acp Related to the acp (access control) system epic Includes sub-issues feature New feature or request priority/high security Related to security
Milestone
DefraDB v0.13

Comments

@fredcarle
Copy link
Collaborator

fredcarle commented Jul 31, 2023 •
edited by islamaliev
Loading

This epic tracks the list of tasks related to integrating the ACP into DefraDB.

After initial implementation lands, here are the next steps:

  1. The following features currently are not allowed or don't work (these can be worked on by the team in a parallel manner):

Note: The above (1) features don't require to expand or tinker with the ACP interface defined in the ./acp package at all. When adding tests don't forget to add a use case example to the README in the ./acp folder.

  1. The following features may have undefined/unstable behavior and should be assumed to not be working until proper testing is added for them, so add testing for these:

Note: The above (2) features likely might just need to add tests, when adding tests don't forget to add a use case example to the README in the ./acp folder.

  1. Somewhat requires ACP/Sourcehub/Identity Knowledge, better to pick above issues if they are available by most devs:

Future features:

Resolved:
@fredcarle fredcarle added feature New feature or request epic Includes sub-issues security Related to security labels Jul 31, 2023
@fredcarle fredcarle added this to the DefraDB v0.7 milestone Jul 31, 2023
@shahzadlone shahzadlone modified the milestones: DefraDB v0.7, DefraDB v0.8 Sep 14, 2023
@shahzadlone shahzadlone modified the milestones: DefraDB v0.8, DefraDB v0.9 Nov 9, 2023
@fredcarle fredcarle modified the milestones: DefraDB v0.9, DefraDB v0.10 Jan 19, 2024
@shahzadlone shahzadlone added the area/acp Related to the acp (access control) system label Feb 29, 2024
@shahzadlone shahzadlone mentioned this issue Feb 29, 2024
Merged
19 tasks
This was referenced Apr 1, 2024
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Open
shahzadlone added a commit that referenced this issue Apr 3, 2024
@shahzadlone
feat: Add Access Control Policy (#2338)
a46c1fa 
## Relevant issue(s)
Part of Epic #1738
Resolves #2019
Resolves #2020
Resolves #2228

## Description
- [Rendered ACP
Doc](https://github.com/shahzadlone/defradb/blob/lone/acp-sourcehub-module/acp/README.md)
- Introduces an ACP Interface that needs to be satisfied for any type of
access control module.
- This PR implements a local embedded access control system for DefraDB
by implementing the ACP interface.
- There should be no remote calls (everything introduced here is local).
- There should be no blocking of any kind, no async stuff was
introduced.
- Documents now have the following three states, from a requester's POV:
    - Public document (always accessible).
    - Accessible document (accessible if requested by the owner)
- Inaccessible/private document (can't access if requested by non-owner
or non-identity request)
- Ability to add permissioned schema.
- Added permissioned schema examples.
- Ability to add policy.
- Added policy examples (JSON and YAML).
- Ability to specify simple identity addresses on requests.
- Mocks and Documents Generated (old and new).

### Demo
```go
AddPolicy{
	Creator: "source1zzg43wdrhmmk89z3pmejwete2kkd4a3vn7w969",
	Policy: `
		description: a test policy which marks a collection in a database as a resource

		actor:
		  name: actor

		resources:
		  users:
			permissions:
			  read:
				expr: owner + reader
			  write:
				expr: owner

			relations:
			  owner:
				types:
				  - actor
			  reader:
				types:
				  - actor
			  admin:
				manages:
				  - reader
				types:
				  - actor
	`,
	ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001",
}

SchemaUpdate{
	Schema: `
		type Users @Policy(id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", resource: "users") {
			name: String
			age: Int
		}
	`,
}

CreateDoc{
	Identity: "source1zzg43wdrhmmk89z3pmejwete2kkd4a3vn7w969",
	Doc: `{"name": "John", "age": 27 }`,
}

Request{
	Identity: "source1zzg43wdrhmmk89z3pmejwete2kkd4a3vn7w969",
	Request: `
		query {
			Users {
				_docID
				name
				age
			}
		}
	`,
	Results: []map[string]any{
		{
			"_docID": "bae-88b63198-7d38-5714-a9ff-21ba46374fd1",
			"name":   "John",
			"age":    int64(27),
		},
	},
}

// The Following Requests Don't Have Access

Request{
	Request: `
		query {
			Users {
				_docID
				name
				age
			}
		}
	`,
	Results: []map[string]any{},
}

Request{
	Identity: "cosmos1x25hhksxhu86r45hqwk28dd70qzux3262hdrll".
	Request: `
		query {
			Users {
				_docID
				name
				age
			}
		}
	`,
	Results: []map[string]any{},
}

```

### Features
- [x] In-memory and filebased acp module.
- [x] Registering of a document with ACP Module on creation.
- [x] Add policy command HTTP & CLI.
- [x] Detect Policy Marshal Format on adding.
- [x] Add permissioned schema.
- [x] Reject schema on validation failure.
- [x] Accept schema on validation success.
- [x] Permissioned Fetcher
- [x] Specify Identity on doc create.
- [x] Specify Identity on doc delete.
- [x] Specify Identity on doc update.
- [x] Specify Identity on request.


#### Things That Are In Scope Of This PR:
- All previous features should behave as they were.
- There should be no performance costs/hits to any previous
functionality.
- Access Control would only be registered for a document on a collection
if:
    - Have ACP Module initialized / avaialable programatically
    - The creation request had an Indentity attached.
- The collection has a permission on it (i.e. collection is
permissioned).
- Access Controled Read/Write, after Creation and Registering:
- If there is no ACP module, have access to all documents (as if acp is
turned off).
- If there is no Indentity can only operate on public documents
(unregistered documents with acp).
- If there is Permissioned Collection, Identity and ACPModule then
operate on access controled document.
- Cosmos Identity Addresses.
- Adding of a policy (CLI & HTTP), Tests:
./tests/integration/acp/add_policy
- Validation of Linked Policy Resource DPI on Schema Add:
    - Accepting of a permissioned schema on Valid DPI Resource
    - Rejecting of a permissioned schema on Invalid DPI Resource
    - Tests for both here: ./tests/integration/acp/schema/add_dpi


#### Things That Are Out Of Scope Of This PR:
- Full Fledged Indentity with Authentication
- Using ACP with any other feature is not supported.
    - P2P
    - Secondary Indexes
    - Type Joins
    - Aggregates
    - Backup & Recover
    - Views
    - Lens


#### De-scope to after merge.
- Update tracking issue with road-map and priority of next tasks.
- Add simple identity generate utility command
- Add simple identity validation utility command
- Fix the identity validation panic

### For Reviewers:

#### Recommendations:
- To begin might want to read and familiarize yourself with some
material under:
-
[acp/README](https://github.com/shahzadlone/defradb/blob/lone/acp-sourcehub-module/acp/README.md)
- When looking at tests can read more about what they test in:
- [ACP Test
Structure](https://github.com/shahzadlone/defradb/blob/lone/acp-sourcehub-module/tests/integration/acp/README.md)
- [Add Policy
Tests](https://github.com/shahzadlone/defradb/blob/lone/acp-sourcehub-module/tests/integration/acp/add_policy/README.md)
- [Add Schema With DPI
Tests](https://github.com/shahzadlone/defradb/blob/lone/acp-sourcehub-module/tests/integration/acp/schema/add_dpi/README.md)
- Would highly encourage commit by commit review.

#### Commit Priorities:
- Commits with label `PR(-)` are unrelated/irrelevant to this PR, can be
ignored.
- Commits with label `PR(ACP)` are most important to review as they are
specific to ACP implementation.
- Commits with label `PR(IDENTITY)` are also important as they are
specific to Indentity implementation.
- Commits with label `PR(*-TEST)` are test related commits that should
be looked at.
- Commits with label `PR(ACP-*)` are assisting ACP commits (medium
priority).
- Commits with label `PR(IDENTITY-*)` are assisting ACP commits (medium
priority).
- Commits with label `PR(WIP)` Should not exist before merge (work in
progress commits).
- Commits with label `PR(DROP)` Temporary commits that will be dropped
before the merge.

## Tasks
- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [ ] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?
- [x] There are add policy tests
- [x] There are adding of permissioned schema (with dpi) tests.
- [x] There are end-to-end tests with doc creation and read using
identity.

Specify the platform(s) on which this was tested:
- Manjaro WSL2
@shahzadlone shahzadlone mentioned this issue Apr 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shahzadlone shahzadlone

Labels
area/acp Related to the acp (access control) system epic Includes sub-issues feature New feature or request priority/high security Related to security
Projects
None yet
Milestone
DefraDB v0.13
Development

No branches or pull requests
3 participants
@fredcarle @shahzadlone @AndrewSisley