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

SEP-12: default to memo type of ID #683

Open
yuriescl opened this issue Mar 5, 2023 · 2 comments
Open

SEP-12: default to memo type of ID #683

yuriescl opened this issue Mar 5, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@yuriescl
Copy link
Contributor

yuriescl commented Mar 5, 2023

This might be a bug, or maybe it's intended behavior.
When calling the SEP-12 PUT /customer endpoint, I noticed that if I use a SEP-10 token without a custodial memo to authenticate the request, it let's me use memo as one of the fields in the payload. Shouldn't the authenticated SEP-10 account/memo be required to match the PUT /customer account/memo?

How to reproduce:

  1. Generate a non-custodial SEP-10 token, that is, without a memo attached
  2. Use that SEP-10 token to call SEP-12 PUT /customer, and put memo=1 in the payload fields
  3. Polaris will accept the request and let me edit any customer by putting any memo= value in the payload
  4. Is this intended behavior?

Also, apparently I also have to set memo_type=id in the payload, otherwise Polaris will raise an error saying the memo does not match memo_type. memo_type should default to id in SEP-12 calls, right?

How to reproduce:

  1. Call SEP-12 PUT /customer with a memo=1 in the payload, but without memo_type
  2. Polaris will raise an error saying the memo doesn't match the memo_type
  3. Is this intended behavior?
@yuriescl yuriescl added the bug Something isn't working label Mar 5, 2023
@JakeUrban
Copy link
Contributor

When calling the SEP-12 PUT /customer endpoint, I noticed that if I use a SEP-10 token without a custodial memo to authenticate the request, it let's me use memo as one of the fields in the payload.

This is the intended behavior, because SEP-31 senders don't include a memo in their SEP-10 challenge, since they're authenticating as themselves instead of as one of their users. Instead they have to specify the memos in their PUT /customer requests to identify their sending & receiving customers.

The downside is that this makes it possible for a SEP-6 wallet to omit their customer's memo in SEP-10. In reality though, this has minimal impact since anchors and custodial / omnibus wallets almost always have agreements and the wallet's public key is whitelisted on the anchor's backend.

Also, apparently I also have to set memo_type=id in the payload, otherwise Polaris will raise an error saying the memo does not match memo_type. memo_type should default to id in SEP-12 calls, right?

This sounds like a bug, thanks for finding.

@yuriescl
Copy link
Contributor Author

yuriescl commented Mar 6, 2023

This is the intended behavior

Got it, makes sense

This sounds like a bug, thanks for finding

Sure

@JakeUrban JakeUrban self-assigned this Mar 6, 2023
@JakeUrban JakeUrban changed the title SEP-12 PUT /customer allowing memo without a SEP-10 token memo SEP-12: default to memo type of ID Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants