[Koha-bugs] [Bug 29523] Add a way to prevent embedding objects that should not be allowed

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29523

--- Comment #154 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
(In reply to Marcel de Rooy from comment #152)
> Just a few dumb questions:
> 
> Name unredact_list Where does this name originate from ?

I'm open to a name change.. initially I thought 'unredact' felt silly too when
Jonathan first introduced it.. but I came to the conclusion it made sense.. we
now encourage 'allow lists' rather than 'deny lists' to default to secure.  So
it's literally a list of those fields we don't want to redact rather than
having to list the one's we do.

> 
> 
> +        $patron_11_1 = Koha::Patrons->find( $patron_11_1->borrowernumber );
> Added a few times. This seems not needed?  Why do you add it?

Basically we need a fresh Koha::Patron object so that we flush the object level
micro cache that was introduced in the patch (the micro cache was introduced to
improve performance when this is called in a large loop, for example an embed
call with many related objects being returned). I don't think we can do a
simple discard_changes as that only flushes the dbic level result cache and not
the localised object caching we employ here.

> 
> t/db_dependent/Koha/REST/Plugin/Objects.t 
> -      $builder->build_object( { class => 'Koha::Patrons', value => { flags
> => 1 } } );
> +      $builder->build_object( { class => 'Koha::Patrons', value => { flags
> => 0 } } );
> => If I set the user to no permissions, the test still PASSes. Can you point
> me to an example in the tests where we can actually see the difference, i.e.
> where flags makes a difference?

Good question.. I'll have to have a dig on that one.. it's long enough ago in
my memory that I'm strugglig to remember the test logic in this case.

> How do we make sure that we did not forget some calls of patron->to_api with
> user parameter ?

It's a good question.. I ran the full api test suit and ripgrepped for to_api
manually too.  Not sure there's an easy way to automate further.  What we
really need is for anywhere patron can be embedded in the API schema, for there
to be a corresponding api unit test.. I think when I last checked that was a
pretty solid assumption.. but that may have changed since.

-- 
You are receiving this mail because:
You are watching all bug changes.