inpsyde / wp-translation-downloader

Composer plugin to download WordPress translations
MIT License
46 stars 3 forks source link

[Feature Request]: Add possibility to configure auth for GlotPress #35

Closed Chrico closed 1 year ago

Chrico commented 1 year ago

Is your feature request related to a problem?

Most of the time when using inpsyde/wp-translation-downloader, we're dealing with custom client code and using a self-hosted GlotPress installation to manage those translation files.

Due security and privacy reasons we need to protect the translation files behind some authentication. This is currently not supported when resolving the packages to endpoints. The only option we have is to use Basic Auth and add username:password into the API endpoint url in wp-translation-downloader.json-file.

Describe the desired solution

In order to identify the wp-translation-downloader doing requests we should consider following enhancements/possibilites:

Enhancement: Add identifier to request header

When calling via TranslatablePackage::readEndpointContent() the JSON Endpoint of GlotPress, we should add some custom HTTP Request Header to identify that this request is comming from inpsyde/wp-translation-downloader

New feature: Make similar use as it is in Composer auth.json

We could implement a similar logic like Composer does with the auth.json (and/or env var). This would basically allow us to configure something like:

wp-translation-downloader-auth.json

{
    "http-basic": {
        "my-custom-glotpress.com": {
                "username": "{user}",
                "password": "{password}"
        },
    },
    "bearer": {
        "my-custom-glotpress.com": "{token}"
    }
}

By allowing this, we could even configure the JSON via Github Actions as "secret" and injecting it into the env when running the Github Action.

Code of Conduct

2ndkauboy commented 1 year ago

While having the username:password in the URL works in a browser, it does not work when using the update command. But is t asks for the username and password, which might also be OK as then we don't have to hardcode credentials into a "published" configuration file and only developers with knowledge of the basic auth credentials can do the update.

 - project: found 2 translations. Installing...
    Endpoint: https://userame:password@example.com/glotpress/api/translations/project?version=2.0.26
  - Downloading wp-translations-downloader/project-de_de-2022-11-02t10-13-12-00-00 (2.1.0-development)
    Authentication required (example.com):
      Username: username
      Password: 
gmazzap commented 1 year ago

@Chrico are you sure this is not supported already? :)

The package uses Composer's downloaders. And Composer's downloaders will use whatever configuration is set in the Composer's auth.json

So if you use auth.json to set auth for the domains of the custom Glotpress installations, it should just work.

For example, having a configuration like this:

{
    "languageRootDir": "wp-content/languages",
    "languages": ["de_DE", "it_IT"],
    "api": {
        "names": {
            "acme/*": "https://glotpress.acme.tld/api/translations/%packageName%?version=%packageVersion%"
        }
    }
}

then if you set in Composer's auth.json:

{
    "http-basic": {
        "glotpress.acme.tld": {
            "username": "username",
            "password": "s3cr3t!"
        }
    }
}

Then Composer should just understand it and pass basic auth credentials to Glotpress.

Of course... it has to be tested :)

Chrico commented 1 year ago

@Chrico are you sure this is not supported already? :)

Yes and no, For downloading the translation zip files via Composer Packages it is might be supported, but not for getting the JSON file which contains the ZIP files.

JSON Request is not using any auth: https://api.wordpress.org/translations/core/1.0/

--> https://github.com/inpsyde/wp-translation-downloader/blob/2.3/src/Package/TranslatablePackage.php#L163-L165 🙈

ZIP Request might does: https://downloads.wordpress.org/translation/core/5.8-beta/af.zip

--> https://github.com/inpsyde/wp-translation-downloader/blob/2539fd343a5540871f15a9c9b0dd569a759f26b0/src/Util/TranslationPackageDownloader.php#L122-L127 🤔

@gmazzap

gmazzap commented 1 year ago

Ah, ok @Chrico

Do we need to protect getting the JSON file?

If so, are we sure that the credentials for getting the JSON file are the same to get the zip files?

Because otherwise, the configuration format should allow a double configuration...

If the auth can be assumed the same, we could use Composer FileDownloader instead of file_get_contents() and so use the same auth config used to download the zip files.

2ndkauboy commented 1 year ago

It looks like currently you have to add the basic auth credentials in two places:

  1. In the api/name URL in the wp-translation-downloader.json file
  2. In the auth.json file

The credentials are the same for both. Setting it in both places allows us to successfully pull translations from a GlotPress instance protected with basic auth.

Chrico commented 1 year ago

Do we need to protect getting the JSON file?

That's a question we need to answer and decide.

I guess there is not much data which will be exposed by reading the GlotPress JSON endpoint. More important is to protect the ZIP files from being downloaded. But at the same time, the problem is, that the custom GlotPress just links the ZIP files which are stored in /wp-content/traduttore/{project}-{version}.zip. Auth can only be added via server configuration and not via PHP since those files are accessed directly.

So I would suggest to also support auth for the JSON Endpoint.


The credentials are the same for both.

Thats good to know, we should document that down. But it's also kind of not ideal since it only supports Basic Auth this way and also exposes username/password.

gmazzap commented 1 year ago

Right now, you have to enter the credentials in both places because one is used by file_get_contents and the other by Composer downloader.

if we use a Composer downloader for the JSON, then only auth.json would suffice.

@Chrico I guess we can start with that, it should be easy enough. I could work on this... if i can find one hour in the next few days. But if anyone wants to do it, feel very free.

Chrico commented 1 year ago

@gmazzap as you want, just go for it 💪🏻

I guess we can just load those "available translations" outside in the TranslatablePackageFactory and inject them into the TranslatablePackage. Makes no sense to load them inside of a Package due "side effects".

Chrico commented 1 year ago

I've found some time during easter to implement this feature. See https://github.com/inpsyde/wp-translation-downloader/pull/38

CC @gmazzap