ecency / hivesigner-sdk

Hivesigner SDK
https://hivesigner.com
MIT License
2 stars 1 forks source link

TypeError: Expected String when attempting to authorize new user #10

Closed shawnlauzon closed 3 years ago

shawnlauzon commented 3 years ago

I am attempting to debug a problem which is happening on leofinance.io. When attempting to use hivesigner for a new user, I successfully signed in with the user, and then when attempting to authorize a new user:

The leofinance requires your posting authority in order for you to be able to interact with it. By clicking "Continue" you are allowing posting access. This can be withdrawn by you at any time by clicking HERE.

I receive the following error: image

Here is the URL that is displayed at the time:

https://hivesigner.com/authorize/leofinance?client_id=leofinance&redirect_uri=https%3A%2F%2Fleofinance.io%2Fcallback%2Fhivesigner&scope=posting&state=%7B%22lastPath%22%3A%22%2F%22%7D&app=leofinance&signature

Here's the stack in the console:

0c8470f.js:1 Failed to broadcast transaction TypeError: Expected String
    at m (6e9e1a8.js:formatted:59720)
    at Object.decode (6e9e1a8.js:formatted:59782)
    at 4a04011.js:formatted:304
    at M (4a04011.js:formatted:308)

It looks like happening in D.decode():

    function M(e) {
      return new L.PrivateKey(function(e) {
        var t = D.decode(e);
        if (128 !== t[0])
          throw new Error("private key network id mismatch");
        return t.slice(0, -4)
      }(e).slice(1))
    }

Thank you.

feruzm commented 3 years ago

@shawnlauzon is there any reason why not use normal oauth2 url ?

Documentation: https://docs.hivesigner.com/h/guides/get-started/hivesigner-oauth2

Example: https://github.com/ecency/ecency-vision/blob/ffbff8e83bd5c546b259d99650a03d839f98a77a/src/common/helper/hive-signer.ts#L3

appukuttan66 commented 3 years ago

Authorizing a new user is giving the same error ... Got the URL from getLoginURL()

feruzm commented 3 years ago

@appukuttan66 Perhaps leofinance also require decoding of state parameter from URL? Check here: https://gitlab.syncad.com/hive/condenser/-/merge_requests/269

appukuttan66 commented 3 years ago

This issue is not limited to leofinance ... the error was for a new website I made .. I don't think I added a state param https://ecency.com/hive-193084/@unklebonehead/re-sxt6nrzogi4dnmq3ipiebm

feruzm commented 3 years ago

@appukuttan66 I just tried both your app and leofinance, both working. Are we sure person logged in or using active key on Hivesigner? With posting private key you cannot authorize app so that might be causing this issue. I will try to reproduce, so far no luck, working on my end.

appukuttan66 commented 3 years ago

I did try to reproduce the problem a few times using my test acc and main acc but I sadly couldn't find his problem ... Will ask him if he is using his active key ( shouldn't that give him a different error , the one with "active authority is missing" ? )

shawnlauzon commented 3 years ago

Sorry for being so long responding to this. I'm working on a completely different app and hitting the same error. I'm using a different URL, with the normal oauth2 url: https://hivesigner.com/oauth2/authorize?client_id=dapplr&redirect_uri=https%3A%2F%2Fapi.dapplr.in%2Fcallback&scope=posting&response_type=code and hit the same error.

With posting private key you cannot authorize app so that might be causing this issue.

And that was the problem! I cleared my cookies and restarted with using my active key, and it worked!

First, thank you :)

Second, unfortunately this is a problem, because: 1) The UI says posting key is acceptable: image 2) All I asked for was posting scope, so why is active key required? 3) If the user gives the posting key, there's no way to know that wasn't correct, 4) If they guessed they needed the active key, there's no way to add it (eg like KeyChain for Hive) without clearing cookies.

Thanks for the reply, and I hope something can be done. I think it should either: a) Require active or owner, say that on the UI, and fail with a nice error if they give the wrong one b) Accept posting key if the scope passed doesn't necessitate something more (better IMO)

appukuttan66 commented 3 years ago

Only "authorizing" the app requires active key ... this is a one timer ... i usually just authorize an app using an incognito window and login to the app on using posting key on a normal window ... The previous UI had this "Authorization Failed" error which was easier to figure out for the user ...

shawnlauzon commented 3 years ago

Thanks, it makes sense that this is possible, but it's not at all user friendly. If we want to expand Hive usage to the general public, telling people to use an incognito window to sign in the first time will turn off nearly everyone.

And since I'm now doing app development with HIve, it's not even an option.

feruzm commented 3 years ago

We are improving authorization part, if logged in user doesn't have required key, it will prompt for import.

shawnlauzon commented 3 years ago

Great, thank you @feruzm. Also would be stellar not to require the active key just for posting and upvoting.

feruzm commented 3 years ago

@shawnlauzon If you have authorized any app already, you don't need active key for posting/upvoting

shawnlauzon commented 3 years ago

Authorized any app? So you're saying that if I already authorized a different app with my key, then it should only require posting key? That is definitely not my experience: the error I posted above is with a user which has previously authorized an app.

feruzm commented 3 years ago

@shawnlauzon yes, that's the bug we have fixed in staging.hivesigner.com, not pushed into production, yet. Please feel free to test staging.hivesigner.com and let us know if it works as intended, other changes are in progress still.

shawnlauzon commented 3 years ago

Just tried staging.hivesigner.com. First thing is that the original issue is resolved, yay!

However, the UI displayed:

image

Makes it seem like leofinance is requesting active key, which then will make it less likely for people to accept, especially since during oauth, we just requested posting.

shawnlauzon commented 3 years ago

After giving it my active key and reloading the app, I needed to signin again. It again showed the above screen, but then gave this error:

image

feruzm commented 3 years ago

Could you retry please?

feruzm commented 3 years ago

We are about to push update to production, please do test on staging to see if issue resolved. @shawnlauzon

shawnlauzon commented 3 years ago

Ok trying it now ...

shawnlauzon commented 3 years ago

Good news: it works now with my posting key!

Bad news: I wonder if it shouldn't have. I just realized that the scope I passed was vote,comment,comment_options,offline,custom_json,claim_reward_balance. Since I'm sure some of those things require the active key, perhaps it should have said that the active key was required.

Anyway, it's better than it was before (no random error and didn't require active key), so updating production seems like a positive.

I'm happy to close this issue as well since the original issue is solved.

feruzm commented 3 years ago

All scopes you mentioned require posting key/authority. custom_json with active key should be hot signed. Yes, we have pushed updates into production. Thank you for double checking. In future UI related issues could be written into https://github.com/ecency/hivesigner-ui 🙇