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

ACP Draft design flaw #277

Closed
2 tasks
matthieubosquet opened this issue Nov 20, 2021 · 18 comments · Fixed by #300
Closed
2 tasks

ACP Draft design flaw #277

matthieubosquet opened this issue Nov 20, 2021 · 18 comments · Fixed by #300

Comments

@matthieubosquet
Copy link
Member

matthieubosquet commented Nov 20, 2021

On proposal

ACP: Remove the acp:apply and acp:access predicates in favour of a single acp:policy predicate.

Details

Description

ACP is a data model to express the conditions of access to resources.

An ACP Access Control Resource mandates access over a resource.

Why talk about controlling access to Access Control Resources when the spec does not talk about controlling access to Access Controls, Policies and Matchers?

The only thing ACP is talking about is controlling access to resources and it should not talk about controlling access to Access Control Resources because there is nothing special about them.

RDF and Cool URIs talk about what a resource is, ACP is orthogonal to those documents.

The Solid Protocol defines the relationship between a resource and its auxiliary resources, for example authorization resources, ACP is orthogonal to the Solid Protocol.

In the case of the Solid Protocol, an authorization resource lifecycle depends on the lifecycle of the subject resource that they are associated with.

Currently, the ACP spec treats the Access Control Resource as a specifically separate resource with its own access modes which is incorrect, hints at a separate lifecycle and is hardly compliant with Solid.

The ACP specification should remove every mention of an ACR mandating control over itself.

It is not up to ACP to mandate that permissions over an Access Control Resource will be mandated by another Access Control Resource (ACR on ACR), by a specific access mode (the Solid compliant acl:Control) or by another mechanism.

Logically, this also entails removing the acp:access and acp:apply predicates that invent a new way of mandating control over Access Control Resources hence implying that they are special, as opposed to Access Controls, Policies and Matchers.

The acp:access and acp:apply predicate should be removed in favour of a neutral and spec orthogonal predicate acp:policy.

Details of the current situation

In ACP there are currently two predicates to link an Access Control to a Policy:

  • acp:apply links an Access Control to the policies applying to the Resource itself:
    :acr acp:resource :resource .
    :acr acp:accessControl :acl .
    :acl acp:apply :policy .
    # :policy applies to :resource
  • acp:access links an Access Control to the policies applying to the Access Control Resource:
    :acr acp:resource :resource .
    :acr acp:accessControl :acl .
    :acl acp:access :policy .
    # :policy applies to :acr

Drawbacks from an implementers’ perspective in addition to the fundamental design flaw might be worth mentioning:

  • The distinction between acp:apply and acp:access is confusing, that is, the semantics of these predicates is unclear/misleading and has previously been discussed as far from ideal.
  • The distinction hints that there would be a separate lifecycle for a resource and access control resource which is not the case in Solid.
  • The distinction drives ACP away from Solid Protocol compatibility where the access modes are/should be defined as entailing operations and where acl:Control is used.
  • Policies can be reused and applied to both Access Control Resources and Resources with no indication whatsoever of which. That makes it easy to over-permission a Policy and is a security concern.
  • It is inconsistent with the rest of the ACP data model.

Proposal

The proposed solution would be to use a single acp:policy predicate which would remove the design flaw, spec orthogonality issues and aforementioned implementers’ problems:

  • acp:policy links an Access Control to a Policy:
    :acr acp:resource :resource .
    :acr acp:accessControl :acl .
    :acl acp:policy :policy .
    :policy acp:allow acl:Read, acl:Write, acl:Control .

Acceptance criteria

What actions are needed to resolve this issue?

  • Remove acp:access and acp:apply from the ACP Draft in favour of acp:policy
  • Ammend all text in the spec that relates to those predicates
@elf-pavlik
Copy link
Member

https://solid.github.io/authorization-panel/acp-specification/#access-control-resource

acp:resource
The ACP resource predicate links an ACP Access control resource to the resource over which it mandates access. acp:resource has a range of rdfs:Resource and a domain of acp:AccessControlResource. An ACP Access control resource only ever mandates access over one resource and inversely a resource only ever has one ACP Access control resource mandating access over it. The one-to-one cardinality of acp:resource is denoted by it being an owl:FunctionalProperty & owl:InverseFunctionalProperty. The acp:resource predicate is the inverse of acp:accessControlResource.

Could you provide an example of ACR that includes policies over the resource, which ACR is auxiliary of, and policies over itself? Especially what would be referenced with acp:resource.

@matthieubosquet
Copy link
Member Author

@elf-pavlik I think you're pointing out an interesting ambiguity/potential inconsistency. The acp:resource predicate should probably as per being the inverse of acp:accessControlResource be called acp:isAccessControlResourceFor.

I think such inverse properties could hold for each of the ACP elements, that is: acp:isAccessControlFor; acp:isPolicyFor and acp:isMatcherFor...

To respond more directly to your question:

  1. Let resource x be linked to resource y via a Link rel="acl" header
  2. Resource y is therefore the Access Control Resource for resource x
  3. Let resource y contains the following graph:
    <> acp:accessControl <#ac1> .
    <#ac1> acp:policy <#p1> .
    <#p1> acp:allow acl:Read, acl:Write, acl:Control .
  4. In the RDF representation of resource y, policy <#p1> gives Read, Write and Control access over resource x (hence allowing editing of resource y itself as well).

@acoburn
Copy link
Member

acoburn commented Nov 22, 2021

I am fine with removing acp:access.
I am 👎 on changing acp:apply to acp:policy. Doing so will break all existing clients and servers, and for what reason?

@matthieubosquet
Copy link
Member Author

If the acp:access predicate is removed, there is no reason to have an inconsistent ontology. And the cognitive cost if that ontology is to be used and adopted widely which is what we're aiming for will be on the whole much higher than having this fixed now.

The schema allowing us to have:

R1 --acp:accessControlResource--> ACR1 --acp:accessControl--> AC1 --acp:policy--> P1

And inverse:

P1 --acp:isPolicyFor--> AC1 --acp:isAccessControlFor--> ACR1 --acp:isAccessControlResourceFor--> R1

Is much cleaner/intuitive/consistent/easy to describe and understand.

I do get your point about existing clients and servers, but there will be no moment in the future where such change will be even remotely possible or acceptable. I am very willing to make the effort right now for the sake of consistency and coherence.

@elf-pavlik
Copy link
Member

In the RDF representation of resource y, policy <#p1> gives Read, Write and Control access over resource x (hence allowing editing of resource y itself as well).

I really don't like acl:Control access mode. If one wants to give someone read access to ACR that would require adding acl:ControlRead or something like that. I think using modes like Read or Write on ACR as well is very elegant in current version of ACP.

If you have acp:resource owl:inverseOf acp:accessControlResource (let's forget any possible renaming for now), to have ACR have their own ACR one could use acp:accessControlResource as link relation in HTTP header. (forgetting rel="acl" option for now). If one would use that and make ACR referencing itself as its own ACR, which would be similar to current approach of using acp:access. Than we would have ACR linking to two different resources with acp:resource. This would not allow setting separate policies on ACR and the other resource.

Alternative would be removing acp:resource and don't even bother defining inverse for acp:accessControlResource, one can always swap subject and object in the statement. Having that, we can define property with domain acp:AccessControl named something like acp:policiesApplyTo which would have any resource (including ACR) as range. This way we could have more than one resource linking to the same ACR and each Access Control would apply to one specific resource. This would preserve use of plain Read, Write access modes on ACRs.

@matthieubosquet
Copy link
Member Author

matthieubosquet commented Nov 22, 2021

If the acl:ControlRead mode is actually something that is required in Solid, it should be discussed and added.

I think we need to have a crystal clear map of which modes entail which operations in the Solid Protocol. And that is not ACP specific.

Now, when it comes to ACP, there is no reason for ACP to have two ways of determining access to resources documented within the specification.

So if we want to have ACRs on ACRs in Solid, we should have that properly discussed and there is nothing in the ACP data model that technically stops you from writing ex:acr1 acp:accessControlResource ex:acr2 .. So there is really no reason whatsoever to have two predicates to link Access Controls to policies.

Having that, we can define property with domain acp:AccessControl named something like acp:policiesApplyTo which would have any resource (including ACR) as range.

The range is an inference property, so it would not have "any" in range. I don't know where you're going really with such a predicate. What you're describing is: ex:AC1 acp:policiesApplyTo ex:ACR1 . which doesn't make sense. It also hints towards the fact that the current domain model is indeed misleading and ambiguous.

The chain is:

resource -> access control resource -> access control -> policy -> matcher

Note that access control resources, access controls, policies and matchers are resources (maybe the name access control resource is also confusing and access control list would be better and less inducing of the wrong idea that "resource" stands for a document). All of the nodes in an ACP authorization graph could be document or non-document resources, as per Cool URIs; it is not up to ACP to determine that.

@elf-pavlik
Copy link
Member

As I understand, the change you propose would not allow using common Read, Write modes on ACRs and would require dedicated modes like ControlRead, 'Control(Write)` to set policies on ACRs?

resource -> access control resource -> access control -> policy -> matcher

That means that in ACR polices can be set just on a single resource which links to that ACR.

So if we want to have ACRs on ACRs in Solid, we should have that properly discussed and there is nothing in the ACP data model that technically stops you from writing ex:acr1 acp:accessControlResource ex:acr2 .

It doesn't seem that ACR can reference itself to include policies on itself

ex:res1 acp:accessControlResource ex:acr1 .
ex:acr1 acp:accessControlResource ex:acr1 .

With above it would not be possible to set different policies on res1 and acr1

@matthieubosquet
Copy link
Member Author

What you are highlighting here is not meant to be valid because one of the restrictions of ACP which I am not propsing to remove here is that one access control resource has one resource. Therefore in the graph you propose:

ex:res1 acp:accessControlResource ex:acr1 .
ex:acr1 acp:accessControlResource ex:acr1 .

The RDFS Resources ex:res1 and ex:acr1 would be inferred to be the same.

However, there is nothing, even in the current situation, making the following graph problematic:

ex:res1 acp:accessControlResource ex:acr1 .
ex:acr1 acp:accessControlResource ex:acr2 .

If we did remove owl:InverseFunctionalProperty from the acp:accessControlResource property, a self referencing Access Control Resource would become sound and that might actually be a good idea because it is a simple level of expressivity we might not want to miss out on.

So in any case, there is nothing really preventing from expressing the second graph which would allow using acl:Read and acl:Write modes on an ACR as of today (to be clear, I am not advocating adopting such a solution in the Solid ecosystem, doing so would require time, evaluation, implementation feedback). Once again, I think that it is a Solid problem, not an ACP problem:

It is not up to ACP to mandate that permissions over an Access Control Resource will be mandated by another Access Control Resource (ACR on ACR), by a specific access mode (the Solid compliant acl:Control) or by another mechanism.


ACP helps you define authorization graphs and match them against request descriptions to determine access modes granted. Solid or any other protocol deciding to implement and use ACP can use those interfaces: Feed context graph; get back an access grant graph.

Solid on the other hand defines the details of interacting with HTTP addressable resources. It is up to Solid to define the details of how authorization auxiliary resources are discovered and which access modes entail which Solid operations. If there is a requirement for ControlRead, it should be brought up discussed and potentially added to the acl ontology (it feels like that one might get through given enough implementation feedback which we currently lack). Both WAC and ACP could use this mode. No need to break compatibility (including with the WAC-Allow header which turns out quite useful from an app developer's perspective). If a requiremeent for ControlAppend becomes obvious, it should be also discussed (I don't think that one is a good idea).

There are a number of discussions about which access modes are actually required by Solid and we should not bypass those:

@elf-pavlik
Copy link
Member

If we did remove owl:InverseFunctionalProperty from the acp:accessControlResource property, a self referencing Access Control Resource would become sound and that might actually be a good idea because it is a simple level of expressivity we might not want to miss out on.

Even if owl:InverseFunctionalProperty gets removed, one will not be able to set different policies on the resource and different set of policies on ACR . acp:access is a current way to apply specific policies to ACR, by removing it there will be no way to assign the regular policy to ACR, instead it would require a workaround like extra access modes when formulating policies that apply to the non ACR resource.

@woutermont
Copy link
Contributor

woutermont commented Nov 23, 2021

@elf-pavlik

I really don't like acl:Control access mode. If one wants to give someone read access to ACR that would require adding acl:ControlRead or something like that. I think using modes like Read or Write on ACR as well is very elegant in current version of ACP.

@matthieubosquet

If the acl:ControlRead mode is actually something that is required in Solid, it should be discussed and added.

This is actually quite an important point, which we already bumped into on a number of projects. The ability for a data owner (or other high-control user) to grant another user partial control access is a huge desirable. So, while I agree with a lot of what @matthieubosquet points out (orthogonality of Solid and ACP, current vocabulary being confusing ...), we should take care that if a switch is made to a single predicate, it is accompanied by another way to achieve this. Constructs like acl:ControlRead can do that, but then we gain clearer orthagonality on one issue by losing it in another.

@matthieubosquet
Copy link
Member Author

Constructs like acl:ControlRead can do that, but then we gain clearer orthagonality on one issue by losing it in another.

Could you expand on what mean by that woutermont?

@bblfish
Copy link
Member

bblfish commented Nov 24, 2021

I like the simplification.

@woutermont
Copy link
Contributor

@matthieubosquet, after reading the panel minutes of yesterday, I feel like not everyone is aligned on the meaning of ControlRead. To clarify: what is needed, is a way to state that someone can control the access rules for a specific access modes, and not others. Since this should be possible for every access mode, of which there can (by extension) be any number, this constitutes an orthogonal dimension to that of the modes themselves.

So, given, a resource R, and it's Access Control Resource A, we should be able to constrain:

  • access to read R (i.e. acl:Read)

  • access to write to R (i.e. acl:Write)

  • any number of other kinds of access to R (e.g. append, list, some yet unknown modes...)

  • access to control read access to R (i.e. what I meant by acl:ControlRead)

  • access to control write access to R (e.g. acl:ControlWrite)

  • access to control any number of other kinds of access to R

(Note that these are all different access control modes for R, not A; for example: setting acl:Read for A is not the same as setting acl:ControlRead for R.)

Given that the "control modes" will always run parallel to the "base access modes" (this is the orthogonality), it is semantically weird to have define them separately (e.g. acl:Read and acl:ControlRead); and that is what something like acp:access+acl:Read and acp:apply+acl:Read could capture nicely.

Long story short: I agree with your intuition about the design flaw (ACRs are just resources), but I would redefine acp:access, instead of dropping it.

@matthieubosquet
Copy link
Member Author

@woutermont my proposal is not about the nature of the acl:Control access mode or its potential specialisations, say acl:ControlRead. In fact, it is irrelevant to what I am describing.

ACP is a data model that is here to describe the conditions of access to a resource. As I have described in another comment already, the acp:access predicate is unnecessary even if you wanted to define access to an ACR and apply to it the modes acl:Read and acl:Write.

Let us have an example Solid resource ex:SolidResourceA linked via the acl header to an example auxiliary authorization resource ex:SolidAuxiliaryACPAuthorizationResourceB. That auxiliary authorization resource's RDF representation could very well be the following graph:

ex:SolidResourceA acp:accessControlResource ex:SolidAuxiliaryACPAuthorizationResourceB .

ex:SolidAuxiliaryACPAuthorizationResourceB acp:accessControl ex:ac1 .

ex:ac1 acp:policy ex:p1 .

ex:p1
  acp:allow acl:Read, acl:Write ;
  acp:anyOf ex:resourceAdminMatcher .

ex:resourceAdminMatcher
  acp:agent ex:bob, ex:alice .

# And also:
ex:SolidAuxiliaryACPAuthorizationResourceB acp:accessControlResource ex:acr1 .

ex:acr1 acp:accessControl ex:ac1 .

ACP would understand the above graph as controling access to both the resource and its auxiliary authz resource via the access control ac1, policy p1 and matcher resourceAdminMatcher. Hence it would grant the access modes acl:Read and acl:Write over both ex:SolidResourceA and ex:SolidAuxiliaryACPAuthorizationResourceB.

Whether the Solid Protocol accepts that the acl:Read and acl:Write modes can be granted over an auxiliary resource is a different debate.

The acp:access predicate is a mistake. It really should not exist, there is not reason for it to. I am not talking "intuitively" about it, I realised this fact by implementing ACP multiple times, seeing how damaging to code implementation this bad modelling decision was and reasoning about the nature of the model.

@woutermont
Copy link
Contributor

@matthieubosquet, this is one of those issues where we both agree on the observation, but will keep talking in circles about the conclusion. So, even though I believe it to be a lot more efficient to tackle them together, I'll continue my point in another issue. Apologies for the inconvenience!

@elf-pavlik
Copy link
Member

To clarify: what is needed, is a way to state that someone can control the access rules for a specific access modes, and not others.

I'm planning to submit soon my proposal for delegation, I believe this will provide a more elegant way of sharing access and only require the storage owner to update any ACR in the storage, more precisely their Authorization Server would do it on their behalf - solid/solid-oidc#18 (comment)

https://solid.github.io/authorization-panel/authorization-ucr/#uc-delegation-subset already mentioned that idea of acl:Control can be considered naive, given that via impersonation one can effectively share access they have anyways.

TBC soon in dedicated issue


If we conclude that ACR of ex:SolidResourceA has to include rules for its auxiliary resources (if they can have their own rules). It might be worth introducing separate predicate, for example:

ex:SolidResourceA acp:accessControlResource ex:SolidAuxiliaryACPAuthorizationResourceB .

ex:SolidAuxiliaryACPAuthorizationResourceB acp:accessControl ex:ac1 .

ex:SolidAuxiliaryACPAuthorizationResourceB acp:accessControlForAuxiliary ex:ac5 .

ex:ac1 acp:policy ex:p1 .
# ex:p1 details aren't relevant

ex:ac5 acp:auxiliraryRelation acp:accessControlResource.
ex:ac5 acp:policy ex:p5 .

ex:p5
  acp:allow acl:Read, acl:Write ;
  acp:anyOf ex:acrMatcher .

# ex:acrMatcher details aren't relevant

As we see this would allow Access Control to apply to auxiliary resource based on given relation. It would also allow use of regular access modes like Read, Write.

@matthieubosquet
Copy link
Member Author

matthieubosquet commented Dec 2, 2021

If we conclude that ACR of ex:SolidResourceA has to include rules for its auxiliary resources (if they can have their own rules). It might be worth introducing separate predicate, for example:

@elf-pavlik I think we're not 100% aligned on the meaning of acp:accessControlResource.

This predicate links a resource to the thing that mandates access over it. Therefore it is perfectly possible to have a graph such that:

ex:SolidResourceA acp:accessControlResource ex:SolidAuxiliaryAccessControlResourceA .

ex:SolidAuxiliaryDescriptionForResourceA acp:accessControlResource ex:AccessControlResourceB .

ex:SolidAuxiliaryAccessControlResourceA acp:accessControlResource ex:AccessControlResourceC .

This graph could further contain everything about ex:SolidAuxiliaryAccessControlResourceA, ex:AccessControlResourceB and ex:AccessControlResourceC. This graph could be the representation of the Solid Access Control List auxiliary resource ex:SolidAuxiliaryAccessControlResourceA.

There is no need to create a new predicate for Solid auxiliary resources (or every type or Solid auxiliary resources) because they are resources.

Access Control Resources and therefore Access Controls can already apply to Solid Auxiliary Resources with this simple data model.


Note that this point is exactly why I pondered the idea of having acp:AccessControlList instead of acp:AccessControlResource as a less confusing name, because the ACP ontology defines access to resources and it is orthogonal to Solid's definition of Solid Resources and Solid Auxiliary Resources. Furthermore, very practically/literally, an acp:AccessControlResource is a list of access controls (member or not) and nothing else.


include rules for its auxiliary resources (if they can have their own rules).

Absolutely, we should clarify on the Solid Protocol level which access modes can be granted over Solid Auxiliary Resources as entailing operations on each type thereof AND whether those can be granted separately from the access modes granted over a Solid Resource.

I can imagine for example that it would be quite handy to have only a certain class of agents able to edit the description or shape of a resource. The acp:accessControlResource predicate enables that already.

@elf-pavlik
Copy link
Member

After rethinking the problem I think we don't actually need to set specific policies on ACR itself. Just as the storage owner has implicit access to all the resources in the storage. We are going to have Authorization Server associated with each Resource Server (Storage). I think this associated Authorization Server can have similar implicit access to all ACRs.

In that case, Resource Owner sets policies using their Authorization Agent which results in the generation of Data Grants. Later the Authorization Server associated with Storage (RS) based on those Data Grants can set ACRs on the storage. I think it is not needed or even desired for any party other than AS to modify ACRs. Given all that I see no need to granular access control over ACRs themselves, implicit (as not expressed via ACRs) access for AS associated with RS seems sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants