LD4P / sinopia_server

[Deprecated - switching to MongoDB] Sinopia Back-end CRUD Service. LDP-inspired, HTTP Server taking JSON-LD resources & administrative metadata.
Apache License 2.0
1 stars 1 forks source link

figure out ACL update strategy (from JS SDK side) #42

Closed jmartin-sul closed 5 years ago

jmartin-sul commented 5 years ago

we'll be using an auto-generated (by swagger-codegen) method for updating resources (including groups), but we'll probably want our own wrapper method for updating an ACL for a group resource, since:

  1. there's no route in the swagger spec specifically for ACL update (because update of a group's ACL is differentiated from update of the group resource itself only by a URL param, it's not capturable in swagger as a different route).
  2. even if there were a route and a defined structure in the swagger spec for ACL updates, we'd need to do some resource wrangling that is very RDF specific (e.g. take the group's ACL as it is, modify it as desired, and then replace the entire existing ACL with the updated version).
ndushay commented 5 years ago

So would this sort of approach for "api" calls work? As an initial set of "operations"?

jmartin-sul commented 5 years ago

@ndushay, group creation and deletion would be handled by auto-generated methods from the swagger spec (createGroup and deleteGroup, respectively). this ticket is just about manipulating the JSON that defines an ACL on a group that already exists in trellis.

i do think this ticket is about at least the following from your above list:

i'm not sure about:

jmartin-sul commented 5 years ago

slight edit to the description wording to make things clearer

ndushay commented 5 years ago

@jmartin-sul actually, the ACL also needs to know about the groups, so ... would the swagger API call do that as well? Add the group to, remove the group from the ACL? Or would "add a user to" and "remove a user from" be expected to do that?

ndushay commented 5 years ago

@jmartin-sul yes, by "user" I meant a webID in the ACL.

ndushay commented 5 years ago

so "add user to group(s)" or "remove user from group(s) would get a webID and a list of group names, and would then

  1. lock the webACL file for updates on server
  2. get the webACL file from server
  3. update the webACL file "locally" to add/remove webID to/from group(s)
  4. write the new webACL file to server
  5. release lock

???

ndushay commented 5 years ago

is there something more elegant than a lock? This is starting to seem like we should schedule a design meeting - maybe for Fri afternoon or for Monday. Or while I'm gone. @jermnelson

jmartin-sul commented 5 years ago

actually, the ACL also needs to know about the groups, so ... would the swagger API call do that as well? Add the group to, remove the group from the ACL? Or would "add a user to" and "remove a user from" be expected to do that?

i'm not sure i follow here... the swagger API gives us a method that can be used to create the group resource (a basic container, e.g. https://github.com/LD4P/sinopia_server/blob/master/fixtures/group_defs/stanford.jsonld). but it doesn't give us a method to retrieve or update the ACL on a particular group. the former is done by doing a GET on the resource with the addition of the ext=acl param on the URL. the latter would be done by PUTing to the container with the ext=acl param and the JSON for the desired full ACL as the request body. e.g.: https://github.com/LD4P/sinopia_server/wiki/Draft-Notes-for-Sinopia-Server-API-Spec#update-acl-permissions-on-base-container

the ACL for a group does, it seems, refer to the group itself, but i think that's just a detail of how we generate JSON for a particular group ACL?

so "add user to group(s)" or "remove user from group(s) would get a webID and a list of group names, and would then

  1. lock the webACL file for updates on server
  2. get the webACL file from server
  3. update the webACL file "locally" to add/remove webID to/from group(s)
  4. write the new webACL file to server
  5. release lock

so this might be the main point of miscommunication, but i don't think there's a single unified webACL file on the server (i mean there's that users file, but we're not gonna use that in prod, because it has passwords in plaintext and is just for testing). the way that ACLs are retrieved/stored, AFAICT from christina's wiki page, is by GETing/PUTing the ACL for a single group from that group's resource URI (w/ the ext=acl param as a modifier).

so for our basic use case of adding/removing users to/from one group, it seems like the naive approach is just:

  1. retrieve the existing JSON by GET on resource with ext=acl
  2. parse it to a JSON object*
  3. add or remove the desired user from the overall JSON structure
  4. spit out a new JSON string from the modified JSON object
  5. PUT the entire JSON blob back on the server
  6. most recent update wins, but probably not an issue since there's probably only one admin (michelle)? but that is a gotcha (and my main concern here, but i don't think we can do anything more atomic, given how trellis stores this info).

*this is a wrinkle i hadn't thought about... at group creation, there's just the default anyone-can-do-anything permissions for things contained by that group. is this what you were getting at in the "ACL needs to know about the group comment"?

so yeah... update methods like add/remove user could also init the ACL structure the first time they're run? or we could have a wrapper method that did "real" group creation by calling the swagger generated method to create the BasicContainer, then updating to a default locked-down ACL?

i agree, a design meeting would be a good idea, likely easier to discuss by talking and whiteboarding. i'd definitely prefer monday, but could do friday if need be.

ndushay commented 5 years ago

is there something more elegant than a lock? This is starting to seem like we should schedule a design meeting - maybe for Fri afternoon or for Monday. Or while I'm gone. @jermnelson

ndushay commented 5 years ago

the way that ACLs are retrieved/stored, AFAICT from christina's wiki page, is by GETing/PUTing the ACL for a single group from that group's resource URI (w/ the ext=acl param as a modifier).

The WebACL doc does not itself have passwords, does it? But it does have groupIds containing webIds, and AFAICT, it will need them. The groups themselves are maintained in the server, but the permissions of who can write to said groups ... is that not in the WebACL for a group?

I'm more than happy to skip dealing with locks; we may decide to just make a note that coping with concurrent update issues is a future concern. It's also possible we can use jq or some such to get info from and write info to the WebACL JSON for a group.

Clearly, a realtime discussion, with laptops to check stuff, is in order.

ndushay commented 5 years ago

I'm now at this notion of the functions:

ndushay commented 5 years ago

do we also need the default permissions for a new group (container) to be diff from base container?

jermnelson commented 5 years ago

I agree that we should have a design meeting and double check our assumptions (I was under the impression that each group had it's own ACL file) I'll be traveling Friday afternoon but would be available Monday.

ndushay commented 5 years ago

are we going to add (these) ops to the swagger spec?

ndushay commented 5 years ago

and you both are correct - there is a webACL for each group .. but there is still ONE webACL for each group. As I said, happy to punt on locking the file for update for now.

jmartin-sul commented 5 years ago

final word from @michelleif today in storytime is that we won't have a default group. user must be added explicitly to any group under which they should have the right to create resources.

ndushay commented 5 years ago

Thanks for the info -- no default group. So I believe this is what we need (or at least an excellent start):

ndushay commented 5 years ago

Had slack convo with @jermnelson. We are agreed the above commands are a good start. We also agreed:

Note that there are some npm packages for node ACL manipulation

Rando comment: team members need to be added to npm registry when we create npm packages (sinopia_client, sinopia_acl)

Note that there does not seem to be an existing WebACL validation package -- JSON schema, npm module, RDF shapes, whatever

jmartin-sul commented 5 years ago

@ndushay and @jermnelson, that all sounds perfect based on what i know! if i can be of assistance, i'm happy to pair on any of that, take subtickets, whatever (or something else entirely, if @jermnelson would prefer).

jermnelson commented 5 years ago

Hi John, I think the ACL work is critical path so if you want to work on this that would be great! As Naomi is creating tickets surrounding this work. I'll move them into the ready column when they are available. I'll go ahead and close this ticket out.

michelleif commented 5 years ago

Thanks for the info -- no default group. So I believe this is what we need (or at least an excellent start):

  • list the "groups" (can be via Trellis web GUI)
  • list the current webIDs in a single group (not sure if this is in Trellis Web GUI)
  • (stretch?) create a new group
  • add a single webID to a single group

    • (variant) add one or more webID to one group
    • (variant) add one webID to one or more groups
  • remove a single webID from a single group

    • (variant) remove one or more webID from one group
    • (variant) remove one webID from one or more groups
    • (variant) remove one webID from all groups

From admin (me) standpoint, variants are nice but not required. I'm find with just "add single WebID to single group" and "remove single WebID from single group".