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

Disable Brave redirect when Companion global redirect is enabled #1301

Open
lidel opened this issue Oct 13, 2023 · 3 comments
Open

Disable Brave redirect when Companion global redirect is enabled #1301

lidel opened this issue Oct 13, 2023 · 3 comments
Labels
area/brave Issues related to Brave Browser kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@lidel
Copy link
Member

lidel commented Oct 13, 2023

This is MVP subset of #1281 requested by the Brave team.

Problem

Currently, Companion logic is clashing with Brave logic when both have redirects enabled.

Preconditions:

"Method to resolve IPFS resources": Brave local IPFS node
"Automatically redirect requests for IPFS network resources to the configured gateway.": is ON
IPFS daemon is started, but repository is cleaned
Network access is blocked

Steps:

  1. You try to open the address: https://dweb.link/ipfs/bafkreiedqfhqvarz2y4c2s3vrbrcq427sawhzbewzksegopavnmwbz4zyq
  2. Brave tries to redirect request throug the local node:
    https://localhost:45008/ipfs/bafkreiedqfhqvarz2y4c2s3vrbrcq427sawhzbewzksegopavnmwbz4zyq

Expected results:
The request: https://localhost:45008/ipfs/bafkreiedqfhqvarz2y4c2s3vrbrcq427sawhzbewzksegopavnmwbz4zyq
is failed, infobar with proposition to load original URL (i.e. from step1) is shown

Actual result:
In case of IPFS Companion is enabled: It redirects us to ipfs://bafkreiedqfhqvarz2y4c2s3vrbrcq427sawhzbewzksegopavnmwbz4zyq as well,
and redirect happens again once we try to redirect to original URL.
So we never can redirect back to original URL
Sometimes I even observed redirect loop (edited)

Proposed fix

The provisional fix is to periodically check if all below are true:
- Companion's global redirect is enabled
- Companion's backend type is Kubo RPC provided by Brave
- Brave settings have redirect enabled (read via chrome.ipfs.getSettings)

If true, we want to

  • store Brave setting and timestamp for later
  • disable the Brave redirects (via chrome.ipfs.setGatewayFallbackEnabled(false) and chrome.ipfs.setAutoRedirectToConfiguredGatewayEnabled(false)), ensuring only Companion ones are executed
@lidel lidel added kind/bug A bug in existing code (including security flaws) area/brave Issues related to Brave Browser need/triage Needs initial labeling and prioritization labels Oct 13, 2023
@lidel
Copy link
Member Author

lidel commented Oct 13, 2023

@whizzzkid fysa I've filled this as a smaller scope of work when compared to #1281 to unblock Brave.

Does this sound sensible, or is there a better way you see to avoid redirect clash?

@whizzzkid
Copy link
Contributor

@lidel thanks, yes this sounds reasonable, one thing I'm not really sure about is why does this need to be "periodically" can't they not persist these values? Also, there is a priority flag for redirect rules. I'm guessing those are being ignored.

@lidel
Copy link
Member Author

lidel commented Oct 14, 2023

re: periodically: Brave user can toggle Brave redirect from OFF to ON again via brave://settings etc, but maybe it will be enough if we check when that URI is opened and on extension start?

Those are different than web extension ones, so priority is most likely ignored.

@whizzzkid whizzzkid added P1 High: Likely tackled by core team if no one steps up and removed need/triage Needs initial labeling and prioritization labels Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/brave Issues related to Brave Browser kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up
Projects
No open projects
Status: Needs Grooming
Development

No branches or pull requests

2 participants