Skip to content
New issue

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

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

Already on GitHub? # to your account

[Rewards 3.0] Add adaptive captcha modal #25799

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Conversation

zenparsing
Copy link
Collaborator

@zenparsing zenparsing commented Oct 3, 2024

Resolves brave/brave-browser#41403

(This is a port of the existing adaptive captcha user interface, copied from the existing Rewards panel implementation.)

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@github-actions github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Oct 3, 2024
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@zenparsing zenparsing force-pushed the ksmith-rewards-3-captcha branch 2 times, most recently from 44af36f to 783c735 Compare October 3, 2024 21:40
@github-actions github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Oct 3, 2024
@zenparsing zenparsing force-pushed the ksmith-rewards-3-captcha branch from 783c735 to e0904b1 Compare October 4, 2024 00:30
@github-actions github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Oct 4, 2024
@zenparsing zenparsing marked this pull request as ready for review October 4, 2024 14:23
@zenparsing zenparsing requested a review from a team as a code owner October 4, 2024 14:23
DCHECK(!tooltip_popups_[tooltip_id]);
tooltip_popups_[tooltip_id] =
new brave_tooltips::BraveTooltipPopup(profile, std::move(tooltip));
if (!tooltip_popups_[tooltip_id]) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I kept running into this DCHECK while testing. I think there's something odd in the staging environment that keeps trying to show the tooltip over and over. In any case, it seems like it's not an invariant that we can hold.

Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

browser/ui/BUILD.gn Show resolved Hide resolved
@zenparsing zenparsing force-pushed the ksmith-rewards-3-captcha branch from e0904b1 to 59606b8 Compare October 4, 2024 15:00
Copy link
Contributor

github-actions bot commented Oct 4, 2024

[puLL-Merge] - brave/brave-core@25799

Description

This PR introduces several changes to the Brave Rewards system, primarily focusing on the implementation of an adaptive captcha feature and various UI improvements. The changes span across multiple components and files, enhancing the user experience and security of the Rewards system.

Changes

Changes

  1. browser/ui/BUILD.gn:

    • Moved the "brave_adaptive_captcha" dependency.
  2. browser/ui/views/brave_tooltips/brave_tooltip_popup_handler.cc:

    • Modified the tooltip popup creation logic to prevent duplicate popups.
  3. browser/ui/webui/brave_rewards/rewards_page_data_source.cc:

    • Added content security policy for adaptive captcha challenges.
    • Introduced new localized strings for captcha-related UI elements.
  4. browser/ui/webui/brave_rewards/rewards_page_handler.cc and rewards_page_handler.h:

    • Implemented new methods for handling captcha information and results.
    • Modified the ads statement handling to use a new AdsStatement struct.
  5. components/brave_rewards/common/mojom/rewards_page.mojom:

    • Added new structs and methods for captcha handling and ads statement.
  6. New and modified React components:

    • Added account_balance.tsx for displaying account balances.
    • Created captcha_modal.tsx and captcha_modal.style.ts for the captcha challenge UI.
    • Updated various other components for UI improvements and captcha integration.
  7. components/brave_rewards/resources/rewards_page/lib/app_model.ts:

    • Added new interfaces and methods for captcha handling.
  8. components/brave_rewards/resources/rewards_page/lib/webui_model.ts:

    • Implemented logic for updating captcha information and handling results.

Possible Issues

  • The captcha implementation might need thorough testing to ensure it doesn't interfere with normal user flow or cause unexpected behavior in edge cases.
  • The changes to the ads statement handling should be carefully reviewed to ensure accuracy in reporting and display of ad-related information.

Security Hotspots

  1. Captcha implementation:

    • The new captcha feature is a security measure, but its implementation should be reviewed to ensure it doesn't introduce new vulnerabilities.
    • The communication between the captcha iframe and the main application should be scrutinized for potential security risks.
  2. Content Security Policy changes:

    • The modifications to the CSP in rewards_page_data_source.cc should be carefully reviewed to ensure they don't inadvertently allow unintended content or scripts.

Overall, this PR significantly enhances the Brave Rewards system with new security features and UI improvements. The captcha implementation is a notable addition that should improve the system's resistance to automated abuse.

@zenparsing zenparsing merged commit ef97559 into master Oct 4, 2024
17 checks passed
@zenparsing zenparsing deleted the ksmith-rewards-3-captcha branch October 4, 2024 21:23
@github-actions github-actions bot added this to the 1.72.x - Nightly milestone Oct 4, 2024
@brave-builds
Copy link
Collaborator

Released in v1.72.63

bridiver added a commit that referenced this pull request Oct 6, 2024
bridiver added a commit that referenced this pull request Oct 6, 2024
fix gn check that broke again because of an out of order merge of #25799
cdesouza-chromium added a commit that referenced this pull request Oct 9, 2024
For some unknown reason this PR has broken the Windows debug use of
`json::convert_all_numbers_to_string` and others.

Resolves brave/brave-browser#41497

This reverts commit a235817.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CI/storybook-url Deploy storybook and provide a unique URL for each build puLL-Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Rewards 3.0] Add adaptive captcha modal
3 participants