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

Globalization of error messages #615

Closed
vivshankar opened this issue Aug 5, 2021 · 6 comments
Closed

Globalization of error messages #615

vivshankar opened this issue Aug 5, 2021 · 6 comments

Comments

@vivshankar
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I am looking for a way to provide translated error descriptions based on the request locale. This is a common enough requirement when a service is provided globally.

Describe the solution you'd like

  1. Supply a directory of locale bundles in the Config. Nil falls back to the current behavior.
  2. Wrap the hard-coded error message strings with GetTranslatedMessage({message_code}, {default message}). The current messages would be the second parameter. This ensures backward compatibility if the locale bundles are not supplied as part of Config.

Describe alternatives you've considered

Compare based on the verbose error messages and translate prior to sending response. This isn't a scalable approach, given the project will continue to evolve and new messages would be added.

Additional context

@aeneasr
Copy link
Member

aeneasr commented Aug 6, 2021

Hello there and thank you for the idea. Unfortunately the OAuth2 spec does not have translation thought in and requires certain error messages and codes (in plaintext english). So you would fail some certifications (e.g. OpenID Connect) if those errors where in another language. Therefore I am not sure if we should build this into fosite :/

@vivshankar
Copy link
Contributor Author

@aeneasr Thanks for your response.

However, I would like to point out a couple of things.

First, the OAuth 2.0 specification released in 2012 says -

error_description
         OPTIONAL.  Human-readable ASCII [USASCII] text providing
         additional information, used to assist the client developer in
         understanding the error that occurred.
         Values for the "error_description" parameter MUST NOT include
         characters outside the set %x20-21 / %x23-5B / %x5D-7E.

It does not say that the error_description should be in English only. It also does not mandate what that error description text should be.

Second, the proposal here is to allow an optional language bundle directory to be provided by a consumer of fosite. If the language bundle isn't provided or locale-specific files are unavailable, you would fall back to the current message. So there would be no conformance violation.

Third, given that OAuth and Open ID Connect is basically used everywhere in the world now, it doesn't seem right to not provide an option to translate the error description, as desired.

I am happy to make a contribution if that would help.

@aeneasr
Copy link
Member

aeneasr commented Aug 6, 2021

I was specifically referring to the section right above the cited one: https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1

These error messages are checked during certification (we have gone through certification several times) and even small typos can lead to failing certification!

If we were to add translation I would highly recommend to go with a well developed library (maybe you know one?). Alternatively writing your own error writer could also be an option!

@vivshankar
Copy link
Contributor Author

I am not proposing to change the error codes themselves. So unauthorized_client for example is not something I am seeking to translate.

@aeneasr
Copy link
Member

aeneasr commented Aug 6, 2021

I see - I misunderstood that then!

@vivshankar
Copy link
Contributor Author

vivshankar commented Aug 23, 2021

@aeneasr Apologies for the delay in responding to your previous post about the approach.

New interface that allows for the consumer to implement GetMessage however they choose. There is a default fallback. I did look at go-i18n and the template package, but I feel this is overkill and also there can be a performance hit.

type MessageCatalog interface {
	GetMessage(ID string, tag language.Tag, v ...interface{}) string
}

I am adding a property to the Fosite struct - MessageCatalog i18n.MessageCatalog.

The proposed message format continues to use the standard printf format. In the case of multiple args, the placeholders should contain indices. For example - %[1]s bought a bit of %[2]s can take in two arguments, as it does currently (say, "Betty" and "butter"). https://play.golang.org/p/1H1Uy_5ed-j illustrates the approach. This caters for the shuffling of the placeholders on translation in different locales.

With regards to the tedious changes to update all existing messages. At this point, there are two options:

  1. Change the caller to use the MessageCatalog: For example, modify NewAccessRequest to call f.MessageCatalog.GetMessage before passing the return value to WithHintf.
  2. Add a WithCatalog to RFC6749Error and make changes in functions within the struct: Potentially more error prone for future contributions but "backward compatible". I am leaning towards this approach.

In both cases, all existing hint strings need to be identified by a specific message ID and that message ID should be passed into WithHintf and WithHint in place of the actual string. The implementation of WithHint can fall back to using the parameter value itself if the message ID does not match.

Please let me know if you see any issues with my proposal. I am going to try to submit a PR in the next couple days.

vivshankar added a commit to vivshankar/fosite that referenced this issue Aug 30, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Aug 30, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Aug 30, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Aug 30, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Aug 30, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Sep 29, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Sep 29, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Oct 16, 2021
vivshankar added a commit to vivshankar/fosite that referenced this issue Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants