Postgraphile: Question: Input validation

[andrioid]

I've been playing around a bit with Postgraphql (together with Vue.js) and I'm impressed.

What is a good way to handle input validation. As I understand it, I can with the express middle-ware use Postgraphql as a start-backend for a frontend project.

But how do I make sure some input doesn't compromise the site? Such as XSS or SQL injections. It's very tempting to just let the frontend mutate (pending auth) through GraphQL, but is it safe?

[calebmer]

Yep, PostGraphQL can do input validation in the same way it does most of its best features, through PostgreSQL :blush:

For instance, in the forum example we have this table:

create table forum_example_utils.person_account (
  person_id        int not null primary key,
  email            varchar not null unique check (email ~* '^.+@.+\..+$'),
  pass_hash        char(60) not null
);

Note how the email field has a CHECK constraint on it (in this case a regular expression). To do more complex validations and/or XSS and SQL sanitization look into INSTEAD OF Triggers in PostgreSQL. With that you can alter values before you actually insert them. Although in the specific case of XSS and SQL injection, XSS sanitization can be done equally effectively on the client and PostGraphQL always uses placeholders for input values :+1:

[prevostc]

Check validation does work well but it would be great if we had a way to add a custom error message or error code.

In the email example above, I cannot find a simple way to say to the user that his email is not in a valid format.

Maybe we could use comments with something like:

create table forum_example_utils.person_account (
  person_id        int not null primary key,
  email            varchar not null 
                   constraint email_unique unique
                   constraint email_format check (email ~* '^.+@.+\..+$'),
  pass_hash        char(60) not null
);
comment on constraint email_unique 
  on forum_example_utils.person_account is 'Email already exists';
comment on constraint email_format 
  on forum_example_utils.person_account is 'Email format invalid';

This is a concise way to add user-readable messages and also match postgresql's comment purpose.

[calebmer]

I generally advise not to do error validation messages server side, it adds a lot of complexity when it’s most of the time easier and better for UX to just duplicate logic and do validation on the client. But that’s a reasonable proposal @prevostc, if anyone wants to submit a PR implementing that I’d merge it. I’d just also advise the comments be a description of what the constraint requires (from a stylistic perspective only, people could do what they want :blush:).

comment on constraint email_unique 
  on forum_example_utils.person_account is 'Emails must be unique.';
comment on constraint email_format 
  on forum_example_utils.person_account is 'Email must be in the correct format.';

[prevostc]

Having it thought twice, you are right about server side generated messages.
PostGraphQL shouldn't force users to use server-side generated messages. But should give them the opportunity to generate the right error message on the client side too.

Ex (given a person with ID already exists):

mutation {
  insertPersonAccount (input: {
    personId: 1,
    email: "[email protected]",
    passHash: "hey"
  }) { personAccount { id } }
}
---------->
{
  "data": {
    "insertPersonAccount": null
  },
  "errors": [
    {
      "message": "duplicate key value violates unique constraint \"person_account_pkey\"",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "stack": "blabla"
    }
  ]
}

mutation {
  insertPersonAccount (input: {
    personId: 1,
    email: "INVALIDEMAIL",
    passHash: "hey"
  }) {
    personAccount {
        id
    }
  }
}
---------->
{
  "data": {
    "insertPersonAccount": null
  },
  "errors": [
    {
      "message": "new row for relation \"person_account\" violates check constraint \"person_account_email_check\"",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "stack": "blabla"
    }
  ]
}

I don't know about any standard for business error output in GraphQL or Relay so my idea is this: add a new field that is unique to each constraint to let client code handle error messages.

• a new key field with the constraint name
• a new comment field if this is of any help

This works well with application errors like constraints and exceptions from procedures.

If this seems like a good solution then I will be happy to work on it :)

[calebmer]

Yep, this does seem good. Why don't we also expose the error code when a constraint does not apply? https://www.postgresql.org/docs/8.1/static/errcodes-appendix.html

[calebmer]

I’m going to close this for now as there is no real clear confusion. PostGraphQL can definitely do a better job with errors, but we’re going to need a more focused discussion. Also, error handling in GraphQL in general is still a field with many unanswered questions.