schwarz / ueberauth_bnet

Ueberauth Strategy for Battle.net Login
MIT License
4 stars 6 forks source link

Allow to set region via params #2

Closed tomasz-tomczyk closed 3 years ago

tomasz-tomczyk commented 3 years ago

I had an issue where I need to support multiple regions in an app and I couldn't get dynamic provider configuration implemented in ueberauth quite working.

This PR adds two features:

When region is added to the request URL, it will use it when requesting the code from the BNet API.

When the code is returned, infer the region based on the first two characters. Wasn't too sure about it, as it overrides the configuration but I couldn't think of a nice way of passing along the region from the request.

What do you think?

schwarz commented 3 years ago

If possible we should probably prefer dynamic provider configuration going forward, but supporting multiple regions should definitely be a thing.

Did you test the kr and tw regions on Blizzard's API? According to the docs they seem to have been replaced with a combined apac for OAuth:

{region} is one of the following: us, eu, or apac. APAC is short for Asia-Pacific and replaces the previously-used kr and tw regions.

We adjust the URL based on region in the client here: lib/ueberauth/strategy/bnet/oauth.ex#L52, kr or tw should break.

tomasz-tomczyk commented 3 years ago

If possible we should probably prefer dynamic provider configuration going forward, but supporting multiple regions should definitely be a thing.

Yeah I couldn't get it to work - do you have an example of how to do this?

Did you test the kr and tw regions on Blizzard's API? According to the docs they seem to have been replaced with a combined apac for OAuth

Oops, you're right! I'll amend 👍

tomasz-tomczyk commented 3 years ago

@schwarz changed the KR/TW regions to APAC - seems like the token is prefixed with KR there