Package localization

[tpaksu]

Hello,

I found out a way to translate this package's response strings in laravel/passport package, but there's something I can't figure out and I need your thoughts about it.

The exception response body that oauth2-server provides contains:

    [ 
        "error" => $key
        "message" => $message,
        "hint" => $hint
    ]

which the error parameter I can use as key for translating the string into another languages. But, this package has dynamic messages using same keys and I want to distinguish between them.

For example;

OAuthServerException.php line 131-158:

/**
     * Server error.
     *
     * @param $hint
     *
     * @return static
     *
     * @codeCoverageIgnore
     */
    public static function serverError($hint)
    {
        return new static(
            'The authorization server encountered an unexpected condition which prevented it from fulfilling'
            . ' the request: ' . $hint,
            7,
            'server_error',
            500
        );
    }

The server_error key might be a lot of things, because the hint parameter is concatenated to the error message.

Also,

OauthServerException.php line 151-161:

/**
     * Invalid refresh token.
     *
     * @param null|string $hint
     *
     * @return static
     */
    public static function invalidRefreshToken($hint = null)
    {
        return new static('The refresh token is invalid.', 8, 'invalid_request', 401, $hint);
    }

and

OauthServerException.php line 68-83

/**
     * Invalid request error.
     *
     * @param string      $parameter The invalid parameter
     * @param null|string $hint
     *
     * @return static
     */
    public static function invalidRequest($parameter, $hint = null)
    {
        $errorMessage = 'The request is missing a required parameter, includes an invalid parameter value, ' .
            'includes a parameter more than once, or is otherwise malformed.';
        $hint = ($hint === null) ? sprintf('Check the `%s` parameter', $parameter) : $hint;

        return new static($errorMessage, 3, 'invalid_request', 400, $hint);
    }

have the same key as invalid_request.

May I change that or add more keys to the Response object which differentiates the error messages?

For example, I'm thinking of doing something like this:

A response object example:

[
    "error" => "invalid_request",
    "message" => "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.",
    "hint" => "Check the `code` parameter",
    "translatable" => [
          "error_key" => "invalid_request.code",
          "hint_key" => "invalid_request.code.hint",
          "error_parameters" => [],
          "hint_parameters" => [ "code" ]        
    ]
]

Which then I can get a translation string (for [response]->translatable->hintkey) like "Check the %s parameter" and fill it with hint_parameters array. Then the receiving package (like laravel\passport) can dismiss the "translatable" key from the response.

What do you think?

[tpaksu]

Will also fix #747.

[Sephster]

I think this implementation could be simplified. Ideally what I'd like to see in the package is a language file which will hold all of the various error messages and refactor the library to read from this language file instead of have the strings hard coded.

By doing this, we can then just specify the language we want to use and have the code retrieve the appropriate message so we don't need to do on-the-fly translations. The codebase should retrieve the appropriate string straight from the language file itself.

I think it is appropriate that we can have differing messages for the same error type. For example, a run time exception can be raised for many different reasons but in general, devs just raise a run time exception with a custom message to hint at the more specific issue. Obviously this can only be done up to a point before you want to separate out your exception types but I think the current implementation doesn't require this.

[tpaksu]

Hi @Sephster ,

TL;DR oauth2-server package doesn't give the user the ability to customize the messages, still it would be hardcoded localized strings for the user. but packages with laravel service providers have this option. By adding this to oauth2-server package, we would destroy this package's portability.

---

When thinking in the Laravel scope, the user would do something like this (if I understood you correctly):

    "invalid_key" => "Your key seems to be expired, please send an email to [email protected] with the key "abcdefg" to request a new one",

If I put this message inside oauth2-server package, it will overwrite the message every time the package gets updated, right?

Instead of this, I would prefer to export the laravel/passport package's language file to the application resource folder (within the service provider's boot method - oauth2-server doesn't have one because it fits not only in laravel, but also in every framework which supports composer).

And for doing this, I would need the dynamic error messages, less dynamic, or expose more information to it's parent package which then could handle the message translations. It'll be better that laravel/passport handles the translation feature and phpleague/oauth2-server to be more open for translation.

Or maybe, just a thought, we can use a separate class which builds these error messages with sprintf and variable arrays, and then use something like dependency injection in the base class to inject the custom message builder to the oauth2-server package, to enable overriding the stock message builder class.

[tpaksu]

Until I realized that some messages are dynamically generated, this was my implementation:

in laravel/passport package I've added a new method to src/Http/Controllers/HandlesOAuthErrors.php:

 /**
     * Translates the response content
     *
     * @return \Illuminate\Http\Response
     */
    protected function translateResponse(Response $httpResponse)
    {
        // get response content as object
        $content = json_decode($httpResponse->content(), false);

        // backup the old message and hint properties
        $message_backup = property_exists($content->message) ? $content->message : false;
        $message_hint_backup = property_exists($content,"hint") ? $content->hint : false;

        // translate strings
        $content->message = trans("passport::messages.".$content->error);
        $content->hint = trans("passport::messages.".$content->error."_hint");

        // if the translation file doesn't contain the key, restore from backup, and if property didn't exist before, delete the keys from the message
        if($content->message == "passport:messages.".$content->error) $content->message = $message_backup;
        if($content->message == false) unset($content->message);
        if($content->hint == "passport:messages.".$content->error."_hint") $content->hint = $message_hint_backup;
        if($content->hint == false) unset($content->hint);

        return $httpResponse->setContent(json_encode($content));
    }

which modifies this part:

catch (OAuthServerException $e) {
            $this->exceptionHandler()->report($e);

            return $this->translateResponse($this->convertResponse(
                $e->generateHttpResponse(new Psr7Response)
            ));
        }

and it gets the translations from resources/translations/en/messages.php which can be published into app/resources/lang/vendor/passport/en/messages.php in the main framework layer. Still keeping the portability of oauth2-server package.

[strongholdmedia]

Many of the errors are returned from Grants. Sadly, the way Passport is integrated with Laravel currently does not make it quite easy to override/extend parts of it, but it can be done.

[lordrhodos]

@Sephster I have been looking into possible ways of translation handling and I have some question about the approach you prefer.

From what I understand all issues and requests related to translation are fully optional and related to the provided error responses based on the OAuthServerException::generateHttpResponse method:

https://github.com/thephpleague/oauth2-server/blob/4e4a6b6a7e6c2e9cdfefe6d2fd310de4fa4abd8d/src/Exception/OAuthServerException.php#L291-L316

As I like this as a convenience method I think the generation of the error responses should be decoupled from the exceptions. The exception themselves should never be translated. They belong to the process logic of this library and should be decoupled from the error responses.

To add support for translatable error responses I would recommend to move the generation of the error responses into a dedicated exception response handler which gets injected a translator interface. The library will be shipped with a default translator.

I think this implementation could be simplified. Ideally what I'd like to see in the package is a language file which will hold all of the various error messages and refactor the library to read from this language file instead of have the strings hard coded.

By doing this, we can then just specify the language we want to use and have the code retrieve the appropriate message so we don't need to do on-the-fly translations. The codebase should retrieve the appropriate string straight from the language file itself.

Is XML Localization Interchange File Format (XLIFF) support ok for you? I think the exception messages can still remain in the codebase. They can be used as translation keys for a response handler.

The symfony/translator package is quite powerful and offers a lot of features, but I think it comes with way too much overhead, so instead I would go with one translation file format and come up with an integrated loader / reader for that format.

What do you think?

[strongholdmedia]

This idea is excellent.

In fact I am yet to see a single instance of actual implementation in which the exceptions have been translated. But given this is a feature, and it already exists, I am not against it per se.

That being said, the notion of Symfony translator being overkill is absolutely on spot. If that is a default implementation, fine - but please provide options for both overriding it and turning it off altogether.

I personally prefer the "array key"/"hashmap" methods across PHP stuff, but then again, others prefer gettext, ... Then again, localization and translation are related, but distinct concepts. This may present a confusion for people mixing up these. For a simple translation, one that should suffice here, any proper localization approach (with plurals, date/clock format, and stuff) is probably overkill.