Group ACL support - #24
Conversation
- Implement agentGroup parsing and serializing - Implement group.initFromGraph(), add tests - Add checkAccess() support with loading group listings
Fixes issue #12.
|
I can't get group ACLs working yet: https://pastebin.com/92cB4BB8 |
|
Here's my group listing: @prefix acl: <http://www.w3.org/ns/auth/acl#>.
@prefix vcard: <http://www.w3.org/2006/vcard/ns#> .
<> a acl:GroupListing.
<#meandi>
a vcard:Group;
vcard:hasUID <urn:uuid:8831CBAD-1111-2222-8563-F0F4787EG398:ABGroup> ;
# Simply local and remote identity for myself
vcard:hasMember </profile/card#me>;
vcard:hasMember <https://solid.kjernsmo.net/profile/card#me>.I don't know what the UID is, so I just put something in there. Isn't this about the minimal group listing there could be? |
timbl
left a comment
timbl
left a comment
There was a problem hiding this comment.
Nothing jumps out as broken from cursory looking at the diffs
RubenVerborgh
left a comment
RubenVerborgh
left a comment
There was a problem hiding this comment.
It works. But the code is not in good shape (just so we know).
| }, | ||
| "description": "Web Access Control based permissions library", | ||
| "main": "./lib/index", | ||
| "main": "./src/index", |
There was a problem hiding this comment.
This seems wrong. Unless we're not transpiling anymore.
| "files": [ | ||
| "lib" | ||
| "lib", | ||
| "src" |
There was a problem hiding this comment.
…but if we're not transpiling, what is lib doing here?
| */ | ||
| constructor (options = {}) { | ||
| this.uri = options.uri | ||
| this.uid = options.uid |
There was a problem hiding this comment.
Let's get rid of this. It doesn't seem to serve any purpose. Or does it, @dmitrizagidulin?
There was a problem hiding this comment.
Oh man, I do not remember...
| if (!rdfType) { | ||
| console.warn(`Possibly invalid group '${uri}', missing type vcard:Group`) | ||
| } | ||
| this.uid = graph.anyValue(group, ns.vcard('hasUID')) |
| this.rdf = options.rdf | ||
| this.graph = options.graph | ||
| if (this.rdf && this.graph) { | ||
| this.initFromGraph(this.graph, this.rdf) |
There was a problem hiding this comment.
This can't work. initFromGraph expects uri as first param.
There was a problem hiding this comment.
…but we're not using this, I guess.
| this.uid = options.uid | ||
| this.members = options.members || new Set() | ||
| this.listing = options.listing | ||
| this.rdf = options.rdf |
There was a problem hiding this comment.
should probably be options.rdf || require('rdflib')
| this.members = options.members || new Set() | ||
| this.listing = options.listing | ||
| this.rdf = options.rdf | ||
| this.graph = options.graph |
There was a problem hiding this comment.
Should probably be options.graph || this.rdf.graph()
| let groupMatches = graph.match(fragment, vocab.acl('agentGroup')) | ||
| let groupMatches = graph.match(fragment, ns.acl('agentGroup')) | ||
| groupMatches = groupMatches.map(ea => { | ||
| return new GroupListing({ listing: ea }) |
There was a problem hiding this comment.
This does not result in a parsed group (missing params).
4 participants
Corresponds with solid-server PR nodeSolidServer/node-solid-server#719