[NOCI] Update bot checking logic #347
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sblaurock
reviewed
Nov 5, 2024
| isBot() { | ||
| if (this.getIsHumanFromSessionStorage()) { | ||
| return false; | ||
| } |
esezen
approved these changes
Nov 6, 2024
sblaurock
approved these changes
Nov 7, 2024
| const humanity = new HumanityCheck(); | ||
|
|
||
| expect(humanity.isHuman()).to.equal(false); | ||
| expect(humanity.hasPerformedHumanEvent).to.equal(false); |
Contributor
There was a problem hiding this comment.
Thanks for fixing the tests! Sorry I didn't do that earlier.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates:
Notes:
This PR was aiming to keep things mostly the same without changing too many things. There are a couple different ways we could go about updating the existing logic and here are a few (feel free to comment if you have other thoughts/ideas as well):
Current Solution:
The current state works well for the most part, but fails if the crawler/scraper is able to perform some "human-like" action due to the current logic we have in place. Once humanity is determined, it will always rely on the isHuman variable that's saved to storage and doesn't do any more checks. This makes it easy for crawlers/scrapers to bypass any additional checks.
Proposed Solution 1 (This PR):
Not a whole lot changed except for checking the User Agent every time before a request is queued in addition to making sure that the user has proven that they're human.
Proposed Solution 2:
Check the User Agent when the client is instantiated and set
sendTrackingEventsto false if it's a bot-like User Agent. This would disable tracking events entirely and no events can be queued.Proposed Solution 3:
The isHuman (
__cnstrc_is_human) variable is currently set after a human-like action has been performed on the page (i.e. scroll, click, mouseover, etc). We could add another condition where we check the User Agent before the isHuman variable ets set, so even if the “user” performs a human-like action, they would be required to have a non-bot-like user agent for events to get queued.