Conversation
csarven
force-pushed
the
feature/authorization-condition
branch
from
f470bc8 to
6e5a2ec
Compare
uvdsl
left a comment
uvdsl
left a comment
There was a problem hiding this comment.
Thank you @csarven for putting this together!
I left some inline comments. And here are more general questions:
- Do we need to mention the new
linkheader relation in #http-definitions? - Do you consider the section #access-conditions to be appendable? Could we add additional conditions (time, attested attribute, ...) in that section going forward (when consensus is reached in the CG, of course)? Or how do you envision such specification evolution, if at all?
| <p id="consider-acl-resource-activities">Implementations are encouraged to use mechanisms to record activities about ACL resources for the purpose of accountability and integrity, e.g., by having audit trails, notification of changes, reasons for change, preserving provenance information.</p> | ||
| <p id="consider-provenance-accountability">Implementations that want to allow a class of write or control operations on resources are encouraged to require agents to be authenticated, e.g., for purposes of provenance or accountability.</p> | ||
| <p about="" id="consider-client-agnostic-control" rel="spec:advisement" resource="#consider-client-agnostic-control"><span property="spec:statement">Implementations are <span rel="spec:advisementLevel" resource="spec:Encouraged">encouraged</span> to consider scenarios in which Authorizations granting <code>acl:Control</code> are client-agnostic and issuer-agnostic, avoiding inadvertent <a href="#loss-of-control-mitigation" rel="rdfs:seeAlso">loss of control</a> if a client or issuer becomes unavailable or untrustworthy, as well as scenarios in which restricting control access to specific clients or issuers could expose the controller to manipulation through a malicious client or issuer.</span></p> | ||
| <p about="" id="consider-condition-legacy" rel="spec:advisement" resource="#consider-condition-legacy"><span property="spec:statement">Implementations are <span rel="spec:advisementLevel" resource="spec:StronglyEncouraged">strongly encouraged</span> to be aware that Authorizations including <code>acl:condition</code> values evaluated by a server without condition support, such as following data migration from a conditions-aware server, could result in elevated access rights. Implementations are strongly encouraged to verify Authorizations with conditions against the condition capabilities of the server prior to deployment.</span></p> |
There was a problem hiding this comment.
Implementations are strongly encouraged to verify Authorizations with conditions against the condition capabilities of the server prior to deployment.
Would you consider making this statement normative in the discovery section, to be explicit about the expectation that a client may have about a server's support of authorization conditions? This relates to my above comment on the discovery section.
There was a problem hiding this comment.
I think this statement in particular stands as advisory because it is something deployments should be aware of and not something that's testable as a requirement (at least within the scope of this specification).
I've updated the wording on #client-link-condition to signal the purpose better. #acl-condition also mentions discovery of supported condition types, and as a related matter for servers in #server-condition-evaluation that unrecognised is not part of evaluation.
There is always the possibility that a deployment will mess things up, and that's not limited to downgrading from a WAC 1.1 to 1.0 server meanwhile using the same ACL resources. For instance, even while at WAC 1.1. the server may switch authentication mechanisms or even not provide the necessary information to WAC about issuer/client for instance. Of course, these are all possible and some may be (un)likely. But I think the current text in the specification is adequate.
TallTed
left a comment
TallTed
left a comment
There was a problem hiding this comment.
Generally looks OK to me. A few small tweaks to be made.
Note: I have not reviewed with a fine tooth comb. If that is desired, please request another review, preferably with pointer to specific sections about which you're concerned.
| <section id="acl-resource-condition-discovery" inlist="" rel="schema:hasPart" resource="#acl-resource-condition-discovery"> | ||
| <h3 property="schema:name">ACL Resource Condition Discovery</h3> | ||
| <div datatype="rdf:HTML" property="schema:description"> | ||
| <p about="" id="server-link-condition" rel="spec:requirement" resource="#server-link-condition"><span property="spec:statement">When a server wants to enable applications to discover supported condition types (<cite><a href="#access-conditions" rel="rdfs:seeAlso">Access Conditions</a></cite>) for a given <a href="#effective-acl-resource">effective ACL resource</a>, the <span rel="spec:requirementSubject" resource="spec:Server">server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> advertise each supported condition type by responding to an HTTP request on the effective ACL resource including a <code>Link</code> header with the <code>rel</code> value of <code>http://www.w3.org/ns/auth/acl#condition</code> and the condition type as link target [<cite><a class="bibref" href="#bib-rfc8288">RFC8288</a></cite>].</span></p> |
There was a problem hiding this comment.
My first thought would be to discover it in Storage Description, LWS advertises capabilities this way https://w3c.github.io/lws-protocol/lws10-core/#storage-description-capabilities
Does WAC expect that available conditions may differ across resources in the same storage. In that case it seems that mechanism to manage that variability is left out of scope.
There was a problem hiding this comment.
Placing condition signalling in the HTTP response header of the effective ACL resource means clients cannot alter what is supported or enforced for a given resource. Variability across resources is accommodated by the per effective ACL resource discovery and ultimately left to the URI owner to manage as appropriate. How a resource server manages that variability internally is deliberately out of scope, keeping the specification simpler and orthogonal to resource server implementation choices ( #http-interactions ).
Co-authored-by: Jesse Wright <63333554+jeswr@users.noreply.github.com>
Co-authored-by: Christoph Braun <braun@kit.edu>
csarven
force-pushed
the
feature/authorization-condition
branch
from
eb2f500 to
cafa174
Compare
Co-authored-by: Christoph Braun <braun@kit.edu>
|
@uvdsl , thanks for co-authoring and reviewing.
#http-definitions was intended to indicate parts that can be defined or registered elsewhere. Extension relation types (as with
Yes, other condition types can be added to this section given need, consensus, and interest to implement. It should stay consistent with what's intended by #authorization-extensions. (Though I hope that we don't end up extending indefinitely because that'd make WAC more complicated than it was ever intended. Simple > Complex. Easier to add, harder to remove.) |
|
Thank you @csarven, this looks good now. I think the option to add more condition types is valuable, and I agree that proposals must be carefully considered before adding to not end up with an "open registry". |
|
We at dokieli Enterprises commit to implementing the client features as described in this PR. |
|
I plan to implement checking the client and issuer conditions as well as providing the URLs of supported conditions for servers to advertise in my Java implementation https://github.com/uvdsl/solid-wac-java |
|
Plans for updating https://github.com/solid-contrib/specification-tests/tree/main/web-access-control |
|
@Potherca, would you be interested in supporting client and/or issuer restrictions with WAC in the Nextcloud plugin and the PHP Solid Server? This PR might be of interest to you and your team. |
|
@woutermont, given that you implemented such checks for clients in the past, or maybe also @joachimvh, would you be interested in supporting client and/or issuer restrictions with WAC in CSS? |
5 participants
This PR supersedes #133 and closes #81 .
This PR introduces
acl:conditionas an additional requirement on anacl:Authorization, building on the notion of extensibility previously referenced in the Authorization Extensions section of the specification. The feature rests on a capability detection mechanism: servers that support specific condition types, e.g.,acl:IssuerConditionandacl:ClientCondition(specified in this version of the specification), advertise them viaLinkheaders on the effective ACL resource. Clients discover supported condition types from those headers and deploy condition-bearing authorizations accordingly. Condition types not signalled by the server are not used in Authorization Evaluation. When multiple conditions are present, they are conjunctive, i.e., all must be satisfied for an Authorization to be applicable. This mechanism paves the way for additional condition types to be incorporated in the future based on needs and implementations in the ecosystem, e.g., time-based conditions or ODRL policies, as anticipated in Authorization Extensions.The PR includes the following changes (also included in the
#changelogof the specification) with correction classes:acl:conditionas part of an applicable Authorization.Related TODOs (as separate PRs):
Other TODOs:
Preview