Skip to content

Commit

Permalink
feat: Add ability to add/delete relationship for all actors (#3254)
Browse files Browse the repository at this point in the history
## Relevant issue(s)
Resolves #3255 

## Description
- Can target all actors using `"*"` to add or delete acp relationships.
- All explicitly added relationships are unaffected upon revocation
using `"*"` (they will keep access).

### For Reviewers
- Commit by commit review should be easier.
- [x] todo: Pushing the crashing gql tests fix once
#3267 is merged

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] 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?
- Integration tests

Specify the platform(s) on which this was tested:
- Manjaro WSL2
  • Loading branch information
shahzadlone authored Nov 26, 2024
1 parent e2c657e commit c7b8b93
Show file tree
Hide file tree
Showing 13 changed files with 1,278 additions and 73 deletions.
40 changes: 40 additions & 0 deletions acp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,26 @@ Result:
Error: document not found or not authorized to access
```

Sometimes we might want to give a specific access (form a relationship) not just to one identity, but any identity.
In that case we can specify "*" instead of specifying an explicit `actor`:
```sh
defradb client acp relationship add \
--collection Users \
--docID bae-ff3ceb1c-b5c0-5e86-a024-dd1b16a4261c \
--relation reader \
--actor "*" \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac
```

Result:
```json
{
"ExistedAlready": false
}
```

**Note: specifying `*` does not overwrite any previous formed relationships, they will remain as is **

### Revoking Access To Private Documents

To revoke access to a document for an actor, we must delete the relationship between the
Expand Down Expand Up @@ -695,6 +715,26 @@ defradb client collection docIDs --identity 4d092126012ebaf56161716018a71630d994

**Result is empty from the above command**

We can also revoke the previously granted implicit relationship which gave all actors access using the "*" actor.
Similarly we can just specify "*" to revoke all access given to actors implicitly through this relationship:
```sh
defradb client acp relationship delete \
--collection Users \
--docID bae-ff3ceb1c-b5c0-5e86-a024-dd1b16a4261c \
--relation reader \
--actor "*" \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac
```

Result:
```json
{
"RecordFound": true
}
```

**Note: Deleting with`*` does not remove any explicitly formed relationships, they will remain as they were **

## DAC Usage HTTP:

### Authentication
Expand Down
36 changes: 34 additions & 2 deletions acp/acp_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,25 @@ func (l *ACPLocal) AddActorRelationship(

ctx = auth.InjectPrincipal(ctx, principal)

var newActorRelationship *types.Relationship
if targetActor == "*" {
newActorRelationship = types.NewAllActorsRelationship(
resourceName,
objectID,
relation,
)
} else {
newActorRelationship = types.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
)
}

setRelationshipRequest := types.SetRelationshipRequest{
PolicyId: policyID,
Relationship: types.NewActorRelationship(resourceName, objectID, relation, targetActor),
Relationship: newActorRelationship,
CreationTime: creationTime,
}

Expand Down Expand Up @@ -285,9 +301,25 @@ func (l *ACPLocal) DeleteActorRelationship(

ctx = auth.InjectPrincipal(ctx, principal)

var newActorRelationship *types.Relationship
if targetActor == "*" {
newActorRelationship = types.NewAllActorsRelationship(
resourceName,
objectID,
relation,
)
} else {
newActorRelationship = types.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
)
}

deleteRelationshipRequest := types.DeleteRelationshipRequest{
PolicyId: policyID,
Relationship: types.NewActorRelationship(resourceName, objectID, relation, targetActor),
Relationship: newActorRelationship,
}

deleteRelationshipResponse, err := l.engine.DeleteRelationship(ctx, &deleteRelationshipRequest)
Expand Down
64 changes: 42 additions & 22 deletions acp/acp_source_hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,18 +273,28 @@ func (a *acpSourceHub) AddActorRelationship(
creationTime *protoTypes.Timestamp,
) (bool, error) {
msgSet := sourcehub.MsgSet{}

var newActorRelationship *acptypes.Relationship
if targetActor == "*" {
newActorRelationship = acptypes.NewAllActorsRelationship(
resourceName,
objectID,
relation,
)
} else {
newActorRelationship = acptypes.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
)
}

cmdMapper := msgSet.WithBearerPolicyCmd(&acptypes.MsgBearerPolicyCmd{
Creator: a.signer.GetAccAddress(),
BearerToken: requester.BearerToken,
PolicyId: policyID,
Cmd: acptypes.NewSetRelationshipCmd(
acptypes.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
),
),
Creator: a.signer.GetAccAddress(),
BearerToken: requester.BearerToken,
PolicyId: policyID,
Cmd: acptypes.NewSetRelationshipCmd(newActorRelationship),
CreationTime: creationTime,
})
tx, err := a.txBuilder.Build(ctx, a.signer, &msgSet)
Expand Down Expand Up @@ -323,18 +333,28 @@ func (a *acpSourceHub) DeleteActorRelationship(
creationTime *protoTypes.Timestamp,
) (bool, error) {
msgSet := sourcehub.MsgSet{}

var newActorRelationship *acptypes.Relationship
if targetActor == "*" {
newActorRelationship = acptypes.NewAllActorsRelationship(
resourceName,
objectID,
relation,
)
} else {
newActorRelationship = acptypes.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
)
}

cmdMapper := msgSet.WithBearerPolicyCmd(&acptypes.MsgBearerPolicyCmd{
Creator: a.signer.GetAccAddress(),
BearerToken: requester.BearerToken,
PolicyId: policyID,
Cmd: acptypes.NewDeleteRelationshipCmd(
acptypes.NewActorRelationship(
resourceName,
objectID,
relation,
targetActor,
),
),
Creator: a.signer.GetAccAddress(),
BearerToken: requester.BearerToken,
PolicyId: policyID,
Cmd: acptypes.NewDeleteRelationshipCmd(newActorRelationship),
CreationTime: creationTime,
})

Expand Down
8 changes: 8 additions & 0 deletions cli/acp_relationship_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ Example: Let another actor (4d092126012ebaf56161716018a71630d99443d9d5217e9d8502
--actor did:key:z7r8os2G88XXBNBTLj3kFR5rzUJ4VAesbX7PgsA68ak9B5RYcXF5EZEmjRzzinZndPSSwujXb4XKHG6vmKEFG6ZfsfcQn \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac
Example: Let all actors read a private document:
defradb client acp relationship add \
--collection Users \
--docID bae-ff3ceb1c-b5c0-5e86-a024-dd1b16a4261c \
--relation reader \
--actor "*" \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac
Example: Creating a dummy relationship does nothing (from database perspective):
defradb client acp relationship add \
-c Users \
Expand Down
9 changes: 7 additions & 2 deletions client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ type DB interface {
// If failure occurs, the result will return an error. Upon success the boolean value will
// be true if the relationship already existed (no-op), and false if a new relationship was made.
//
// Note: The request actor must either be the owner or manager of the document.
// Note:
// - The request actor must either be the owner or manager of the document.
// - If the target actor arg is "*", then the relationship applies to all actors implicitly.
AddDocActorRelationship(
ctx context.Context,
collectionName string,
Expand All @@ -128,7 +130,10 @@ type DB interface {
// be true if the relationship record was found and deleted. Upon success the boolean value
// will be false if the relationship record was not found (no-op).
//
// Note: The request actor must either be the owner or manager of the document.
// Note:
// - The request actor must either be the owner or manager of the document.
// - If the target actor arg is "*", then the implicitly added relationship with all actors is
// removed, however this does not revoke access from actors that had explicit relationships.
DeleteDocActorRelationship(
ctx context.Context,
collectionName string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ Example: Let another actor (4d092126012ebaf56161716018a71630d99443d9d5217e9d8502
--actor did:key:z7r8os2G88XXBNBTLj3kFR5rzUJ4VAesbX7PgsA68ak9B5RYcXF5EZEmjRzzinZndPSSwujXb4XKHG6vmKEFG6ZfsfcQn \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac

Example: Let all actors read a private document:
defradb client acp relationship add \
--collection Users \
--docID bae-ff3ceb1c-b5c0-5e86-a024-dd1b16a4261c \
--relation reader \
--actor "*" \
--identity e3b722906ee4e56368f581cd8b18ab0f48af1ea53e635e3f7b8acd076676f6ac

Example: Creating a dummy relationship does nothing (from database perspective):
defradb client acp relationship add \
-c Users \
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/acp.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ type AddPolicy struct {
Policy string

// The policy creator identity, i.e. actor creating the policy.
Identity immutable.Option[identityRef]
Identity immutable.Option[identity]

// The expected policyID generated based on the Policy loaded in to the ACP system.
ExpectedPolicyID string
Expand Down Expand Up @@ -159,13 +159,13 @@ type AddDocActorRelationship struct {
// The target public identity, i.e. the identity of the actor to tie the document's relation with.
//
// This is a required field. To test the invalid usage of not having this arg, use NoIdentity() or leave default.
TargetIdentity immutable.Option[identityRef]
TargetIdentity immutable.Option[identity]

// The requestor identity, i.e. identity of the actor creating the relationship.
// Note: This identity must either own or have managing access defined in the policy.
//
// This is a required field. To test the invalid usage of not having this arg, use NoIdentity() or leave default.
RequestorIdentity immutable.Option[identityRef]
RequestorIdentity immutable.Option[identity]

// Result returns true if it was a no-op due to existing before, and false if a new relationship was made.
ExpectedExistence bool
Expand Down Expand Up @@ -251,13 +251,13 @@ type DeleteDocActorRelationship struct {
// The target public identity, i.e. the identity of the actor with whom the relationship is with.
//
// This is a required field. To test the invalid usage of not having this arg, use NoIdentity() or leave default.
TargetIdentity immutable.Option[identityRef]
TargetIdentity immutable.Option[identity]

// The requestor identity, i.e. identity of the actor deleting the relationship.
// Note: This identity must either own or have managing access defined in the policy.
//
// This is a required field. To test the invalid usage of not having this arg, use NoIdentity() or leave default.
RequestorIdentity immutable.Option[identityRef]
RequestorIdentity immutable.Option[identity]

// Result returns true if the relationship record was expected to be found and deleted,
// and returns false if no matching relationship record was found (no-op).
Expand Down
Loading

0 comments on commit c7b8b93

Please sign in to comment.