search

satoshipay / solar

🌞 Stellar wallet. Secure and user-friendly.
https://solarwallet.io
MIT License
192 stars 57 forks source link

StellarGuard support #595

Open andywer opened 5 years ago

andywer commented 5 years ago

On transaction submission (see TransactionSender component):

See https://stellarguard.me/faq#wallet-developers.

Out of scope

Nice to have

Update (May 29)

Need to provide a way to paste, sign and submit a transaction XDR 😕

andywer commented 5 years ago

@Marcel-Ebert Please try to add the new Stellar Guard icon to Solar and show it next to the account name (instead of the usual multi-sig icon) for every Stellar Guard - enabled account.

PS: Set the color property in the SVG to currentColor, so we show it in the text color.

StellarGuard_logomark_blue (1).svg

pselden commented 5 years ago

Regarding

Need to provide a way to paste, sign and submit a transaction XDR 😕

Is that in relation to StellarGuard setup/removal transactions? Or something else? If related to StellarGuard, do you support SEP-0007 URIs or QR? If so, I can add one in to StellarGuard.

andywer commented 5 years ago

Hey @pselden! Nice to see you around here :)

Is that in relation to StellarGuard setup/removal transactions?

Yes, it is. We do support SEP-7 requests, but only use them for our multi-signature coordinator so far. We implemented most of it, but not all of it. You have published a SEP-7 npm package as I saw recently, right?

Using SEP-7 for that purpose would be better and in the mid-term we are definitely heading for that approach. It comes with a bit of overhead to make sure we don't make it too easy for scammers to tell the user "just scan this QR code - what could possibly go wrong", though.

I think a quick-and-dirty way to import a SEP-7 request via QR code scan might indeed make a good first iteration. Just got to sleep a night about how to prevent users from falling prey to malicious actors for now.

We already have a pretty neat solution for all this in the drawer, but it's too much effort to implement that now. Will sleep a night about it. Maybe we just hard-code the StellarGuard public key you would use to sign the SEP-7 requests for now and reject any other SEP-7 reqs in the first iteration.

Also pinging @erasmus. Interesting topic, sensitive subject, very relevant to comply with the stellar.org guidelines.

pselden commented 5 years ago

Yes, it's here: https://github.com/stellarguard/stellar-uri - it relies heavily URL and URLSearchParams to do the parsing.

One issue that was brought up when I was talking to someone was that URLSearchParams encodes spaces as + instead of %20. That's in contrast to encode/decodeURIComponent which does %20. If you have a url like the demo one here: https://stellarguard.github.io/stellar-uri/demo/

web+stellar:tx?xdr=AAAAADPMT6JWh08TPGnc5nd6eUtw0CfJA4kQjkHZzGEQqGWHAAAAZAAGXSAAAAABAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAAM8xPolaHTxM8adzmd3p5S3DQJ8kDiRCOQdnMYRCoZYcAAAAAAAAAAACYloAAAAAAAAAAAA%3D%3D&msg=order+number+123&callback=url%3Ahttps%3A%2F%2Fexample.com%2Fstellar&origin_domain=test.stellarguard.me&signature=TwoRggPieF6UorVeLHSYZhRRKv8mMwezVUiirms%2F8N6oe8EZOCYKSsNWAn2o1rVb8jhEVte%2FEFZcRkzyXEZdBw%3D%3D

Does it parse correctly for you? I was considering doing some overrides to make it use %20 instead of + based on feedback about compatibility with other parsers.

Maybe we just hard-code the StellarGuard public key you would use to sign the SEP-7 requests for now and reject any other SEP-7 reqs in the first iteration.

You could check the origin_domain and see whether it's a stellarguard domain AND whether the signature is valid: https://github.com/stellarguard/stellar-uri/blob/master/src/lib/stellar-uri.ts#L206

andywer commented 5 years ago

Keeping the issue open until we have added a way to set up StellarGuard for an account.

pselden commented 5 years ago

How can I help with this? I will work on adding a SEP7 QR code and we can go from there? The space encoding is probably not an issue either way if I don't send the msg parameter.

andywer commented 5 years ago

Yeah, sorry Paul, I had quite little time for this this week. Marcel has come pretty far with the protocol handler, though!

We will go straight for the SEP-7 link solution and it should be ready within a few days. The QR code might work as well, but a button with a SEP-7 link would be much more convenient, esp. since the user would otherwise need two devices - one to display the QR code and one to scan it.

PS: Next week I will be able to spend more time on finishing this feature :)

andywer commented 5 years ago

The space issue is still important: If it’s a testnet request, the network passphrase needs to be passed which contains spaces.

But I think @Marcel-Ebert got it to work with the current encoding. AFAIK he also had to replace the encoded space, though.

gpg90 commented 5 years ago

@andywer Hey, maybe you could add Lobstr Vault support while you are at it?

Integration is the same as Stellarguard, just different signer key.

https://vault.lobstr.co/

Readme has the integration instructions: https://github.com/Lobstrco/Vault-Android#developers-integrate-lobstr-vault-with-your-service

andywer commented 5 years ago

Good point. Wasn't aware that their 2FA service works the same way.

gpg90 commented 5 years ago

@andywer Cool - please reach out if you’ll run into any issues! Added you on Keybase

andywer commented 5 years ago

@pselden We can simplify the whole setup flow a bit with SEP-7 URIs: Instead of copy-pasting the public key into the StellarGuard page and then creating a transaction for that account we could create a SEP-7 tx request with nulled source account and sequence number.

The user will be able to select the account in the wallet, so that the wallet will fill-in the source account and the sequence number. Thus we can effectively simplify the setup by merging two steps ("paste public key" + "copy transaction") into one ("click on setup link").

What do you think? Is that something that you can easily change in the StellarGuard webapp? Maybe as an alternative flow.

pselden commented 5 years ago

Yes I think that's reasonable. The only reason I didn't go with something like that already is that most wallets do not support SEP7 at all (this is changing now).

I have a backlog item to completely redo the setup flow and this is a good inclusion for that.

I think what I will end up doing is ask the user what wallet they use and then offer specific instructions based on that wallet. Right now the most likely thing that will happen if I give them a SEP7 link without more prompting/flow is that it will fail completely since I don't think it's possible to support it in a web wallet (let me know if you've seen web wallets that do) and most users don't know what it is -- so I have to be careful about how I present it.

The other issue would be detecting when a user completes the setup. Right now I can check it by looking at the public key and querying for signers on that account. However it doesn't work out quite that smooth if I don't know what public key they are using in advance. I'll think of some solutions for this.

andywer commented 5 years ago

@pselden Sounds good! I was daydreaming about a very similar flow... 😄

The other issue would be detecting when a user completes the setup

That's easy, I think: Set a callback URL on the SEP-7 request and we shall submit the transaction back to you instead of straight to horizon. Put an identifier in the URL and you can match it to the right user account. This way you know when the user has finished the manual step and you know the account ID (= tx source account).

andywer commented 5 years ago

@pselden You let us know once you have got a PoC for the sep-7 URI setup journey? We have a first implementation ready on our end :)

pselden commented 5 years ago

I'm working on it this week. Should have something to try out early next week.

andywer commented 5 years ago

Update: Lobstr icon. See https://static.lobstr.co/media/b26ec021-c6b8-4107-9ea3-84eff7ea8f42.svg.

pselden commented 5 years ago

Sorry I dropped the ball on this - I ran into an issue because the most recent SEP7 specification does not include any ability to do source account replacements: https://github.com/stellar/stellar-protocol/commit/d4c556634d4bf41b44e5f30abc1ecd105b445d47#diff-afc043c02ee1eef064c106f886302a9b

So it appears that I still have to ask the user for their public key.

andywer commented 5 years ago

We even implemented the sequence number replacement already... I can talk to @nikhilsaraf about this, but I think he is afk for two weeks right now.

pselden commented 5 years ago

I made a post asking for clarifications on the Stellar mailing list https://groups.google.com/forum/#!msg/stellar-dev/cEx42wvutQ8/xFRSP020DQAJ

It's a bit confusing to me now since the sequence number replacement stays, but the source account replacement is removed.

I don't think there's an easy way to not specify the source account when building the transaction... this may be just an SDK limitation though (although a fundamental one in this case).

andywer commented 5 years ago

I think we created an account instance manually (new Account()) and passed GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF as the account ID - which is the Stellar public key representation of an all-zero-bytes buffer. Then we instantiated a TransactionBuilder with that.

pselden commented 5 years ago

Yeah that was the old way of doing it (when it was included in the spec). It appears the spec currently allows a completely missing source account from the transaction "If the source account is set and the sequence number is 0 then the URI handler should replace the sequence number before signing."

That if implies that it's possible for it not to be set -- but I don't see how that's actually possible to do using the js sdk at least.

andywer commented 5 years ago

@pselden How about a quick work-around for the time being?

We might want to link to the StellarGuard website for 2FA setup anyway. So what if we just set a query parameter containing the account's public key when linking to you?

This way you would already know the account when you create the setup transaction.

andywer commented 5 years ago

@pselden I talked to Nikhil from SDF yesterday and he says if we really need that functionality now, we should just stick to the current implementation (according to the SEP-7 spec before the change), as a new solution won’t make it into the SEP-7 spec before the end of the year.

pselden commented 5 years ago

@andywer Yeah I think that is the best path forward. BTW Lobstr is handling it slightly different in that they completely ignore the source account and just replace it with their user's. I'll go with GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHFfor now.

gpg90 commented 5 years ago

If we could get this "zero" account back to SEP-7, we would be happy to change how we handle this.

Looks like almost everyone, who touches SEP-7 almost immediately needs this feature (replacing source account), so I believe this "zero" account concept should be added back - it makes a lot of sense and is easy to implement. @nikhilsaraf

DMT012 commented 1 week ago

GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF

This is my account and I have proof of deposits. Is it possible two of us were using the same accounts?

Anyone know how I can reopen this #205 and #595 to try to fix. I have been trying to resolve this for two years.

Add [Steve Huffman] #personal GAAAAA…AAAWHF #205 Closed stellar-expert-deploy opened this issue on Oct 27, 2022 · 7 comments Closed Add [Steve Huffman] #personal GAAAAA…AAAWHF

205

stellar-expert-deploy opened this issue on Oct 27, 2022 · 7 comments Comments @stellar-expert-deploy

stellar-expert-deploy commented on Oct 27, 2022 Address: GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF Tags: personal Title: Steve Huffman Requested by @shuff18

Over 35000 XLM (Stellar) tokens sent to my ledger live account. I cannot transfer these tokens from this account! I have password phrase for my Nano X hardware and have tried many times to RE-set and still not able to transfer, sell or trade my tokens under this account. Ledger support says must be a bug or burn token address-how can this be? Please investigate and inform. Thank you, Steve Huffman @CryptoHobbyist

CryptoHobbyist commented on Oct 27, 2022 @orbitlens @CryptoHobbyist

CryptoHobbyist commented on Nov 12, 2022 @shuff18 did you get ledger to help you out? @shuff18

shuff18 commented on Nov 12, 2022 via email No! Complete runaround sending transaction records, etc….Somehow the stellar account with ledger live was a test or “burn” address from the according to Michael who was the support rep for ledger assisting me.I deposited several thousand$ worth of stellar tokens in conjunction with a few thousand XLM tokens deposited into that account from stellar as payments of some sort.I always had my original pass phrase for my ledger nano and tried everything I could to sell or withdrawal my 35000 XLM tokens with no positive results.After everything I closed the stellar account out of frustration and total disappointment!

Sent from Yahoo Mail for iPhone

On Friday, November 11, 2022, 12:13 PM, CryptoHobbyist @.***> wrote:

@shuff18 did you get ledger to help you out?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***> @shuff18

shuff18 commented on Nov 12, 2022 via email Can you investigate/help?I deleted that account in ledger live, although I still have all transactions in my ledger Nanox going back to 2019!Please let me know.Thank you,Steve Huffman

Sent from Yahoo Mail for iPhone

On Friday, November 11, 2022, 12:13 PM, CryptoHobbyist @.***> wrote:

@shuff18 did you get ledger to help you out?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***> @CryptoHobbyist

CryptoHobbyist commented on Nov 14, 2022 @shuff18 Stellar.expert is a Stellar Blockchain explorer to help people have a tool to look at transactions other than developing your own coding to view the open transactions it has no connection to support from the Stellar Development Foundation.

As you seek help be very careful to NEVER GIVE ANYONE your private keys, passwords, or recover phrases. NEVER USE A THIRD PARTY WEBSITE to input this kind of information. Since you have commented her be very careful that someone posing as "Tech Support" contacts you and asks for these things. @CryptoHobbyist

CryptoHobbyist commented on Nov 14, 2022 • edited @shuff18 Tech Support at ledger is your best bet or message boards who support ledger can have some very helpful people. Just be very cautious as state above. Always post questions in open forms scammers will try to DM people.

Stellar.expert can help you look up transactions on the Stellar Blockchain Ledger side which is not dangerous to share with tech support or people who might be able to help. JUST NEVER GIVE KEYS @shuff18

shuff18 commented on Nov 14, 2022 via email Understood!Thank you

Sent from Yahoo Mail for iPhone

On Sunday, November 13, 2022, 1:49 PM, CryptoHobbyist @.***> wrote:

@shuff18 Tech Support are ledger is your best bet or message boards who support ledger can have some very helpful people. Just be very cautious as state above. Always post questions in open forms scammers will try to DM people.

Stellar.expert can help you look up transactions on the Stellar Blockchain Ledger side which is not dangerous to share with tech support or people who might be able to help. JUST NEVER GIVE KEYS

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>