Fox2Code / FoxMagiskModuleManager

A module manager for Magisk because the official app dropped support for it
GNU Lesser General Public License v3.0
2.19k stars 131 forks source link

Add Repo box is not clear enough about which link to put. #216

Closed Jon8RFC closed 1 year ago

Jon8RFC commented 2 years ago

Describe the bug Because Androidacy appears to be down/dysfunctional, I'm trying to add the old Magisk Repo but it isn't working. A test scenario of disabling the Alt Repo and manually adding the Alt Repo also fails.

When adding a custom repo, upon testing and tapping on "website", it truncates and removes everything after the domain--no subdirectories are functional if entered by the user. Entering https://github.com/Magisk-Modules-Alt-Repo functions as https://github.com

For grins, I changed my root index.html to be a redirect to a repo. That only permits the "website" function of the app to work properly, and it does, in fact, redirect to a subdirectory. However, the app will not pull from the redirected repo, which is probably to be expected. Not a complaint, not a suggestion, not anything at all other than I like to be thorough, tinker, test, and I'm sharing what I experimented with...which also applies to other things below. Just sharing information ahead of time to make things easier to understand and plead my case for the suggested changes 😀

No error message or notification of requirements. Forcing a trailing slash at the end of a root-level domain probably technically follows the applicable RFCs (whichever ancient ones they are), but few sites/browsers follow that and that's pretty normal. There is a function in some web servers to force a redirect to a trailing slash for a subdirectory, which certainly follows the applicable RFCs. Again, it's pretty common for trailing slashes to be missing and most software and browsers either don't care or work around it. Unfortunately, at least on Apache httpd, that redirect only functions for subdirectories, not for the root directory hosted. But even if it's technically correct in whichever RFC that is, it seems like more sites/browsers don't respect that than do.

So, if a repo happens to be hosted on the root of a server, such as https://google.com, the user could copy and paste from their browser, but Fox MMM will not accept that URL because it's lacking a trailing slash. However, it doesn't notify the user of that requirement. Fortunately, it doesn't require a trailing slash on subdirectories 👍

It also doesn't show or mention that case-sensitive https:// is both the only thing permitted, and required, to be prepended to the domain.

If, for some reason, http or https is required in the http get (or whatever goes on in the background) and it can't simply be a domain, then if a user enters a domain without it, I'd suggest seamlessly and silently prepending that for users, automatically, on the backend after they've tapped "OK".

To Reproduce

  1. Attempt to add any one of the following URLs as a custom repo:
    Https://testrepo.com/repo
    http://testrepo.com/repo
    https://testrepo.com
    https://testrepo.com:80
    testrepo.com/repo
  2. Notice that it disallows but also offers no instruction as to why
  3. Disable the Magisk-Modules-Alt-Repo
  4. Add the following URL as a custom repo: https://github.com/Magisk-Modules-Alt-Repo
  5. Notice that it is dysfunctional
  6. Tap on the "Website" option within the newly-added custom repo
  7. Notice that it truncates the subdirectory

Expected behavior For a variety of commonly-typed/pasted URLs to be acceptable, including extremely short domains such as a.a or x.com, non-SSL domains, and root-level domain URLs without an appended slash, and to accommodate many software keyboards (which capitalize the first typed letter by default).

Even if nothing were to change with regard to the URL requirements, there is no instruction to the user as to why a variety of commonly-used URLs are unacceptable, so some direction or hints would probably be helpful.

Device info(please complete the following information):

Thanks for keeping this great project going--it's handy and appreciated!

HuskyDG commented 2 years ago

Magisk-Modules-Repo is down and has many broken link

androidacy-user commented 2 years ago

About the Androidacy repository: there's a build in GitHub actions with the necessary fix, we're just waiting for a proper release atm

Jon8RFC commented 2 years ago

Magisk-Modules-Repo is down and has many broken link It's unrelated to that repo, as best I can understand.

Haha, well that's really embarrassing. I made what I thought was a thorough and comprehensive bug report but I guess I just need some help and pointers.

I'm basically following steps 3-7 in the To Reproduce section my bug report, shown in this screen recording of Fox MMM 0.6.6 on my Pixel 7 Pro: https://youtu.be/ihoB6HqEIM0

But it doesn't function for me, and tapping on the website option doesn't lead to the repository like the built-in one does. Yet I have no idea what I'm doing wrong.

I'm sorry for being so dumb, but could you please help me understand what I'm doing wrong, and what I need to do to successfully add a custom repo? I'd be grateful for some assistance, or if you can link me to where the instructions are, I'll read them silently in shame.

Fox2Code commented 2 years ago

@Jon8RFC this seems to be a documentation issue, you need to put the link to the raw modules.json, not the root of the github repo.

I will keep this issue open as long as I didn't put more explicit indication about this in the app.

Jon8RFC commented 2 years ago

@Jon8RFC this seems to be a documentation issue, you need to put the link to the raw modules.json, not the root of the github repo.

Awesome, thank you very much for explaining that!

It's working as expected.

androidacy-user commented 2 years ago

As this is resolved I'm going to go ahead and close

Fox2Code commented 2 years ago

@androidacy-user I keep it open until I add a warning on the add repo box. But yeah I understand the "this is resolved" part, but I think peoples should not need to go on GitHub to understand how to add custom repos. This is why I am treating this question as an issue that should be fixed in app.

Jon8RFC commented 2 years ago

I don't seem to be able to put have fox mmm use the custom repo I put in. I can only use the Magisk-Modules-Alt-Repo as a "custom" one, which was my original test scenario which worked a few days ago. However, I cannot use one I'm manually creating. It accepts it, it just won't display the contents from the json.

For testing, if I change the ID to something else, it fails. If I use my own URLs and my own test "module", it fails. If I use URLs for a Magisk-Modules-Alt-Repo module switched to the owner's actual repository, rather than the mirrored one on Magisk-Modules-Alt-Repo (such as "acc"), it fails. If I change only the url_zip for a Magisk-Modules-Alt-Repo module to something else, it succeeds and downloads that file. So, I'm thoroughly confused.

Does a custom repo modules.json (manually entered and not included as part of the app) need to have every module ID whitelisted, and is every Magisk-Modules-Repo module blacklisted?

I can't attach a .json, so here it is, zipped: modules.zip

androidacy-user commented 1 year ago

Note non-ssl repos will fail due to networksecurityconfig so that's not going to be allowed.

While we will work on the error messaging for 1.2, the accepted URLs will most likely not change.

Fox2Code commented 1 year ago

Yeah, I think it's a good idea to only allow https, because http repos could lead to user getting hacked easily via MiTM, especially because FoxMMM give root access to installed modules.

brunoais commented 1 year ago

I think it's a bad idea to only allow https repos. However, I'm in favor of placing a warning and a ⚠️ when setting up an http repo. I'm a believe of power to the user but informed power. I believe this middle ground gives that power to the user but also informed

androidacy-user commented 1 year ago

I think it's a bad idea to only allow https repos. However, I'm in favor of placing a warning and a ⚠️ when setting up an http repo. I'm a believe of power to the user but informed power. I believe this middle ground gives that power to the user but also informed

This will not be changed and is non negotiable. HTTP is inherently a major attack vector, especially when the files being served will have unlimited root access. HTTPS will always remain required for all traffic to and from FoxMMM.

brunoais commented 1 year ago

OK. Works for me

Fox2Code commented 1 year ago

@brunoais it's easy to get https repos, things like Let's Encrypt and Cloudflare allow you to get https with no cost.

brunoais commented 1 year ago

I've encountered situations where some (few and specific) websites with political stuff were banned by those two for unknown reasons. Hopefully it doesn't also happen with this kind of software. Hence my WFM 🙂

Fox2Code commented 1 year ago

@brunoais I you want to discuss further telegram may be a better place: https://t.me/Fox2Code_Chat