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

API: Editing and Deleting Accounts #196

Closed
emschwartz opened this issue Aug 7, 2019 · 12 comments
Closed

API: Editing and Deleting Accounts #196

emschwartz opened this issue Aug 7, 2019 · 12 comments
Assignees
Labels

Comments

@emschwartz
Copy link
Member

Originally part of #128

@bstrie
Copy link
Contributor

bstrie commented Aug 8, 2019

@emschwartz Currently when an account is created, we check to see if the account has an associated SE and if so send a request to the SE in order to register the account there.

  1. For the purpose of deletion, do SEs currently support removing an account? Alternatively is there a reason not to do so?

  2. If an account is modified such that the SE url changes, what action should be taken?

@emschwartz
Copy link
Member Author

Good questions.

  1. I don't think so, but it might make sense to add that.
  2. It should probably send the registration call to the new SE

@gakonst thoughts?

@bstrie
Copy link
Contributor

bstrie commented Aug 8, 2019

Wrt 2, should it first also delete the account on the prior SE, or is there some kind of notion of inter-SE account transfer that we want to support?

@emschwartz
Copy link
Member Author

Apparently the spec includes an endpoint to delete accounts, though I'm not sure if that's currently supported in the implementation. Maybe it should try to call that endpoint but just ignore the error if that fails

@bstrie
Copy link
Contributor

bstrie commented Aug 12, 2019

Currently in #186 we're changing from using autoincrementing numeric IDs to represent users to user-provided strings to represent users. Regarding account deletion, I'm curious whether we want to make the semantics such that an account that is deleted acts as though it is deleted for the purpose of accessing any info about the account (i.e. behavior is identical to having attempted to access an account with an ID that has never existed), but that also acts as though it does exist for the purpose of creating a new account. In other words, that we support account deletion, but that we don't support the recycling of usernames, and that once an account is made with a username, no new account can ever be made with that username. Regarding implementation details, we could achieve this either by leaving behind a tombstone whenever an account is deleted or by separately storing all usernames that have ever been created and checking against that list upon account creation.

@emschwartz
Copy link
Member Author

I think we should not allow username reuse right now.

For context, I looked at the following posts from some different industries:

Allowing username reuse is a bit of a footgun, and that is all the more serious when you may be asking people to send you money at an address that's a combination of your provider's domain and your username.

We can always add the ability to reuse usernames later if desired (it could also be an optional feature on the node).

I like the option of maintaining a list in the store of deleted_usernames or something like that and disallowing accounts to be created if their username is in that list (that way, we don't need to save the full account details).

@gakonst
Copy link
Member

gakonst commented Aug 15, 2019

We should also allow users to change their default SPSP account, the method exists but is not exposed on the SPSP API implementation, ideally we'd want to be in the routes/settings.rs file

@bstrie
Copy link
Contributor

bstrie commented Aug 21, 2019

The basic functionality is in, but there are still some touch-ups before I'd consider this done:

  1. Reject attempts to create accounts with IDs that have been deleted (waiting for Replace numeric account IDs and configured ILP addresses with usernames and dynamically generated addresses #186)

  2. Remedy the race conditions in the current update and insert logic (waiting for Replace numeric account IDs and configured ILP addresses with usernames and dynamically generated addresses #186)

  3. Properly notify the SE when the account's SE URL changes.

@gakonst
Copy link
Member

gakonst commented Aug 27, 2019

@bstrie I believe the blockers for this from #186 have been merged in #233, is there anything else blocking you?

@bstrie
Copy link
Contributor

bstrie commented Aug 28, 2019

If there are no more username-related changes to be made then I'll go ahead and work on polishing this off. :)

@tarunreddy6
Copy link

tarunreddy6 commented Oct 3, 2019

@bstrie I believe the blockers for this from #186 have been merged in #233, is there anything else blocking you?

but how can we access an account.? while blocking..)

@gakonst
Copy link
Member

gakonst commented Dec 14, 2019

Properly notify the SE when the account's SE URL changes.

This was the only missing feature, and this is implemented in https://github.com/interledger-rs/interledger-rs/blob/master/crates/interledger-api/src/routes/accounts.rs#L248

@gakonst gakonst closed this as completed Dec 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants