pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
299 stars 444 forks source link

Allow journal managers to invite users to adopt a role #3022

Open ajnyga opened 6 years ago

ajnyga commented 6 years ago

Tagging @bozana here

The issue is discussed here: https://forum.pkp.sfu.ca/t/ojs3-editing-user-roles-in-multi-journal-installations/28711

The problem: if a user is registered to more than one journal, the journal manager can not edit the roles the user has in her journal. In OJS2 this was possible and in OJS3 journal managers can still add the reviewer role when doing a review assignment.

Suggestion:

Problem with legislation: @bozana raised the problem with data privacy and user "contracts". Maybe we could just send a notification when journal managers adds a new role for a user. Basically the same thing we do now with the reviewers in OJS3.

NateWr commented 6 years ago

I've given this a "Community Priority" label because there does seem to be demand in the forum for better handling of this scenario.

I think we should consider an "Invite user to be a X" feature. It would send an invitation email, which would grant a user a role when they click on the link in the email. It should also display the invitation URL, so a Journal Manager can copy/paste that into any existing communication they have with the user.

This would address privacy issues and also give editors a more straightforward way to accomplish what they want. @ajnyga any objections to this approach? If so, I may rephrase the issue title to be more specific.

ajnyga commented 6 years ago

Hi,

Invite makes sense and would work very well IMHO, I think this was also discussed as one option in the forum thread above.

A connected matter is that is it ok GDPR wise that for reviewers we create an user account (name, email) without the consent of the user when the reviewer does not exist in the database?

NateWr commented 6 years ago

After some discussions about #2860, it was decided that this is a necessary feature alongside some changes with the new users list panel, so we'll be transitioning the Add User process over to an "invite" workflow for journal managers.

This will include adopting an invite workflow for the Reviewer selection as well, so it will be good to keep #1146, and the whole Reviewer Selection project (https://github.com/pkp/pkp-lib/projects/7) in mind.

Admins will still be able to add or edit a user at will.

ajnyga commented 6 years ago

Just checking, after the planned changes, can an editor remove an editor role from another editor in the same context if that editor is also enrolled to another context?

This is needed when an editorial board changes.

NateWr commented 6 years ago

That's a good question. I think the current system allows that to happen, and don't foresee that changing. But I don't know whether it should be possible. My gut reaction would be to allow it.

ajnyga commented 6 years ago

Pretty sure the current system does not allow it. It will not let a journal manager to access the user profile form at all if the user is registered in multiple contexts.

What if there would be a separate form that would only show the list of available roles for the selected user:

So as you suggested, just allow journal managers to remove roles. I do not think that GDPR poses a problem to this direction.

Enro commented 5 years ago

Hello, sorry to jump in without taking the time to read the full discussion. I just wanted to mention a potential link between this issue and this GDRP-related feedback https://forum.pkp.sfu.ca/t/privacy-concerns-regarding-user-enroll/19860 although I am not sure whether it still applies under OJS v3...

NateWr commented 5 years ago

Hi @Enro, yes we're aware of this issue, which is why it's been assigned to the GDPR project. It is also a blocker for https://github.com/pkp/pkp-lib/issues/2860#issuecomment-388906925. We do see it as a priority and I personally have got a good few weeks' worth of work sitting on the shelf waiting.

jmvezic commented 4 years ago

Are there any updates on this issue?

NateWr commented 4 years ago

No, there have not been any movement recently.

ajnyga commented 4 years ago

Is https://github.com/pkp/pkp-lib/issues/4959 somehow different? @NateWr @israelcefrin ?

NateWr commented 4 years ago

In this issue, a journal manager (ROLE_ID_MANAGER) should be able to invite a user -- any user to any role -- by entering their email address. That user may already have an account in the system for another journal, and in such cases the journal manager should not know that. Journal Managers should not be able to discover who has an account on a separate journal.

So the invitation workflow should work the same whether or not they already have a user account.

In #4959, we want any editor to be able to invite a reviewer. The solution for this issue should provide a general facility that we can use to solve #4959 (the invitation and acceptance workflow should be the same). But I don't think you need to actually support that for this issue. Just the ability to kick off an invitation from the users list should be sufficient for this.

ajnyga commented 4 years ago

Ok, you are probably correct that better tackle users & roles first, since the reviews is probably more complicated. But I already wrote this longer roundup that includes the review stuff as well.

Comments? @NateWr @asmecher @jmacgreg

Requirements

  1. Users & Roles. We add a new "Invite user to a role" action that can be used to invite any user on the site based on an email address.

  2. Add reviewer in Review stage. We replace the current "Add a new user" and "Enroll an existing user" actions with a single "Invite a new reviewer" action.

Workflows for editor

1 a. The editor needs to add an new user. The user has no existing roles in the context and the editor can not see her in the list. The editor selects Add new user and fills in the form. The system says that the email is already used => The system shows a link "Invite the user to acquire a role in [contextName]" => New modal window opens showing the users email (not editable) and a selection of roles (checkbox) that the invitation will include. At least one role needs to be selected to send the invitation.

1 b. The editor needs to add an new user. The user has some role in the context but the editor does not find her in the list for some reason. The editor selects Add new user and fills in the form. The system says that the email is already used => The system shows a link "Invite the user to acquire a role in [contextName]" => New modal window opens showing the users email (not editable) and a selection of roles (checkbox) that the invitation will include. The roles that the user already has are active but checkboxes are disabled. At least one role needs to be selected to send the invitation.

1 c. The editor needs to add a new role to a known user. The user has some role in the context and the editor finds her in the list. There is a new "Invite to acquire a role" action available. => New modal window opens showing the users name and email (not editable) and a selection of roles (checkbox) that the invitation will include. The roles that the user already has are active but checkboxes are disabled. At least one role needs to be selected to send the invitation.

1 d. The user has role X in the context, the editor needs to remove role X from that user. What should be done with this? We could of course allow users the deselect editorial roles by themselves from the User Profile? This would be the most GDPR compliant way.

  1. The editor tries to send a review request but the selected person is not a reviewer yet or the editor does not find her on the list. We have a new "Invite a reviewer" action that opens an ordinary review request form that has an email field on top. The editor fills in an email address and checks the request details and sends in the request. As long as the reviewer has not reacted, the system will only show the email address in the review assignments grid. (@nateWr, I know you have been thinking of using hashkeys, but here the editor really needs to see some concrete information about who has been invited, has to be the email I think!)

Workflows for user

  1. The user receives an email telling her that she has an existing user account in [siteName] and that she has been offered new roles in [contextName]. The email will then list the offered roles and will include an accept link. By clicking the link, the user will be logged in to the context and the offered roles are attached to her account. The system will give a success message.

2 a. The invited reviewer does not have an existing user account. She receives a message saying that she has been invited to do a review in [contextName] the review details are in the bottom. The email explains that she first has to create an user account by following the link given in the email. Clicking the link will open a simplified version of the registration form. After filling in the form, an user account is created, the user is logged in and then redirected to the review form step 1. At this point, the editor starts to see the full name of the reviewer in the review assignments grid. There is also an alternative link for rejecting the request and in this case no user account is ever created.

2 b. The invited reviewer has an existing user account but no reviewer role. She receives a message saying that she has been invited to do a review in [contextName] the review details are in the bottom. Clicking the link will open review form step 1 which is now accessible for users without the reviewer role. By accepting the review, a reviewer role is attached to the user. At this point, the editor starts to see the full name of the reviewer in the review assignments grid. If the user declines, no role is attached.

2 c. The user was a reviewer already, but the editor did not find her and used the Invite a new reviewer action. Here the system will figure things out and will send an ordinary review request. The editor will see the user's name right from the beginning.

NateWr commented 4 years ago

This looks great, @ajnyga. I've had a read through and I think it covers things nicely. A few thoughts on the editor workflow:

On the workflows for user:

ajnyga commented 4 years ago

For the add user workflows (1a-1c), I think we should replace the Add User form with an Invite User form that is just an email address. If I recall, this was one of the GDPR conditions and I think it's a good practice for us to enforce. So the editor only ever provides the email address and the user fills in the details.

Yes, in general we should not be creating user accounts without consent. Of course in many cases where the add user action is used it is usually creating editorial staff accounts and consent is maybe not a big issue. On the other hand, this new approach is also an excellent way of making sure the email actually works.

Maybe leave the old Add user form accessible for site managers?

For 1d, I think the editor can already do this, right? If an editor edits a user, they can remove roles.

They can not access any user settings if the user is enrolled in several contexts.

NateWr commented 4 years ago

Maybe leave the old Add user form accessible for site managers?

I think if we can make the case for GDPR, we should not include it. The invited user should have a chance to fill it out though.

@mfelczak what do you think? For GDPR we are switching from "add a user" where the journal manager adds all the details to "invite a user" where they enter an email and select the roles to invite them too. The user then has to fill in their own details. Do you foresee any gnashing of teeth with this?

mfelczak commented 4 years ago

From my experience I can see possible edge cases where it'd be helpful to retain the ability to manually add users without needing to go through the more formal invite user process. For example, when importing content or testing imports it's sometimes useful to be able to quickly create non-person import user accounts with test/invalid email addresses.

Given that this will be a significant change with impact for all journals, I can also check-in with a sample of our hosted journals to solicit input on this proposed change. I'm also including @amandastevens and @jmacgreg here as they may have additional feedback.

NateWr commented 4 years ago

Thanks @mfelczak. Would a ROLE_ID_SITE_ADMIN capability to simply confirm an invitation (similiar to login as) suffice to solve this problem?

israelcefrin commented 4 years ago

I was wondering how this change could affect a JM workflow and I have two points to draw attention at first moment regarding when a user is invited.

1) If an JM invites a user who was already invited, OJS should let the JM knows that that email address is already invited rather than only in use. This message should cover the time gap from the invitation till confirmation by the user.

2) When an user rejects the invitation, will the JM be able to invite again without any system warning? Because a user might be no willing to join the journal, and different JMs might be not aware of it and invite him over and over again till the user send an email complaining about it.

On that note, could a user have an option such like:

It could add some extra work, but I was wondering how to improve the experience of being invited going further than just yes or no to the invitation.

ajnyga commented 4 years ago

I think the scenario where a user rejects but is invited again mostly applies to review requests. The other case where you usually need the invite feature is when you add new editorial members and the invite process there is more a formality: the new editorial member probably already knows she will be invited before receiving a message.

Now that we are not creating a full user account before the user accepts the review request, I do not know if it is a good idea to limit the amount of requests/invitations. Two reason:

  1. If a person says no to a request concerning submission A, it does not mean she wants say no to submission B. The decisions the user is making is not about creating an user account. It is a decision whether she wants to review this submission and that can depend on many things.
    (I think already now editors are sending review requests for persons who have rejected before. Probably it is even more common than with this new feature, because with the current code after the first request the system has created an user account and a reviewer role so the user is visible in the reviewers list against her will.)

  2. We also have to take multijournal installations into account. If a person says "no" to Journal A, it does not mean she wants to say "no" to Journal B as well.


Partly related to this question is what happens after the rejection of a review request. The only data we have stored is the email. Can we keep on showing that in the rejected review request?

If we decide that it is personal data which we can not store without consent, then what do we do with it? Do we only present it as "Review request #24356" and delete the email? And if we do that, how do we prevent the editor from requesting the same reviewer again on that same submission? Or do we maybe delete the email after the review stage is ready? Do the editors have an obligation to store the email for archiving purposes? Or is it enough that they store the names of the reviewers who accepted and did the actual review?

If we decide that we can store the email without consent when the request is declined, then the user might create an user account by herself later. Do we in that case attach the stored review request to the user account that she creates? Remember that the solution described above is based on the idea of creating a "proto user account" that uses the users table but only stores the email when the invite is created.

I am maybe more inclined to delete the email and the "proto account" after the review stage in cases where the reviewer has declined. I think it is enough for archiving purposes to know the amount of requests and the names of the reviewers who accepted and did the review.

NateWr commented 4 years ago

I agree with @ajnyga that the invite is usually related to a task. It should be possible to be invited multiple times. I suspect the case of being invited again and again is not a big problem out in the real world, but if we find that it is, we can consider recording the number of invites a user has declined and providing that info to the editor when they are making the invitation.

I think that we should store the email address for review requests. Without any user interaction, the data is not storing any private information on what the user did in the system. It is just recording input from another user -- the editor. In this sense, it is the private information of the editor, not the reviewer.

Storing the email address allows us to record a meaningful editorial history and display a sensible list of review requests for editors. If and when they become a user, we can recover that information as part of their own record of activity (we show stats on review requests accepted/declined/completed).

based on the idea of creating a "proto user account"

What about repurposing the disabled column in the users table to a status column, with options for enabled, disabled, and invited? That might make it easier to have all queries by default only return users with status=enabled, so coders have to deliberately ask for disabled or invited users.

mfelczak commented 4 years ago

Thanks @mfelczak. Would a ROLE_ID_SITE_ADMIN capability to simply confirm an invitation (similiar to login as) suffice to solve this problem?

@NateWr, we had a discussion about this on the hosting team and it may be more helpful to use the JM role for the manual confirmation. In our hosting environment, and possibly others where the Site Admin role is reserved for staff who maintain the install, Editors and Journal Managers often create accounts for their editorial team when first getting started with OJS and they like to configure their OJS install before inviting these users to start using the system, e.g. setup all Section Editors, assign them to Sections.

NateWr commented 4 years ago

Thanks @mfelczak. In this case, does the editor want to prevent the invitation email from being sent at all, so that section editors aren't alerted to the new journal yet?

I'm trying to think through how we do this in a way that doesn't just lead to rampant GDPR-abuse-by-ignorance. Ideally, the JM would invite, then have a special button where they could confirm an invitation, and that action would have a clear warning like "Are you sure you want to do this? Before confirming an invitation you should be sure that you have permission from this person to create an account for them." or something like that.

But I think that workflow would result in the invitation email getting sent out anyway...

mfelczak commented 4 years ago

Hi @NateWr, it's probably fine if the invitation email goes out as it'll contain each user's login info. As long as JMs are able to add and start using the accounts for journal setup, e.g. to assign to Sections, without needing to wait for each SE's email confirmation, this should be ok.

NateWr commented 4 years ago

I think we'll need to wait for confirmation before allowing the user to be used throughout the system. But it sounds like maybe it's ok if the JM does the invite then immediately confirms themselves? The SE would receive two emails: one with an invitation and then one for the confirmation.

ajnyga commented 4 years ago

My opinion is that it is inevitable that the Journal Manager's ability to create new user accounts and apply roles to them has to be removed if we want OJS to be GDPR proof. The only question in my mind is that should the Site Manager still have this ability. This is of course open to argument.

The Journal Managers would just be able to either 1) invite existing users to a role or 2) invite a new user to a role. Only the user herself can answer the invite and either 1) accept the role with an existing account 2) accept the role and create a new account.

It will slow down the process of adding new editorial staff to single journals, but not that much I think.

NateWr commented 4 years ago

GDPR proof

I don't think that the software is the start and end of all the relevant conditions. In the cases that @mfelczak outlined, it is entirely plausible that a JM has permission to create accounts for the editorial team when they're setting up the journal. That permission does not have to be granted through the software itself.

The software should facilitate GDPR compliance and the design of the application should encourage it, but in my view this is a case where a JM may reasonably be granted such a capability.

We should still aim to implement it in a way that makes JMs aware of what they're doing.

It will slow down the process of adding new editorial staff to single journals, but not that much I think.

In the cases that @mfelczak outlined, when setting up a new journal, it would cause considerable slowdown.

marcelareinhardt commented 4 years ago

How was that question? News?

NateWr commented 4 years ago

@marcelareinhardt, no there hasn't been any progress on this yet.

ajnyga commented 4 years ago

Hoping to have this ready for 3.3, starting soon.

jmvezic commented 3 years ago

@ajnyga Just checking up on this, is it still the plan to have it released in 3.3?

ajnyga commented 3 years ago

Sorry, did not have time for doing this for 3.3., but starting to work on this now and aiming for 3.4.

I will start by creating the new InviteToRoleForm visible for journal managers.

We can continue discussing the details for these two big questions:

  1. Can (and how) a journal manager create new user accounts without consent, ie. do we preserve the Add user form?
  2. How would this work in review assigments?

Both already have goof answers, but I feel I can easily start with the invite form and we can even go on and merge that before any other discussions here are completed.

jmvezic commented 3 years ago

@ajnyga

Yes, it seems to me like the "invite to role" can be pushed without the conclusion on the "add user" discussion.

As for the "add user", it is a bit tricky. On one hand, people don't like being added to systems and sites without their consent. It also probably violates some GDPR rules. On the other hand, OJS aims to make it easy for all users to deal with the processes needed to publish a journal. I can definitely see a lot of journal managers being angry if the "add user" feature is removed.

Perhaps a middle ground would work best - when adding a user manually, they are only added in an inactive/disabled state and need to confirm/activate their account (with a link being sent to their e-mail).

withanage commented 3 years ago

In a discussion with the legal department about GDPR and the OJS , I got the following info.

I don't think that the software is the start and end of all the relevant conditions. In the cases that @mfelczak outlined, it is entirely plausible that a JM has permission to create accounts for the editorial team when they're setting up the journal. That permission does not have to be granted through the software itself.

This I can confirm again and as long as the managing editor is legally bound by the publisher to obtain signatures they can create the users and this process can be outside the platform.

But there is one point they noted, if the ojs installation is a multi-journal installation, a journal manager / editor has the search right to list the users` email addresses in the users search box by specifically selecting the option. I was wondering, if it is better overall not to list the email address in the search results for the notifications too as with the new laravel queue the OJS server can send queue the email sending without journal managers seeing their email address until they are enroled into the particular journal ?

NateWr commented 3 years ago

Ideally, a journal manager should not have access to any private information about a user who is not enrolled in their journal. They shouldn't even know that the user is enrolled in another journal. In my view, the feature should work like this:

An editor can select the role(s) they want to invite them to, enter their email address, and send the invite.

When the user accepts the invite, they can enter all of their registration details. If the user accepts the invite, but they already have a user account in another journal, then all of their details are already setup.

So the journal manager does not need to know anything about the user in order to invite them.

jmvezic commented 3 years ago

@NateWr Agreed, that seems like the ideal solution which also respects GDPR.

So the workflow would be as follows (If I understand correctly):

NateWr commented 3 years ago

Yes, but I'd like to see the conditional logic only applied when the user accepts an invitation. So:

I think all of these could be done using the editorial UI, not the reader frontend.

alexxxmendonca commented 3 years ago

That workflow looks good, @NateWr.

ajnyga commented 3 years ago

@NateWr do you have preference where the user actually accepts / declines the roles in the UI? user/profile#roles tab is one option.

edit: also thinking that the user/profile#roles tab should list all the roles the user has in the context. And they should be able to remove any editorial roles they want. Not of course have the ability to require editorial roles.

NateWr commented 3 years ago

I want a new, simplified UI for accepting an invitation. After they've accepted, it can take redirect them to their user profile page where they can edit anything they want further.

accept-invitation

This should appear as a backend page but with no header or navigation menu. (Consider adding a new layout similar to layout.tpl for standalone pages like this -- we'll want one for unsubscribe and other similar actions where the user may not be logged in.)

Save the value of the Name field in this form into the preferred name property. Split the name field by space and put the last word as the family name, the rest as given (I know that this will not work for all names, but they can change this in their profile if it's not right).

If this user already has an account, only the Roles field is required.

If you want to get really fancy, automatically generate a value for the username field based on the name field.

ajnyga commented 3 years ago

You mean backend.tpl?

NateWr commented 3 years ago

Yep!

withanage commented 3 years ago

Thanks all. This is a very good plan in respect with the GDPR guidelines.

Until this feature is properly implemented, I am going to remove the https://github.com/pkp/pkp-lib/blob/master/templates/controllers/grid/settings/user/userGridFilter.tpl#L40:L43

locally , so that Journal managers/ Editors can't search other users from mult-journal installations.

If you think, it is also useful the community and the include users feature is not used heavily , I will create a PR for that.

NateWr commented 3 years ago

I think we need to leave that in for now because it is the only way for admins to locate users in the system. However, a plugin that hooked into the search request and returned an error message if includeNoRole is passed would probably be useful. (Removing the HTML won't prevent the request on its own.)

withanage commented 3 years ago

Sure, I will try it as a plugin and look, if I can hook in and disable the search resutls only for JMs and JEs , so that we do not break the functionality for the admins.

jmvezic commented 3 years ago

@ajnyga is the commit you made compatible with 3.3.0-6? I'd love to try it out if you need some testing.

ajnyga commented 3 years ago

That's only part of the work, I maybe have something for testing in June

jmvezic commented 3 years ago

Great to hear, I'll try to replicate my production database (which has a lot of journals and users) to development server, which should be a great testing ground for the feature.

kshuttle commented 2 years ago

+1 for this issue from a journal manager.

librariam commented 2 years ago

+1 from a multi-journal PKP|PS client