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

Feature/add http referrer allow list option #25

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

jkeasley
Copy link
Contributor

This commit adds the Referrer Allow list option, which can be used
to whitelist traffic arriving from certain referrers (internal only)
which is intended to allow plugins like Nelio AB Testing to be usable
on sites which use this plugin to control access to the site.

internal referrer urls can contain query strings, but the input
config should omit the site url, as this is set inside the plugin,
to prevent external referrers from being configured.

The new functionality also only accepts as valid referrer headers
where the configured allowed referrer string appears at the start
of the referrer header, to prevent whitelisted items being passed
as parameters of a referrer to circumvent the access controls.

loading an allow listed referrer url directly in the browser while
unauthenticated does not allow the user to bypass the access control
as when the plugin performs redirection it does not, itself, send the
HTTP_REFERER header.

  • Version number has been bumped

To test:
(the tests are unfortunately specific to judiciary intranet currently)

  • in a local instance of judiciary-intranet, with nelio-ab-testing enabled and configured create a heatmap test for page http://localhost
  • ensure that dxw-members only is configured to redirect visitors to the homepage to the login screen
  • stop the test and attempt to view the results, this should time out with an error image (pictured below, first screenshot)
  • in settings -> dxw members only add the url /wp-admin/admin.php?page=nelio-ab-testing-experiment-view to the Referrers Allow List config
  • return to the nelio AB section and attempt to view the results of the test you created, the display should load as per screenshot 2 (below)
  • log out of the local site and attempt to access the url http://localhost/wp-admin/admin.php?page=nelio-ab-testing-experiment-view you should be redirected back to the login screen.

Screenshot 1, non-allow-listed referrer behaviour
Screenshot 2023-10-23 at 10 24 33

Screenshot 2, referrer allow listed behaviour
Screenshot 2023-10-23 at 10 27 39

This commit adds the Referrer Allow list option, which can be used
to whitelist traffic arriving from certain referrers (internal only)
which is intended to allow plugins like Nelio AB Testing to be usable
on sites which use this plugin to control access to the site.

internal referrer urls can contain query strings, but the input
config should omit the site url, as this is set inside the plugin,
to prevent external referrers from being configured.

The new functionality also only accepts as valid referrer headers
where the configured allowed referrer string appears at the start
of the referrer header, to prevent whitelisted items being passed
as parameters of a referrer to circumvent the access controls.

loading an allow listed referrer url directly in the browser while
unauthenticated does not allow the user to bypass the access control
as when the plugin performs redirection it does not, itself, send the
HTTP_REFERER header.
This commit adds a check that the config array for the referrer
allow list contains valid options, as the behaviour of explode in
this context means that even if the options field is empty at least
one array item, of value '' with be created.

Also Kahlan has been removed from the composer file and composer update
rerun
@jkeasley jkeasley force-pushed the feature/add-http-referrer-allow-list-option branch from bc4790f to f733765 Compare October 23, 2023 09:37
Copy link

@serena-piccioni serena-piccioni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkeasley jkeasley merged commit 3ffecbb into master Oct 23, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants