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

Make compatible with latest SEP-10 spec version #1156

Open
3 tasks
andywer opened this issue Oct 28, 2020 · 3 comments
Open
3 tasks

Make compatible with latest SEP-10 spec version #1156

andywer opened this issue Oct 28, 2020 · 3 comments

Comments

@andywer
Copy link
Contributor

andywer commented Oct 28, 2020

SEP-10 has just recently been updated and so have the SEP-10-related helper functions in the stellar-sdk: see here.

The main gotcha is that we're using our own custom package for SEP-10.

To Do

  • Check whether we still need our own package
  • If so, update it
  • If not, deprecate it and use the stellar-sdk's functions
@andywer andywer added this to To do in Current stories 👩‍💻 via automation Oct 28, 2020
@ebma
Copy link
Member

ebma commented Nov 2, 2020

I had a look at it and as far as i can tell the only two things that are useful here are buildChallengeTx and readChallengeTx.
buildChallengeTx() can be used to replace the fetchWebAuthChallenge() of satoshipay/stellar-sep-10 without any additional work but readChallengeTx() cannot.
The problem is that there is no counterpart for the fetchWebAuthData() function of satoshipay/stellar-sep-10 in the stellar-sdk.

So my suggestion would be to replace the fetchWebAuthChallenge() function by the stellar-sdk counterpart and leave everything else as is.

@andywer
Copy link
Contributor Author

andywer commented Nov 2, 2020

@ebma One important objection: You cannot replace fetchWebAuthChallenge() with buildChallengeTx() as the latter will create a random new challenge. Each challenge tx contains a random nonce and the client of course needs to sign the challenge that the server wants them to sign.

@ebma
Copy link
Member

ebma commented Nov 2, 2020

I see. Seems like what we could do is replace the assertChallengeOK() helper function with readChallengeTx() since they both validate the challenge transaction (see source code of readChallengeTx here)

But that's probably just a tiny improvement. Do you think I should change it @andywer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants