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

exp/services/recoverysigner: validate request parameters on POST/PUT endpoints #2388

Closed
leighmcculloch opened this issue Mar 13, 2020 · 2 comments

Comments

@leighmcculloch
Copy link
Member

leighmcculloch commented Mar 13, 2020

The recoverysigner has a couple endpoints that accept fields for storing alongside a registered account but does very little validation on the contents of those fields.

Validation should be implemented for the following endpoints:

Some ideas for what type of validation should be added:

  • Fields that are not listed as optional should always be provided in the request with a non-empty value. e.g. identities[].role, identities[].auth_methods[].value.
  • Fields that contain specific types of data should have their format validated. e.g. identities[].auth_methods[].value should have its contents validated as either a valid address, phone number, or email based on the adjacent type field.
  • Arrays that we'd normally expect to contain at least one item should validate for that. e.g. identities and identities[].auth_methods.
  • Limit the number of identities that can be added to something reasonable.
  • Limit the number of auth methods that can be added to something reasonable.
@leighmcculloch
Copy link
Member Author

#2390 adds validations of identities, role, auth methods and auth method type. Still need to add validation of auth method values to ensure the stellar address, phone number, or email address provided in the field are valid for each.

leighmcculloch added a commit that referenced this issue Mar 17, 2020
…t role, identities and auth methods (#2390)

### What

Add some validations to the account post endpoint request body:
- Require at least one identity.
- Require a value for the `role` field.
- Require at least one authentication method for each identity.

The change also updates the authentication method type validation that already exists to use the same format.

### Why

There's no validation on most fields. It would be good to make sure that a client is at least passing the fields required for some success.

For #2388

### Known limitations

This doesn't do all the validation we could do. It doesn't validate the more complex fields like the authentication method values, but it does lay the ground work for where that validation can live.
@leighmcculloch leighmcculloch self-assigned this Apr 1, 2020
@leighmcculloch leighmcculloch removed their assignment Oct 27, 2020
@mollykarcher
Copy link
Contributor

Closing all recoverysigner issues

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

No branches or pull requests

3 participants