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

cannot download plan #82

Closed
StCyr opened this issue Aug 16, 2023 · 9 comments
Closed

cannot download plan #82

StCyr opened this issue Aug 16, 2023 · 9 comments
Labels
fixed in next version roadmap Issue to be handled upstream

Comments

@StCyr
Copy link
Collaborator

StCyr commented Aug 16, 2023

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

4.1.0

Expected behavior:

See actual behavior first to understand the issue

In my dashboard, 2 possibilities:

  1. either the list of my organization's plans contains only the plans created by other users of my organization, or;
  2. the list contains the same plans as described in the "actual behavior" but you can open all of them without error

Actual behavior:

In my dashboard, the list of my organisation's plans contains:

  1. The plans for which my organization is the primary research organization
  2. The plans created by other users of my organization.

(only plans whose visibility has been set to "organisation" ofc)

For the plans for which my organisation is the primary research organisation, you cannot open such a plan unless the user who created it is ALSO a user of your organisation

Steps to reproduce:

  1. Let's have 2 users X, and Y, both members of organisation A
  2. Let's have user X create a plan and set its primary research organisation to A
  3. Let's confirm that user Y can see the plan
  4. Let's have user X's organisation changed to organisation B
  5. Let's confirm that user Y cannot see the plan anymore

Though the plan's primary research organisation is set to A, user Y cannot see it because user X has been moved to another organisation

@nicolasfranck
Copy link
Collaborator

I do not understand. Are there plans missing?

From what I understand, it should show the following:

  • list 1: plans that I can participate in, either as owner, co-owner or just read-only
  • list 2: plans that have been shared within my organisation, or which are public, and that do not exist in the previous list (to avoid duplicates in the list). Therefore a list without action buttons.

@StCyr
Copy link
Collaborator Author

StCyr commented Sep 26, 2023

ok, re-reading this issue, I understand it's not clear enough.

I've completed the "steps to reproduce" paragraph to make it clear enough I hope

@nicolasfranck
Copy link
Collaborator

I think there is a misconception here:

  • the primary list of plans is a list of plans that you have been explicitly assigned to (see table roles which is an odd name for this). This has nothing to do with any organization that is part of the template of the plan. You can share plans with anyone, not just members of your organization.
  • the secondary list of plans is a list of plans that is either public (not in our case) or part of your organization. The code makes sure that plans in this are not also in the first list.

In short: access to a plan is explicitly mapped in table roles, not because you are part of the same organization.

@StCyr
Copy link
Collaborator Author

StCyr commented Sep 26, 2023

hmmm on test.dmponline.be, set yourself as working for Belnet, then try to download the "test" plan:

image

You get a blank page, and in the logs, I'll get this:

Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: I, [2023-09-26T13:01:52.471769 #16]  INFO -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca] Started GET "/plans/64841/export.pdf?export%5Bquestion_headings%5D=true" for 193.190.130.1 at 2023-09-26 13:01:52 +0000
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: I, [2023-09-26T13:01:52.474673 #16]  INFO -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca] Processing by PlanExportsController#show as PDF
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: I, [2023-09-26T13:01:52.474766 #16]  INFO -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca]   Parameters: {"export"=>{"question_headings"=>"true"}, "plan_id"=>"64841"}
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: E, [2023-09-26T13:01:52.511655 #16] ERROR -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca] You are not authorized to perform this action.
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: I, [2023-09-26T13:01:52.512251 #16]  INFO -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca] Completed 406 Not Acceptable in 37ms (ActiveRecord: 14.5ms | Allocations: 5859)
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: F, [2023-09-26T13:01:52.513138 #16] FATAL -- : [5dd0d329-62d8-4e70-a66d-4fb778576dca]   
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [5dd0d329-62d8-4e70-a66d-4fb778576dca] ActionController::UnknownFormat (ActionController::UnknownFormat):
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [5dd0d329-62d8-4e70-a66d-4fb778576dca]   
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [5dd0d329-62d8-4e70-a66d-4fb778576dca] app/controllers/application_controller.rb:189:in `render_respond_to_format_with_error_message'
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [5dd0d329-62d8-4e70-a66d-4fb778576dca] app/controllers/application_controller.rb:37:in `user_not_authorized'
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: I, [2023-09-26T13:01:52.572407 #16]  INFO -- : [f73e7576-4513-424b-9c2b-f3b5efecd6f9] Started GET "/favicon.ico" for 193.190.130.1 at 2023-09-26 13:01:52 +0000
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: F, [2023-09-26T13:01:52.574447 #16] FATAL -- : [f73e7576-4513-424b-9c2b-f3b5efecd6f9]   
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [f73e7576-4513-424b-9c2b-f3b5efecd6f9] ActionController::RoutingError (No route matches [GET] "/favicon.ico"):
Sep 26 15:01:52 dmponline 9bc6d88a2440[1233]: [f73e7576-4513-424b-9c2b-f3b5efecd6f9]   

This happens because Lisa created this plan while working for Belnet and is now set to be working for UGent.

This is a bug: either the plan shouldn't appear in the list, or it should be downloadable (or at the very least, the user shall receive an error message explaining the reason why the plan cannot be downloaded)

PS: If you now change lisa's organisation back to Belnet, you'll be able to download the plan

@nicolasfranck
Copy link
Collaborator

nicolasfranck commented Sep 28, 2023

There are a few problems here:

  • one is technical. The controller throws an error ("not authorized") that has to be handled by the main controller, but that controller also wants to respect the request format ("pdf") for which it is not configured. But then again, that plan should not be there in the list in the first place if it cannot be downloaded.
  • the reason why it is in the list is because the plan has an org_id equal to the users org_id. But none of the users are affiliated with that plan (read: there are no roles with users with org id). I had here a local example where the plan.org was "ugent", and all of the users were from "artevelde". Normally, at least the creator is of the same organization. From the code I see that this only happens when the "primary research organization" is checked. In that case, not the organization from the logged in user, but that from that primary research organization is attached to the plan. I don't know why something that is only important for template selection is also attached to the plan..
  • a third reason is this line: https://github.com/DMPRoadmap/roadmap/blob/main/app/controllers/plan_exports_controller.rb#L15. That if-statement only validates if the parameter "form" is also included (which it isn't). If I remove that extra requirement, the code works. That proves that the plan is "privately visible"! If you add parameter export[form]=true it works. I see that that parameter is used on the plan download page (so there the plan download works!).

In short: the policy of organizational visible plans and the exports do not work well together. That parameter export[form]=true is there to make a difference between requests coming from the "plan download page" where settings can be chosen, and requests coming from outside (public!) where settings cannot be chosen (it relies on default values).

@nicolasfranck
Copy link
Collaborator

I have put an detailed issue here: DMPRoadmap/roadmap#3345
Added my thoughts also

@nicolasfranck
Copy link
Collaborator

@StCyr

I have pushed a preliminary fix here: 9c2ef9f

It is done in the override file config/initializers/ugent.rb where all the rest of the overrides are. I am interested in the ideas of the dmproadmap team too.

@StCyr
Copy link
Collaborator Author

StCyr commented Sep 29, 2023

I've rebuild test.dmponline.be.

it seems to be working now

@StCyr StCyr added fixed in next version roadmap Issue to be handled upstream labels Oct 4, 2023
@TheLisaVL
Copy link
Collaborator

Fixed in the december '23 update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version roadmap Issue to be handled upstream
Projects
None yet
Development

No branches or pull requests

3 participants