Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OJS] The current role does not have access to this operation -message needs more details on how to acquire the needed role #2297

Closed
ajnyga opened this issue Feb 17, 2017 · 42 comments
Assignees

Comments

@ajnyga
Copy link
Collaborator

ajnyga commented Feb 17, 2017

A typical problem in multi-journal installations is that a user has registered to journal A and later want's to submit something to journal B as well. However, usually this is not possible, because the user does not have the author role in journal B.

I made a feature request about how OJS should handle this situation. http://forum.pkp.sfu.ca/t/ojs3-missing-author-roles-in-multi-journal-installations/28710

In my opinion having a simple button to enable the author role in that message would be ideal. However, I developed a quick fix where the message text has an additional link leading to the user profile/roles page. This will probably help in most cases. PR in a minute.

@ajnyga ajnyga changed the title [OJS] The current role does not have access to this operation -message need more details on how to acquire the needed role [OJS] The current role does not have access to this operation -message needs more details on how to acquire the needed role Feb 17, 2017
@ajnyga
Copy link
Collaborator Author

ajnyga commented Feb 17, 2017

This is a translation related solution, so tagging @mtub here.

@asmecher
Copy link
Member

I'll continue the forum conversation about the "ideal" solution in a sec, but one expedient option might be to have this specific case use a different (more useful) locale key. That way you wouldn't need to modify the general-purpose one in a way that would be misleading for roles that can't be self-registered.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Feb 22, 2017

Thanks, I will check setAdvice. I am using the above pr in our local installation and it seems to have stopped the quite frequent questions concerning the issue above. However, the hard coded url would have to change in any case like you noticed.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Sep 13, 2017

sorry @asmecher but the authorization system in OJS is just too complicated for me to understand. Also I could not quite understand how the setAdvice is supposed to work :S

I mean, I do not understand how this works: https://github.com/pkp/pkp-lib/blob/master/classes/security/authorization/AuthorizationDecisionManager.inc.php#L128-L135
How can $callOnDeny be anything but null here?

If I add this

		$callOnDeny = array($request, 'redirectURL', array("http://test.fi"));
		$this->setAdvice(AUTHORIZATION_ADVICE_CALL_ON_DENY, $callOnDeny);

to here https://github.com/pkp/pkp-lib/blob/master/classes/security/authorization/RoleBasedHandlerOperationPolicy.inc.php#L39, nothing basically happens.

What should happen, I think, is that the AUTHORIZATION_ADVICE_CALL_ON_DENY should override the default AUTHORIZATION_ADVICE_DENY_MESSAGE and the $callOnDeny should be passed as well, but I am still getting the same error message as before.

I am clearly missing something here.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 28, 2018

@asmecher @NateWr
In multijournals installations this is something that is creating a lot of problems. Users registered in another journal can log in to a journal but they do not have permissions to for example send submissions. We get a lot of these and users seem to be lost as there is no clear instructions how to add the required role. Just an error message saying that "you can not do that".

I think that this could be fixed by making it easier for users to acquire the author role when it is missing. Can you check my last comment from September at some point so that I can move forward in implementing the new feature mentioned above.

@NateWr
Copy link
Contributor

NateWr commented Mar 28, 2018

Personally, I don't think users should need to "acquire" the Author role at all. Unless a journal has shut down non-commissioned submissions (ie - by closing all registrations), any user should be able to initiate a submission.

If they already have an Author role in any context, and they initiate a submission in another context, we should just take them to the submission page and make them select an Author-ish role as the first item in the submission form.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 28, 2018

Would this cause problems with things like GDPR? When I suggested that journal manager should be able to add roles for users from other journals, @bozana raised the question of GDPR and the fact that the user has to register the role herself for the related legistlation to be fulfilled.

Or do you think that doing a submission can be regarded as a contract as such? I mean automatically add the role when the user starts to send a manuscript. Usability wise it would be a perfect solution imho.

@NateWr
Copy link
Contributor

NateWr commented Mar 28, 2018

I am not a lawyer, but if the journals are all part of the same site (eg - journal.fi/...) I think they would all fall under the same terms. In the same way that an admin can assign a user a new role, so could we do this automatically, I would think.

Even if the journals are separate, though, it's hard to see what the difference would be. When a user makes a submission, to comply with GDPR the site would need to specify how their data is stored/used. If a role ID is one part of their submission data, it doesn't seem unique in that sense.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 28, 2018

Just tagging the other issue here: #3022
And the discussion: https://forum.pkp.sfu.ca/t/ojs3-editing-user-roles-in-multi-journal-installations/28711

As you can see there, site admin can assign roles freely, but the journal managers can not. But this could just be an issue when someone else is doing something in your behalf.

Actually Alec comments your suggestion here: https://forum.pkp.sfu.ca/t/ojs3-missing-author-roles-in-multi-journal-installations/28710/2
"We had a solution for this way back in OCS – the user would simply gain the required role if possible when the submission link was followed – but that approach won’t work with OJS 3.x/OMP because there may now be several submitting roles and we’ll need the author to choose one."

This lead me to think of a solution where clicking the "Make a new submission" which now just gives you a text saying " The current role does not have access to this operation. " would instead give you an easy way of adding an author role. And that is what was pursued above.

@NateWr
Copy link
Contributor

NateWr commented Mar 28, 2018

This lead me to think of a solution where clicking the "Make a new submission" which now just gives you a text saying " The current role does not have access to this operation. " would instead give you an easy way of adding an author role. And that is what was pursued above.

I'd rather just get them started on their submission. They can select an Author role as part of the first form they submit in the process. Something like "Submitting as" with a select of Author roles (or, if only one ROLE_ID_AUTHOR role exists, we can include a hidden field).

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 28, 2018

That would probably mean changing some of the access policies in the form, but I do think that is a good idea to add those to the form.

If @asmecher agrees I can try doing this during the weekend (while eating mämmi).

@bozana
Copy link
Collaborator

bozana commented Mar 28, 2018

Hi all, just a comment regarding data privacy: I think the problem is when someone else (e.g. journal manager or editor) does something for another person/user (and the user does not know about that i.e. does not agree i.e. does not have a chance to agree/disagree). I.e. it would need explicitly agreement of the user. If a user him/herself would like to do something, that is OK.
Thus, for example, the problem would/could for example be:
-- when a journal manager or an editor creates a new user (e.g. reviewer) or enrolls a user for a journal without his/her agreement
-- when a journal manager or editor from one journal can see the (user) data that is actually registered for another journal.
It could be that all journals in that installation are separate and do not have anything to do with each other. It could be also the other way around, that all journals belong to one instance. In both use cases there could be both: only one main domain for all journals or each journal could have its own domain. E.g. a university library could host all university journals under one domain, however those journals could be (also only partly) separate entities and would not have to do anything with each other.
We consider and would like to support both/all use cases, that's why we separate the data per default per journal, but still allow the other use case. We should probably improve all that. However, in both cases, the users should exactly know what is happening (with their data) and how, before they register and start to use the system i.e. agree with that way/kind of their data use.
It is normal that one special user (admin) can see everything, I believe. It is similar to a system admin, that can see all the data in the DB. This is a special kind of role and it should be at minimum i.e. not many users should have that kind of access.
-- Just my/me thinking...

@asmecher
Copy link
Member

I am not a lawyer, but if the journals are all part of the same site (eg - journal.fi/...) I think they would all fall under the same terms.

Yes, but probably not for service providers using OJS as SaaS, where journals might appear at different domains with no indication of shared platform.

I'd rather just get them started on their submission. They can select an Author role as part of the first form they submit in the process. Something like "Submitting as" with a select of Author roles (or, if only one ROLE_ID_AUTHOR role exists, we can include a hidden field).

Yes, but for journals using the default role configuration (99% I'd guess) there will really only be a single author role allowing self-registration, so it's a moot point. I think we can choose the author role when there's only one, and prompt for the few remaining cases, and if the latter group finds it unworkable we can ask them for suggestions for improvement.

@asmecher
Copy link
Member

...while eating mämmi...

I had to look that up -- happy Easter!

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 28, 2018

So to sum up:
User without the author role selects "Make a new submission"
=> we check if the context has only one author role, if it does we automatically add the role
=> if there are more than one role, then add the role selection to the first stage of the submission role: "You need to select one of the author roles available for this context".

Because the author is doing the selections, we are not running into trouble with GDPR.

@asmecher
Copy link
Member

Yes, sounds great. This might mean excluding the first step of the submission wizard from requiring the author role, but OTOH we've done something like that elsewhere so there might be a pattern to follow.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 29, 2018

But @asmecher do check the AuthorizationDecisionManager code I mention above.

Because if you look at line https://github.com/pkp/pkp-lib/blob/master/classes/security/authorization/AuthorizationDecisionManager.inc.php#L129
And line https://github.com/pkp/pkp-lib/blob/master/classes/security/authorization/AuthorizationDecisionManager.inc.php#L124

I can not understand how the if on line 129 could ever be true?

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 29, 2018

Hi @asmecher and @NateWr

I know this is not what we discussed.

But while doing this change, I found this function that is already used in several places $userGroupDao->getDefaultByRoleId. Like you probably guessed, it fetches a single default author role from a given context.

This seemed so perfect, that I created a pr based on that: #3542

If you still feel like we should give the authors a choice if there are several author roles, then I can do the additional changes needed. It is actually not much.

Also I funny thing I noticed after I changed the policy so that users with no roles could access the submission form: there was already a secondary check there. When the author role was missing, the form showed this error: https://github.com/pkp/pkp-lib/blob/master/locale/en_US/submission.xml#L16

If you think that additional changes are not needed, then I do not think that locale key is needed anymore. But if you think we need the role selection, then I would use that same key just by removing the last sentence. What do you think.

Also Alec, check my last comment as well. I am fairly sure that is a bug although this pr is not affected anymore.

@NateWr
Copy link
Contributor

NateWr commented Mar 29, 2018

That sounds good to me. I won't be able to code review this until at least next week but happy to take a look then.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Mar 29, 2018

No hurry from my part!

@asmecher
Copy link
Member

Thanks, @ajnyga and @NateWr -- and apologies as always for being slow to respond. Nate, let me know if you're not sure about something.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Apr 11, 2018

Thanks! @asmecher, did you check the bug here: #2297 (comment) (not related to the pr anymore, but will most likely affect something else later)

@asmecher
Copy link
Member

I can not understand how the if on line 129 could ever be true?

Check out https://github.com/pkp/pkp-lib/blob/master/classes/security/authorization/AuthorizationDecisionManager.inc.php#L228 -- the $callOnDeny is passed into this by reference, thus gets set in the calling scope too.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Apr 11, 2018

Thanks!
I wonder why the example here did not work for me: #2297 (comment) ? (not really important if you know that the code is correct at the moment)

@asmecher
Copy link
Member

@ajnyga, I just tried that and it did work for me. I suspect your policy tree is resulting in a DENY from another policy before your advice gets processed.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Apr 12, 2018

I give up! Hopefully I do not have to work with the policies any time soon 🤣

@ajnyga
Copy link
Collaborator Author

ajnyga commented Apr 12, 2018

@NateWr nevermind the conversation above, the pr above is ready for review whenever you have the time.

@ajnyga
Copy link
Collaborator Author

ajnyga commented Apr 24, 2018

@asmecher this would be ready for review as well. #3542

@NateWr
Copy link
Contributor

NateWr commented May 9, 2018

FYI, I've taken over this PR and I'll roll it into changes related to #3575.

NateWr added a commit to NateWr/pkp-lib that referenced this issue May 9, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 9, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 9, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 10, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 10, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 17, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 17, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 23, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue May 23, 2018
@NateWr
Copy link
Contributor

NateWr commented May 24, 2018

This issue was address in a PR merged with #3575.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants