Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Page 2.0 #1416

Merged
merged 17 commits into from Sep 14, 2017

Conversation

Projects
None yet
6 participants
@WaLLy3K
Copy link
Collaborator

WaLLy3K commented May 2, 2017

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


The two primary goals of re-writing the block page is to keep the styling consistent with the Admin Console, and to minimise the use of Javascript as we should not expect users to have JS enabled on known-bad domains.

The changes within index.php are dependant on an alteration of the output of queryFunc() within /usr/local/bin/pihole, as well as /var/www/html/admin/scripts/pi-hole/php/queryads.php

WaLLy3K added some commits May 2, 2017

Update blockingpage.css
* Block page UI overhaul to replicate the style of the Admin Console
* Block page UI is now mobile friendly
* Users can safely customise text in order to make the block page more friendly for their household
Update index.php
* An "About Pi-hole" link on the block page provides an ELI5 explanation to those not familiar with Pi-hole
* An email contact link on the block page provides users of your Pi-hole with a means to easily get in touch with you
* Browsing to your Pi-hole's address will show a simple "landing page", which can be replaced by adding "landing.php" within "/var/www/html"
* Users manually browsing to file/image based content (i.e: non HTML based content) on blocked sites will be greeted with a small "Blocked by Pi-hole" image
* Sites that are manually blacklisted will display a notice of this on the block page
* Sites that aren't directly blocked, but have a CNAME record, will show a notification on the block page (e.g: If raw.githubusercontent.com is not blocked, but github.map.fastly.net is)
* On the block page, "Back to Safety" now directs the user to "about:home" if Javascript is disabled
* Whitelisting is disabled for installs without a password, or if a client does not have Javascript

* Known issues:
  * Admin Console needs a text field under "Web User Interface" where the admin can enter a preferred contact email when a site needs to be whitelisted, to be saved to setupVars.conf with the key "ADMIN_EMAIL"
  * Admin Console needs a text field under "Networking" where the admin can enter their Pi-hole's externally contactable FQDN, allowing access to their landing page when browsing to mypi.duckdns.org, to be saved to setupVars.conf with the key "FQDN"
  * I am not aware of expected output of `$_SERVER["VIRTUAL_HOST"]`, so I have assumed it should be filtered as if it's a domain

@WaLLy3K WaLLy3K referenced this pull request May 2, 2017

Merged

QueryFunc() rewrite #1417

5 of 5 tasks complete

@WaLLy3K WaLLy3K referenced this pull request May 2, 2017

Merged

Disable HTTPS & shift functions to block page #1420

5 of 5 tasks complete

@WaLLy3K WaLLy3K changed the title [WIP] Blockpage 2.0 Block Page 2.0 May 2, 2017

WaLLy3K added some commits May 3, 2017

Updated landing page location
During development, I had the location of `/pihole/index.php` as just `/index.php`. Just correcting some changes!
@dschaper

This comment has been minimized.

Copy link
Member

dschaper commented May 5, 2017

index.js is no longer needed?

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented May 5, 2017

Just to clarify, #1420 removes the Javascript redirect from lighttpd.conf, and this line will redirect users to a blank but valid Javascript filler.

@DL6ER DL6ER referenced this pull request May 6, 2017

Closed

Unit testing for #1437 #1441

3 of 3 tasks complete
@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented May 11, 2017

Have already implemented the fix as proposed in #1452.

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented May 11, 2017

Remaining commit order:

  • AdminLTE #488 Supporting aLTE change for core tweak/blockpage branch
  • This PR, #1416
  • #1420 Disable HTTPS & shift functions to block page

Once AdminLTE #488 is done, queryads.js will need to be adjusted for the new pihole -q output.

The block page will attempt to use FQDN (external RPi address)/ADMIN_EMAIL (block page email contact) keys from setupVars.conf to provide extra functionality, so input fields will need to be added to the Admin Console at a later point.

@WaLLy3K WaLLy3K requested a review from DL6ER May 13, 2017

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented May 18, 2017

Note to self: confirm and fix issue #1479 before continuing

@dschaper
Copy link
Member

dschaper left a comment

Fix #1479 before merge

@WaLLy3K
Copy link
Collaborator Author

WaLLy3K left a comment

Now using HTTP_HOST to fix #1479

@WaLLy3K WaLLy3K changed the title Block Page 2.0 [WIP] Block Page 2.0 May 18, 2017

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented May 18, 2017

Renamed [WIP], awaiting merge of AdminLTE #488.

// or no extension (index access to some folder incl. the root dir)
$showPage = true;
// Determine block page redirect
if ($serverName === "pi.hole") {

This comment has been minimized.

@diginc

diginc May 18, 2017

Member

Shouldn't all of the AUTHORIZED_HOSTNAMES be redirected?
e.g. localhost, 127.0.0.1, ::1, and $VIRTUAL_HOST variable contents (for my docker ;))

This comment has been minimized.

@WaLLy3K

WaLLy3K May 18, 2017

Author Collaborator

So the logic is that we need to determine if the website that's being visited is one for the Pi, or one on a block list.

Virtual Host will redirect to the splash page (Local Pi landing page) by L42, while everything else is covered by checking if it's a valid IP in L68.

@@ -1 +1 @@
var x = "Pi-hole: A black hole for Internet advertisements."
var x = ""

This comment has been minimized.

@DL6ER

DL6ER May 21, 2017

Member

You can remove the file in another PR (where the source branch is on the team repo). I can adjust the tests for your.

@Mcat12 Mcat12 referenced this pull request Jun 10, 2017

Merged

Supporting aLTE change for core tweak/blockpage branch #488

5 of 5 tasks complete

@Mcat12 Mcat12 added this to the v3.2 milestone Jun 10, 2017

@WaLLy3K WaLLy3K removed this from the v3.2 milestone Jul 2, 2017

@WaLLy3K WaLLy3K changed the title [WIP] Block Page 2.0 Block Page 2.0 Jul 18, 2017

@WaLLy3K

This comment has been minimized.

Copy link
Collaborator Author

WaLLy3K commented Jul 18, 2017

Currently required merges to support this PR:

All are ready to go.

@WaLLy3K WaLLy3K added this to the v3.3 milestone Jul 20, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Sep 3, 2017

We still need to get around the installer not updating the block page on people's installs.

I guess we could just make the decision to not support custom block pages, in order to ensure that the block page the user is running is always up to date.

Failing that, could we add a file detection/redirect to this block page? e.g if /var/www/html/pihole/customBlock.php exists, redirect to that instead of displaying this one...

@Mcat12

This comment has been minimized.

Copy link
Member

Mcat12 commented Sep 3, 2017

I like the idea of having a customBlock.php file override the default instead of having to do some strange change detection.

@PromoFaux PromoFaux referenced this pull request Sep 9, 2017

Merged

Allow for Custom block page #1688

7 of 7 tasks complete

@PromoFaux PromoFaux modified the milestones: v3.3, v3.2 Sep 14, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Sep 14, 2017

Approved

Approved with PullApprove

@WaLLy3K WaLLy3K merged commit e7ae62b into pi-hole:development Sep 14, 2017

4 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/pullapprove Approved by PromoFaux, WaLLy3K
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@WaLLy3K WaLLy3K deleted the WaLLy3K:blockpage2 branch Sep 14, 2017

WaLLy3K added a commit that referenced this pull request Oct 4, 2017

WaLLy3K added a commit that referenced this pull request Oct 7, 2017

Merge pull request #1723 from pi-hole/tweak/bpcss
Ensure Block Page (#1416) matches current style of Web Interface

WaLLy3K added a commit that referenced this pull request Oct 8, 2017

Merge pull request #1722 from pi-hole/fix/bpfqdn
Improve FQDN Authorized Hosts functionality (Fix for #1416)

WaLLy3K added a commit that referenced this pull request Oct 9, 2017

Merge pull request #1692 from andofrjando/cors_mixed_content_fix
Fix for bug on block page (#1416) caused by CORS mixed content when behind reverse proxy using SSL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.