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

Allow nonce checks to provide custom required referrer URL. #17228

Merged
merged 2 commits into from Feb 17, 2021

Conversation

diosmosis
Copy link
Member

Description:

For the GoogleAnalyticsImporter the oauth nonce check fails since the referrer is google. In this case we want to ensure it is from google and not a local URL.

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@diosmosis diosmosis merged commit 715ded1 into 4.x-dev Feb 17, 2021
@diosmosis diosmosis deleted the custom-nonce-referrer-check branch February 17, 2021 22:55
@MatomoForumNotifications

This pull request has been mentioned on Matomo forums. There might be relevant details there:

https://forum.matomo.org/t/google-analytics-importer/48279/1

kl-he added a commit to bruun-rasmussen/matomo that referenced this pull request Nov 28, 2022
Expected referrer would fail a nonce-check, if the referrer was empty or
local. Introduced in 715ded1/matomo-org#17228.
This fix "reverts" to a pure nonce-check, if referrer is empty or
local.

Issue encountered using the the GoogleAnalyticsImporter, also referred
to in matomo-org#17228.
sgiehl added a commit that referenced this pull request Mar 16, 2023
* Allow empty referrer with expected referrer set

Expected referrer would fail a nonce-check, if the referrer was empty or
local. Introduced in 715ded1/#17228.
This fix "reverts" to a pure nonce-check, if referrer is empty or
local.

Issue encountered using the the GoogleAnalyticsImporter, also referred
to in #17228.

* Fixed misunderstanding with localhost-handling

Previous fix would allow localhost, even though expectedReferrerHost
specified something else.

If a referrer is present, it must fail if it is not localhost or
expectedReferrerHost.

* Updated some comments to match what is happening

* Renamed expectedReferrer to allowedReferrer

Renamed expectedReferrerHost to allowedReferrerHost.
Also updated doc-blocks and comments, to reflect the changes.

* Updated comments

* Merged 4.x-dev

* apply some fixes

* fix test

---------

Co-authored-by: Klaus Heigren <klh@bruun-rasmussen.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants