gbv / jskos-server

Web service to access JSKOS data
https://coli-conc.gbv.de/api/
MIT License
6 stars 4 forks source link

Support adding creator to other entity types as well #122

Closed nichtich closed 3 years ago

nichtich commented 3 years ago

Annotations are saved with their creator automatically set via user. For bartoc.org we also want creators to be set on import via PUT/POST. This could be made generic by setting a config value withUser to the entity type.

stefandesu commented 3 years ago

We need to specify exactly what should happen in which case.

@nichtich Can you explain what you mean with the withUser thing, and maybe expand on my start here? Thanks!

nichtich commented 3 years ago

Let's ignore bulk import via admin and anonymous writing and focus on cases with auth: true.

Open question:

stefandesu commented 3 years ago

Okay, this definitely still needs a lot of discussion.

Let's ignore bulk import via admin and anonymous writing and focus on cases with auth: true.

  • The payload fields creator and contributor will alway be ignored
  • Adding a new entity should add the current user as creator
  • Updating an entity should add the current user as contributor unless they are in creator or contributor

👍

  • which identity URI of the user should be used?

Currently, this is not consistent for entity types.

We need to find a solution that can be applied consistently (and doesn't break existing applications).

  • only the user URI or additional information such as name? (I think URI only)

If we go for URI only, we couldn't show any names in Cocoda for mappings and annotations anymore.

I agree that creator and contributor in the payload should be ignored. (Although you could argue that allowing arbitrary values would be good, either if you want to make it clear that this is not a mapping you have created yourself, but taken from somewhere else, or if you want to give someone else access rights.)

However, I think the user should be able to choose which identity and data is added to a mapping. My suggestion would be:

What do you think?

nichtich commented 3 years ago

If we go for URI only, we couldn't show any names in Cocoda for mappings and annotations anymore.

Ok, then add a name

if you want to make it clear that this is not a mapping you have created yourself, but taken from somewhere else, or if you want to give someone else access rights.)

Adding someone else as contributor is a relevant use case (especially for concordances), so one should also be allowed to write this field but this is annother issue to be solved later when required.

  • Use the Login Server URI by default

:+1:

  • Have a parameter like you suggested, for example withIdentity which is the key for an identity provider (orcid, wikidata, github, etc.) to override the default.

As query parameter better use the full identity URI, e.g. identity=https://github.com/nichtich

  • Include the name by default (either the identity name or the Login Server name), but add another parameter

Use identityName as optional parameter.

This should also allow clients to change your existing identity URI and/or name by saving the same item without modification but different identity/identityName

stefandesu commented 3 years ago

Got it. 👌 Last question: If the full identity URI is not actually available in the user's identities, should we:

stefandesu commented 3 years ago

I would suggest:

I know you didn't want to consider anonymous writing and auth: false, but I think we need to because we might otherwise break existing functionality. Since anonymous writing implies crossUser: true, this case is set above. But what about auth: false together with crossUser: false? Is that even a valid configuration? If we don't require authentication, we can't compare the user with the creator anyway.

stefandesu commented 3 years ago

I updated json-anystream to give us the possibility to adjust objects before they are emitted to the stream, so we can do this at a central point (middleware) for all entities equally.

The following is a suggestion for implementing this in jskos-server.

Note that the user getting to this point in code means that they have the rights to perform the current action (this is separate from checking the creator for PUT/PATCH/DELETE though).

For all: Existing fields creator and contributor on the object to be adjusted will be removed.

POST

PUT/PATCH

Edit: Updated with how this is actually handled.

nichtich commented 3 years ago

All right, except for creator and contributor fields should be ignored in the payload.

question: if they are in contributor, should we move them to the top of the list to implicate that they last edited the element?

No, the order is not relevant.

DELETE: Only mentioning this because I have a question: Should contributors be able to delete an entity? I would say no.

No, but not part of this issue anyway.

stefandesu commented 3 years ago

I started an implementation and it seems to work well, but there is still more do be done. Implementations for all services (except annotations which I have done already) need to be adjusted, and we should add some tests to ensure correct implementation.

stefandesu commented 3 years ago

Still missing:

stefandesu commented 3 years ago

Merged into dev!