Use base 64 encoding instead of uri encoding of state param for yahoo

[arushi364]

Yahoo expects base 64 encoding of state param and somehow fails at the consent page if URI encoding is used. This PR intends to change the encoding for Yahoo network. It retains the original behavior for other networks.

For all clients using hellojs for yahoo (not sure if there are any as it seemed busted for yahoo), one will have to base 64 decode the state param before it can be parsed to extract the state param attributes.

[arushi364]

@MrSwitch could you please take a look at the changes ?

[MrSwitch]

Hi @arushi364

That logic should be defined in the yahoo module. As such i can't merge this as is.

[arushi364]

Hi @arushi364

That logic should be defined in the yahoo module. As such i can't merge this as is.

@MrSwitch Thanks for replying back. I tried changing the encoding in the login function. But, once the state param is base 64 encoded in the yahoo login function, the code in hello.js tries to again uri encode it after stringifying the state. The base 64 encoded state does not change when uri encoded, but the problem here is JSON.stringify results in adding quotes in the state param which is eventually converted to %22, which makes yahoo unhappy.

state param in yahoo login function: base64_encoded_string
hello.js transitions it to:
JSON.stringify: "base64_encoded_string"
encodeURIComponent : %22base64_encoded_string%22

This is the code which does the transition and is executed after the provider.login function is executed.

// Convert state to a string
p.qs.state = encodeURIComponent(JSON.stringify(p.qs.state));
line 378 https://github.com/MrSwitch/hello.js/blob/master/src/hello.js#:~:text=//%20Convert%20state%20to,))%3B

Any suggestions on how can I avoid adding quotes here so that no uri encoded characters are added in the state?

[arushi364]

@MrSwitch Wondering if you got a chance to look at my previous comment? Could you please advise with the next step? We really need to fix this library so that we can consume this for talking to Yahoo.

[MrSwitch]

Can you create a property like provider.oauth.base64State define this to true on just the Yahoo module?

Also to make it easier to merge:

1. Write tests where appropriate
2. Document the new property

Thankyou 👍

[arushi364]

Thanks @MrSwitch. Just wanted to confirm, to create a new property provider.oauth.base64State, I would need to update the HelloJSOAuthDef in this file?
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/hellojs/index.d.ts

[arushi364]

@MrSwitch I have added a new property and updated the test cases. Could you please take a look at the changes?

[arushi364]

@MrSwitch Could you please take a look at the PR?

[arushi364]

Gentle ping! @MrSwitch

[arushi364]

@MrSwitch Could please take a look at the changes? We are waiting on merging this fix so that we can consume the library for adding a yahoo account in our App. I would highly appreciate if you could review the changes once?

[MrSwitch]

Looks fantastic

Only change is that individual module handling should not belong in the core, or indeed any reference too. So i've removed the yahoo specific reference from the core tests.

[MrSwitch]

@MrSwitch Could please take a look at the changes? We are waiting on merging this fix so that we can consume the library for adding a yahoo account in our App. I would highly appreciate if you could review the changes once?

Looks great, thank you for persevering 🙏

[arushi364]

Thanks @MrSwitch I have addressed the review comments. Could you please take a look?

[MrSwitch]

LGTM 🎉

[github-actions]

🎉 This PR is included in version 1.20.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ioliva]

@MrSwitch , @arushi364 I have been having problems using this new base64_state property.

I have found that, afterwards, in the responseHandler, the "Could not decode state" error is raised, since the base64 encoded state should be decoded again.

So, I request reopening this issue in order to complete the fix (I will pass PR).