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 Modrinth modpack support #1352

Closed
wants to merge 1,899 commits into from

Conversation

Pyroglyph
Copy link

@Pyroglyph Pyroglyph commented Jun 3, 2022

Purpose

Currently, GDLauncher does not support downloading mods/modpacks hosted on the Modrinth platform. This feature has been requested in issue #768.

I'm aware of GDevs' other project ferium which already supports Modrinth, and also the gdlib-refactor branch (which I assume may eventually succeed GDLauncher?), but that branch is far from completion.

Approach

This PR allows users to search for and install mods and modpacks hosted on Modrinth in the same way that they can currently be installed from CurseForge and FTB.

image

The implementation is highly based on the existing CurseForge/FTB installer code so it adds no new dependencies, which I see as an advantage.

Open Questions and Pre-Merge TODOs

  • Add support for searching for and installing modpacks from Modrinth
  • Add support for searching for and installing individual mods from Modrinth
  • Add a proper way to show errors to the user (i.e. when a mod fails to download, or when the user tried to install an unsupported pack type like Quilt)
  • Modrinth provides fallback download links for overrides. If an override file fails to download, the launcher should retry the download using given alternate links.
  • Should we send a Google Analytics event for Modrinth API calls?

Learning

I have read the Modrinth API docs (linked below).

Resources

blarfoon and others added 30 commits June 1, 2021 06:42
Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
* Change Maximum Value for Max Memory in Global Settings

* Change Maximum Value for Max Memory in Instance Settings

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
s/syncronizing/synchronizing

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
CurseForge doesn't (or no longer) users the yarn property in the
manifest to specify the fabric version. Instead they now include the
version in id property, ex: `fabric-0.11.6`.

When importing an instance we'll check for the yarn property first, for
backwards compatibility, but will then try to extract it from the id if
it's missing.

For exporting we'll now include the loader version in the id and omit
the yarn property, exactly like CurseForge.

Fixes gorilla-devs#942

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
…s | Better progress update (gorilla-devs#962)

* Changed updatePercentage() function to defaut

Changed the function to what all the other Downloads use.
- Checking if progress changed => less CPU load
- Using parseInt() function to prevent errors

* Applied concurrent download setting to FTB file-download on instance creation

* Codacity means I should do that :/
* Added usual MacOS default menu

New Features:
 - option to open About Page
 - Services Submenu
 - Hide Others
 - Show All

* Changed requested Edits

- Wrong quote on line 78
- Revert to original click lambda

* …And another wrong quote

* ...fixed wrong quote

Wow, seems that this happend quite often :/

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
Co-Authored-By: Mr. Hallway <git@mrhallway.me>

Co-authored-by: Mr. Hallway <git@mrhallway.me>
* Fixed Problem whit hidden accounts in modal

Just had to add overflow: auto; and a padding to the parent element.

* Moved CSS-Fix for Account Modal to container initialization

Co-Authored-By: Mr. Hallway <git@mrhallway.me>

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
Co-authored-by: Mr. Hallway <git@mrhallway.me>
Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
* Auto-detect CurseForge modpack loader type

Currently all CurseForge modpacks are assumed to be using Forge, whereas
they may actually be using Fabric. Instead of hardcoding which one is
being used, we can parse the type from the manifest.

Fixes gorilla-devs#961

* Support changes versions of fabric modpacks

A few refactors were needed to enable this:

  - Created a utility for extracting the fabric version from a manifest
  - Revised the change version flow to properly parse fabric vs forge
    loader versions (previously it just assumed it was a forge modpack)
  - Reworked the CreateInstance flow to better support code-sharing

Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
@Eskaan
Copy link
Collaborator

Eskaan commented Jun 20, 2022

Honestly, that's Modrinths problem. It's just not our part to deal with potentially malicious executables. They could do that way more effectively and they can be uploaded as plain mods the same way.

@Pyroglyph
Copy link
Author

It's just not our part to deal with potentially malicious executables

Good point. It's a reasonable expectation that the user has a functioning AV on their system. Be it a paid product or just Windows Defender, if something like that isn't running then that's on them.

@blarfoon
Copy link
Member

I think it is our responsibility to protect our users the best we can, we should wait for modrinth to implement some kind of filters and validation on what type of files can be uploaded before merging this.

@Pyroglyph Pyroglyph marked this pull request as draft June 21, 2022 08:03
@Pyroglyph
Copy link
Author

In the interest of getting this finished sooner, I've come up with another option we could implement while Modrinth do things on their side.
What if, upon detecting an executable file or script in the mrpack, we simply show users a message that the pack is invalid and to try another version? This prevents scripts leaving the archive but still allows players to play versions of the pack that are fine. This should strike a balance between security and user experience.
Obviously, not allowing potentially dangerous files on Modrinth in the first place would be the ideal solution, but since that's not a thing right now, this (and the 3 options I mentioned earlier) is the best I can come up with.

@DioEgizio
Copy link

imo it's not a big issue, if there's a malicious file you can report and modrinth will remove it

@Pyroglyph
Copy link
Author

Pyroglyph commented Jun 21, 2022

if there's a malicious file you can report and modrinth will remove it

Except you can't. At least, there is no official channel for reporting malicious content on Modrinth yet. To report something you'd have to go out of your way to find someone to tell, and not many people want to do that.

The point of this conversation is how we can protect users. Sure, adding a report button to the Modrinth page is good, but that won't stop a user downloading a dangerous pack that hasn't been reported yet. We should be doing what we can to prevent that happening.

@DioEgizio
Copy link

if there's a malicious file you can report and modrinth will remove it

Except you can't. At least, there is no official channel for reporting malicious content on Modrinth yet. To report something you'd have to go out of your way to find someone to tell, and not many people want to do that.

The point of this conversation is how we can protect users. Sure, adding a report button to the Modrinth page is good, but that won't stop a user downloading a dangerous pack that hasn't been reported yet. We should be doing what we can to prevent that happening.

IMG_20220621_115910
Just so you know

@Pyroglyph
Copy link
Author

My bad, I never signed into the website so I never saw that button.

My point still stands though. Not many normal users will have a GitHub account, nor will they sign into the Modrinth website in order to find the report button. Most likely their only interaction with the platform is going to be through the launcher itself, not signed in through the website.

This can be removed once Modrinth begin blocking potentially malicious scripts/binaries
@Ecorous
Copy link

Ecorous commented Jun 21, 2022

.sh and .AppImage aren't on there @Pyroglyph

@Eskaan
Copy link
Collaborator

Eskaan commented Jun 21, 2022

An executable dosen't even need a specific extension (see Linux). It could be any extension and still be executed. Another question would be how those files would be executed in the first place

@AshtakaOOf
Copy link

It can, but extension are used to identify, what type of executable it is from a glance.
And yes if we have to find a way to stop the download of malicious executable like .exe and other it’s going to be really hard.
On linux there are a lot of executable, Here a few: .sh .py .jar .AppImage .x86_64 and more.
The solution would be to make gdlauncher scans every files to see if they are executable, or wait for modrinth to do something about it.

@Eskaan
Copy link
Collaborator

Eskaan commented Jun 21, 2022

Ok Linux you can use any extension for literally anything as long as you include the shebang
Edit: So the filter is practically worthless against a hacker trying to circumvent it.

@Pyroglyph
Copy link
Author

Another question would be how those files would be executed in the first place

That's something I hadn't really thought of. The only way I can think of is if there is also a rogue mod included in the pack. And if that's the case then the bad actor could easily have made that mod a dropper or made it do what the script/binary would have done anyway.

the filter is practically worthless against a hacker trying to circumvent it

Considering the above, I agree. I suppose this stuff really is for Modrinth to deal with, as I don't think we can do anything to effectively combat it on our side. While we wait for that to happen, I'll continue to work on the rest of the PR.

@DioEgizio
Copy link

My point still stands though. Not many normal users will have a GitHub account, nor will they sign into the Modrinth website in order to find the report button. Most likely their only interaction with the platform is going to be through the launcher itself, not signed in through the website.

Looks like it's not necessary anymore (modrinth/knossos@371f14d)

@Ecorous
Copy link

Ecorous commented Jun 23, 2022

[ x ] TODO: Swap reading dependencies for reading manifest

@AshtakaOOf
Copy link

@Pyroglyph Something is wrong i can feel it

@CLAassistant
Copy link

CLAassistant commented Jun 30, 2022

CLA assistant check
All committers have signed the CLA.

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

Successfully merging this pull request may close these issues.

None yet