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

fix: Make requests with no identity work with "*" target #3278

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion acp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,8 @@ 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.
Sometimes we might want to give a specific access (i.e. form a relationship) not just with one identity, but with
any identity (includes even requests with no-identity).
In that case we can specify "*" instead of specifying an explicit `actor`:
```sh
defradb client acp relationship add \
Expand Down
14 changes: 9 additions & 5 deletions internal/db/permission/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,22 @@ func CheckAccessOfDocOnCollectionWithACP(
return true, nil
}

// At this point if the request is not signatured, then it has no access, because:
// the collection has a policy on it, and the acp is enabled/available,
// and the document is not public (is registered with acp).
var identityValue string
if !identity.HasValue() {
return false, nil
// We can't assume that there is no-access just because there is no identity even if the document
// is registered with acp, this is because it is possible that acp has a registered relation targeting
// "*" (any) actor which would mean that even a request without an identity might be able to access
// a document registered with acp. So we pass an empty `did` to accommodate that case.
shahzadlone marked this conversation as resolved.
Show resolved Hide resolved
identityValue = ""
} else {
identityValue = identity.Value().DID
}

// Now actually check using the signature if this identity has access or not.
hasAccess, err := acpSystem.CheckDocAccess(
ctx,
permission,
identity.Value().DID,
identityValue,
policyID,
resourceName,
docID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ func TestACP_OwnerMakesAManagerThatGivesItSelfReadAndWriteAccess_GQL_ManagerCanR

Description: "Test acp, owner makes a manager that gives itself read and write access",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used so test that separately.
testUtils.GQLRequestMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used (only for update requests),
// so test that separately.
testUtils.GQLRequestMutationType,
},
),

Actions: []any{
testUtils.AddPolicy{
Expand Down Expand Up @@ -274,10 +277,13 @@ func TestACP_OwnerMakesManagerButManagerCanNotPerformOperations_GQL_ManagerCantR

Description: "Test acp, owner makes a manager, manager can't read or write",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used so test that separately.
testUtils.GQLRequestMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used (only for update requests),
// so test that separately.
testUtils.GQLRequestMutationType,
},
),

Actions: []any{
testUtils.AddPolicy{
Expand Down Expand Up @@ -442,10 +448,13 @@ func TestACP_ManagerAddsRelationshipWithRelationItDoesNotManageAccordingToPolicy

Description: "Test acp, manager adds relationship with relation it does not manage according to policy, error",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used so test that separately.
testUtils.GQLRequestMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used (only for update requests),
// so test that separately.
testUtils.GQLRequestMutationType,
},
),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,12 @@ func TestACP_OwnerMakesAManagerThatGivesItSelfReadAndWriteAccess_ManagerCanReadA

Description: "Test acp, owner makes a manager that gives itself read and write access",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used with gql (only for update requests),
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),

Actions: []any{
testUtils.AddPolicy{
Expand Down Expand Up @@ -849,10 +851,12 @@ func TestACP_ManagerAddsRelationshipWithRelationItDoesNotManageAccordingToPolicy

Description: "Test acp, manager adds relationship with relation it does not manage according to policy, error",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used with gql (only for update requests),
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),

Actions: []any{
testUtils.AddPolicy{
Expand Down Expand Up @@ -1017,10 +1021,12 @@ func TestACP_OwnerMakesManagerButManagerCanNotPerformOperations_ManagerCantReadO

Description: "Test acp, owner makes a manager, manager can't read or write",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used with gql (only for update requests),
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQ

Description: "Test acp, owner gives write(update) access without explicit read permission, can still update",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used so test that separately.
testUtils.GQLRequestMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used (only for update requests),
// so test that separately.
testUtils.GQLRequestMutationType,
},
),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot

Description: "Test acp, owner gives write(update) access without explicit read permission, can still update",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used with gql (only for update requests),
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ func TestACP_OwnerGivesOnlyReadAccessToAnotherActor_GQL_OtherActorCanReadButNotU

Description: "Test acp, owner gives read access to another actor, but the other actor can't update",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used so test that separately.
testUtils.GQLRequestMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
// GQL mutation will return no error when wrong identity is used (only for update requests),
// so test that separately.
testUtils.GQLRequestMutationType,
},
),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,11 @@ func TestACP_OwnerGivesOnlyReadAccessToAnotherActor_OtherActorCanReadButNotUpdat

Description: "Test acp, owner gives read access to another actor, but the other actor can't update",

SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),
SupportedMutationTypes: immutable.Some(
[]testUtils.MutationType{
testUtils.CollectionNamedMutationType,
testUtils.CollectionSaveMutationType,
}),

Actions: []any{
testUtils.AddPolicy{
Expand Down
Loading
Loading