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