Add validation for records create/edit forms

[Myrfion]

Changes

• add more in-depth validation for forms, which includes validation of record value
• extraction of validation schemas and reusing them
• showing errors for all fields in record form

closes #277

[Genne23v]

I think the issue number is not right @Myrfion

[humphd]

This is really slick, nice work!

Testing locally, here is what I notice:

An invalid name and an invalid A record (e.g., not an IP) are caught correctly:

However, a CNAME shouldn't be an IP address (it has to be a domain name):

I'm not sure where this bug is coming from, might be in the DNS verification code?

[Myrfion]

@humphd because I wanted to reuse the same scheme for both record creation and editing, by just adding an extra field to edit scheme (id). But if I want to validate a subdomain using existing validation functions I also need to pass the username in the validation function and I have access to the user object only in the action function.

[Myrfion]

@humphd I think the problem of validation comes from the validation function which was created before and it looks like this:

export const isValueValid = (type: DnsRecordType, value: string) => {
  if (type === 'A') {
    return isIP(value, 4);
  }

  if (type === 'AAAA') {
    return isIP(value, 6);
  }

  // CNAME can be any non-empty string. Let AWS validate it.
  return value.length >= 1;
};

I believe this is not the way the value really should be validated

[Myrfion]

The ChatGPT is actually pretty good to write this kinda things that don't require a lot of context, here is what it did, looks good to me but need to write some tests too:

const isValueValid = (type: DnsRecordType, value: string) => {
  switch (type) {
    case 'A':
      // IPv4 address (e.g. "192.168.0.1")
      return /^(\d{1,3}\.){3}\d{1,3}$/.test(value);
    case 'AAAA':
      // IPv6 address (e.g. "2001:0db8:85a3:0000:0000:8a2e:0370:7334")
      return /^([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}$/.test(value);
    case 'CNAME':
      // domain name (e.g. "www.example.com")
      return /^([a-zA-Z0-9_-]+\.)*[a-zA-Z0-9_-]+\.[a-zA-Z]{2,}$/.test(value);
    case 'TXT':
      // free-form text data (e.g. "v=spf1 a mx ~all")
      return /^"[^"]*"$/.test(value);
  }
};

[humphd]

Let's try to fix our validation and do it all in one place vs. duplicating it. I'm hesitant to write our own regex for these, as there are so many corner cases with domains. Those functions should work, and if they don't, we should replace them with ones that do.

cc @Genne23v

[Myrfion]

Let's try to fix our validation and do it all in one place vs. duplicating it. I'm hesitant to write our own regex for these, as there are so many corner cases with domains. Those functions should work, and if they don't, we should replace them with ones that do.

cc @Genne23v

I wasn't actually thinking about duplicating it, but just replacing the existing one. If you wanna fix the current function I think we need at least to validate the CNAME value it in it somehow, because right now it is not being validated in it

[humphd]

OK, let's take this now, and please file a follow-up to fix our validation (@Genne23v) and add E2E tests (@Eakam1007) for all these validation cases.

[humphd]

Please file an issue to fix our validation on CNAME, and add e2e tests, and this is good.

[humphd]

Please file an issue to fix our validation on CNAME, and add e2e tests, and this is good.

@Myrfion can you do this too please?

[Myrfion]

Please file an issue to fix our validation on CNAME, and add e2e tests, and this is good.

@Myrfion can you do this too please?

Done #435 #436

[humphd]

Awesome, thank you!