[Koha-devel] Koha::* business objects authorization check

[Tomas Cohen Arazi]

Is there some consensus on having the required permissions defined in the
swagger file? If that was the case, no matter how we check them at
Koha::Object level, we need to make the permissions schema available at
that level. So we could need to parse the swagger file and cache it.

I like it for its simplicity and maintainability. but if we don't agree on
making the swagger-specified permissions globally available, we would then
duplicate the logic, and we could make mistakes or have them divert.

If we think of the REST api alone this is not an issue. Because we have the
Swagger plugin.

El mié., 31 ago. 2016 a las 5:32, Jonathan Druart (<
jonathan.druart at bugs.koha-community.org>) escribió:

> 2016-08-30 21:23 GMT+01:00 Chris Cormack <chrisc at catalyst.net.nz>:
> > * Tomas Cohen Arazi (tomascohen at gmail.com) wrote:
> >> Hi everyone, adding to the discussion about Koha::Objects I have a
> subject that
> >> needs broad (an urgent) discussion.
> >>
> >> The REST api patches are shaping up, and I'd like to focus on an
> important one:
> >> 14868 [1].
> >>
> >> This bug introduces a way of specifying on the Swagger files, the
> required
> >> privileges to do stuff on objects. For example:
> >>
> >>
> >> +      "x-koha-authorization": {
> >> +        "allow-owner": true,
> >> +        "allow-guarantor": true,
> >> +        "permissions": {
> >> +          "borrowers": "1"
> >> +        }
> >>        }
> >>
> >> specifies that to get a patron object you need to either have the
> 'borrowers'
> >> permission, be the object owner, or the guarantor of the object's owner.
> >>
> >> This is really nice, because permissions definition are now higher level
> >> entities (just a configuration thing), and they are part of the API
> >> documentation. There's even a bug with patches that introduce required
> >> permissions in the error messages.
> >>
> >> The current implementation introduces checks for each object class on
> the
> >> controller code.
> >>
> >> My feeling is that each business object should be responsible for such
> a check.
> >> The first step I made was to extend Koha::Object so it exposes an
> ->owner
> >> method that returns the Koha::Patron object that owns the object
> (because we
> >> can get the guarantees from there too).
> >
> > I like it, it is only when we do things like this that the Objects
> become useful,
> > otherwise they are a shim over db access (and a slow shim at that).
> > Once they do more things like this, they will be a lot more useful.
> >
> > Chris
>
> I think it's also quite useful when we manage to migrate our legacy
> code to OO quite easily (reducing the number of lines and adding test
> coverage).
> Note that Koha::Object[s] does not reduce speed compare to using
> DBIx::Class directly, I have already proved that previously. Your turn
> to prove the contrary if you think you are right :)
>
> Cheers,
> Jonathan
>
> >
> >> I did this to eliminate the need of class-specific checks on the
> controller.
> >> But at the end of the day, this still didn't feel it was good enough. I
> belive
> >> such a business matter should not be so tied to the controller/routing
> code and
> >> it is still there.
> >>
> >> Talking with Jonathan we thought we could make Koha::Objects->search
> accept
> >> more parameters (current userid, required permissions) and have the code
> >> checking authorization there, basically creating a filter on the
> search. The
> >> main problem is how to propagate the swagger-specified required
> permissions
> >> down to the Koha::Object in a clean way.
> >>
> >> I'd like to hear how people belive we should handle on this new+fresh
> code
> >> permissions check, so we have good foundations for the future of the
> project.
> >> Cleaning untested business logic from controller scripts is a good
> start! And
> >> this is the time to do it!
> >>
> >> Thanks in advance
> >>
> >> PS. I'm sure Jonathan will explain the discussion better once he comes
> back
> >> from the pub :-D
> >>
> >> [1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14868
> >> --
> >> Tomás Cohen Arazi
> >> Theke Solutions (https://theke.io)
> >> ✆ +54 9351 3513384
> >> GPG: B2F3C15F
> >
> >> _______________________________________________
> >> Koha-devel mailing list
> >> Koha-devel at lists.koha-community.org
> >> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> >> website : http://www.koha-community.org/
> >> git : http://git.koha-community.org/
> >> bugs : http://bugs.koha-community.org/
> >
> >
> > --
> > Chris Cormack
> > Catalyst IT Ltd.
> > +64 4 803 2238
> > PO Box 11-053, Manners St, Wellington 6142, New Zealand
> >
> > _______________________________________________
> > Koha-devel mailing list
> > Koha-devel at lists.koha-community.org
> > http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> > website : http://www.koha-community.org/
> > git : http://git.koha-community.org/
> > bugs : http://bugs.koha-community.org/
> _______________________________________________
> Koha-devel mailing list
> Koha-devel at lists.koha-community.org
> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> website : http://www.koha-community.org/
> git : http://git.koha-community.org/
> bugs : http://bugs.koha-community.org/

-- 
Tomás Cohen Arazi
Theke Solutions (https://theke.io <http://theke.io/>)
✆ +54 9351 3513384
GPG: B2F3C15F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20160831/7707130b/attachment-0001.html>