silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 26 forks source link

Move PDF generation into a separate module #31

Closed chillu closed 6 years ago

chillu commented 6 years ago

The majority of BasePage and BasePageController deal with generating PDFs. That's becoming a less common use case on the web, given that PDF is generally not considered an accessible format for government websites, and because browsers have that built in now (even on Edge).

Even if you don't use this feature on your website, the downloadpdf endpoint is accessible on every page - which increases the DDoS attack surface of every CWP site. It also increases the amount of code a SilverStripe dev needs to review and understand when coming into a CWP project.

Move this feature into a separate module (cwp/pdfexport?). I'd suggest to make it opt-in, and document an upgrade path. This will be an API change (method subclassing on Page no longer works). CWP 2.0 is an opportunity to do that.

You could argue that CWP no longer needs to provide this feature, and hence it shouldn't be a supported module within CWP in the first place (reducing our maintenance burden, e.g. with https://github.com/silverstripe/cwp/issues/3) - but that's a wider discussion that might not be resolved in the CWP 2.0 release timeline.

/cc @tractorcow @robbieaverill

tractorcow commented 6 years ago

This is a smaller part of the "refactor basepage away forever" that I'd like in cwp. How much of this can we do in 2.0 @robbieaverill ?

robbieaverill commented 6 years ago

Yeah why not :-D

robbieaverill commented 6 years ago

Pull requests:

NightJar commented 6 years ago

By the by, Edge is even now the default PDF viewer in Windows.

chillu commented 6 years ago

🎉