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

Add support for TLD check and allowing Cross-Origin iframes option #2079

Merged
merged 3 commits into from
Jan 27, 2024

Conversation

varjolintu
Copy link
Member

Adds support for checking Top-Level-Domains from hostnames, and returning a base domain. The implementation is almost similar to KeePassXC's keepassxreboot/keepassxc#9935 that is using a cookie API for TLD's (there's no direct API for it).

If a Cross-Origin iframe is detected, the popup will show a new option:
Screenshot 2024-01-14 at 10 10 45
Pressing the button will add the tab/window URL to Site Preferences and enables the new "Allow Cross-Origin iframes" option.

Content script's isIframeAllowed() now performs the following:

  • Checks if Cross-Origin iframes are allowed by Site Preferences.
  • Checks the base domain of window/iframe using TLD list.
  • Allows the iframe automatically if it's under the same base domain.
  • Disallows iframes that do not match with the tab's/window's URL.

Unit tests are disabled for now, because it's unclear how the background script can be tested with Cookies API enabled. I ran the same tests directly in the background script when making the feature, and all of those passed.

Fixes #2076.

@varjolintu
Copy link
Member Author

varjolintu commented Jan 15, 2024

There are some problems with the automatic subdomain detection with Firefox. Trying to fix it.

EDIT: It's a problem with First-Party Isolation enabled. Seems there might be a bug in Firefox. So this feature will not work for e.g. Tor Browser automatically. User needs to make a manual exception.

@Felix-Official
Copy link

The same problem. How can I get the fixed version?

@varjolintu
Copy link
Member Author

@Felix-phi What do you mean?

@varjolintu varjolintu mentioned this pull request Jan 26, 2024
Copy link
Member

@droidmonkey droidmonkey left a comment

Choose a reason for hiding this comment

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

Works perfectly on the test/issue sites

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Credential request ignored from another domain
3 participants