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

--- Comment #356 from David Cook <[email protected]> ---
(In reply to Lari Taskula from comment #325)
> (In reply to David Cook from comment #315)
> > Created attachment 194607 [details] [review] [review]
> > Bug 38365: Fix t/db_dependent/Koha/Plugins/Valuebuilder_hooks.t
> > 
> > This initialises the CSP nonce in the unit test so that
> > t/lib/plugins/Koha/Plugin/TestValuebuilder/test_valuebuilder_popup.tt
> > will get its script element rewritten correctly.
> 
> Do we still need this patch after excluding test_valuebuilder_popup.tt in
> Koha::Devel::Files and xt/find-missing-nonce.t?

Yes. It's used by t/lib/plugins/Koha/Plugin/TestValuebuilder.pm

> Is there a way to make Valuebuilder_hooks.t test fail in the first place? I
> tried rebasing before and after this commit but it always passed, so I don't
> understand the purpose of this change and the patch.

Yes, if you comment out these lines:
use Koha::ContentSecurityPolicy;
my $csp = Koha::ContentSecurityPolicy->new;
$csp->set_nonce();

Then it will fail. There must've been an issue with your rebase I'm guessing. 

> `add_csp_nonces.pl --apply` still modifies test_valuebuilder_popup.tt.
> Should we call its Koha::Devel::Files constructor with context => 'nonce' so
> that test_valuebuilder_popup.tt will not be modified by the script?

Good catch. I'll fix that.

> (In reply to David Cook from comment #316)
> > Created attachment 194608 [details] [review] [review]
> > Bug 38365: Make tidy.pl compatible with script elements with attributes
> > 
> > tidy.pl will still tidy whitespace around script elements that have
> > attributes like "nonce"
> 
> OK. `perl misc/devel/tidy.pl --tt` changes
> t/mock_templates/opac-tmpl/bootstrap/en/modules/opac-csp.tt (file introduced
> in this bug).
> 
> (In reply to David Cook from comment #317)
> > Created attachment 194609 [details] [review] [review]
> > Bug 38365: Set __webpack_nonce__ for style-loader plugin and fix fontawesome
> > imports
> This is out of my expertise, how to test?

My bad. I thought that I wrote a test plan for it. You need to rebuild the
resources,
turn on ERM, and visit the ERM home.

Steps:

1. perl build-resources.PL (this is part of the "important" test plan)
2. Enable syspref ERMModule
3. Go to http://localhost:8081/cgi-bin/koha/erm/home
4. Note that no CSP violations appear in the console

> Not to take anything away from the good work and important findings on the
> staff client side as seen in that patch, but we should redefine the scope of
> this Bug. It is clear from the title of this Bug [1] that the scope has been
> expanding. It has also been established that CSP is not ready to be enabled
> in the staff client and my understanding is that a lot of work remains to be
> done there. But as far as we know OPAC is quite close. So my question is how
> much do we want to (and can) work on the staff client side of things in this
> Bug?
> 
> We have a library that is willing to enable this on the OPAC side. In fact
> they are already running an older implementation but we would like to see it
> replaced with this one. Having this tested in the real world would be good
> even if it was initially just OPAC.
> 
> I also fear that we might run out of steam if this Bug's scope expands too
> much into the staff client as well.
> 
> [1] (at the time of this comment the title is "Add Content-Security-Policy
> HTTP header to HTML responses")

Yeah, I think this is as far as we go for bug 38365. We sign this off, QA it,
and push it as is. 

--

Once bug 38365 is pushed, we can require that people add nonces to <script> and
<style> inline elements (the QA tools will alert to this). 

And then I'll continue the work on bug 38407. Since the value builders are
handled by Koha::FrameworkPlugin, that leaves only about 117 lines across 33
files, which isn't too bad at all. (But I agree is out of scope for bug 38365,
which is big enough already.)

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to