samvera-deprecated / sufia

[DEPRECATED] Sufia: a fully featured, flexible Samvera repository front-end.
http://sufia.io/
Other
111 stars 78 forks source link

Visiting the edit page of an AdminSet creates an unused permission template. #2902

Closed jcoyne closed 7 years ago

atz commented 7 years ago

From tracing, I think it comes from this line: https://github.com/projecthydra/sufia/commit/365da7b66e5bf2cfc3f930899c8a5e2c1054a431#diff-bcef1fc87215243466d5c0f0e63ad634R99

PermissionTemplate.find_or_create_by(admin_set_id: @admin_set.id)

Also related to exception error.

atz commented 7 years ago

Here's the test line that should catch the blocking exception: https://github.com/projecthydra/sufia/blob/master/spec/features/admin_admin_set_spec.rb#L25

Except that it models the admin_set thusly:

  before do
    create(:admin_set, title: ["A completely unique name"],
                       description: ["A substantial description"],
                       edit_users: [user.user_key])
    allow(RoleMapper).to receive(:byname).and_return(user.user_key => ['admin'])
  end

And the trace of an actual POST to create the admin set:

Started POST "/admin/admin_sets" for ::1 at 2016-11-21 16:23:25 -0800
  Account Load (0.6ms)  SELECT  "public"."accounts".* FROM "public"."accounts" WHERE "public"."accounts"."cname" = $1 LIMIT $2  [["cname", "localhost"], ["LIMIT", 1]]
Processing by Sufia::Admin::AdminSetsController#create as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"go3IPmqxPsutRuGgvDUhPFAodCnnTWN5CLUuP0kZn0KkFX/k+weG78hHP/f+dy0FjDc1o672bJ/LekvEjIBjFA==", "admin_set"=>{"title"=>"Mega Admin Set", "description"=>"First Admin Set is Best Admin Set"}, "commit"=>"Save"}

Note: NO edit_users. Then when we try to view the edit page for real, we get:

Attempted to init base path `dev`, but it already exists
[CANCAN] Checking edit permissions for user: x@y.com with groups: ["public", "registered"]
[CANCAN] edit_groups: ["admin"]
[CANCAN] edit_users: []
[CANCAN] decision: false
Completed 500 Internal Server Error in 92ms (ActiveRecord: 1.5ms)
atz commented 7 years ago

For the record, the exception (discussed on slack) is upon viewing, e.g. http://localhost:3000/admin/admin_sets/4m90dv48q/edit :

ActionController::UrlGenerationError in Sufia::Admin::AdminSetsController#edit
No route matches {:action=>"show", :controller=>"sufia/admin/admin_sets", :id=>"4m90dv48q"}

This blocks further investigation of the extra permission template, though the cause may be the same.

Apparently reverting to show is the default behavior after the CANCAN rejection above.

atz commented 7 years ago

@jcoyne: Should edit_groups: ["admin"] be enough to edit the admin_set or not? If not, shouldn't the creation of the admin_set remember who created it and save it with that edit_user? Why is anyone allowed to create an object they cannot edit?

mjgiarlo commented 7 years ago

@jcoyne @atz Editing an AdminSet (or its PermissionTemplate) is entirely dependent on stored ACLs, so your username would need to be in its edit_users or one of your groups in its edit_groups. Creating an AdminSet is only possible in the UI via the admin dashboard, which ultimately checks to make sure you are in the 'admin' edit_groups. The related abilities are here:

https://github.com/projecthydra/sufia/blob/master/app/models/concerns/sufia/ability.rb#L65-L75

and here:

https://github.com/projecthydra/curation_concerns/blob/master/app/models/concerns/curation_concerns/ability.rb#L42-L44

The easiest way to have my account added to that group, I find, is to edit config/role_map.yml and make it look like this:

development:
  archivist:
    - archivist1@example.com
  admin:
    - mjgiarlo@stanford.edu
atz commented 7 years ago

Resolved in call this morning: admins should be able to edit any admin_set. This bypasses need for any role_map.yml tweaking. In the long term for Hyku, I think that YML file has to go away (at least be database-driven) to provide any level of reasonable flexibility.

jcoyne commented 7 years ago

@atz role_map.yml is what defines who is an admin.