1

Is there some way to improve these server-side user fields validations?

<cfif Form.LoginName EQ ""><h1>Login Name is required.</h1></cfif>
<cfif Form.Password EQ ""><h1>Password is required.</h1></cfif>
<cfif Form.Password NEQ Form.PasswordConfirmation><h1>Password confirmation does not match Password.</h1></cfif>
<cfif Form.FirstName EQ ""><h1>First Name is required.</h1></cfif>
<cfif Form.LastName EQ ""><h1>Last Name is required.</h1></cfif>

<cfif Form.LoginName EQ "" OR Form.Password EQ "" OR Form.Password NEQ Form.PasswordConfirmation OR Form.FirstName EQ "" OR Form.LastName EQ "">
    <p>User has not been created</p>
    <p>You can use your browser's back button to keep form fields filled and try again.</p>
    <p><a href="users.cfm">Return to users list</a>.</p>
    <cfabort>
</cfif>
Adam Cameron
  • 29,677
  • 4
  • 37
  • 78
Paul
  • 25,812
  • 38
  • 124
  • 247
  • 3
    Take a look at ValidateThis - http://www.validatethis.org/ - it is an open source library to handle validation, both client side and server side. Very cool, very powerful. – Scott Stroz Aug 04 '13 at 15:31

3 Answers3

6

The way you're coupling your business logic to your display leaves a bit to be desired. You could probably benefit from reading up on MVC and separation of concerns.

From the perspective of your logic, your validation rules seem fine, but you're doing the validation twice, which seems excessive: each element, then all elements. This is in part due to the problem I highlight above.

I'd give some thought to stop thinking procedurally, and think in a more OO fashion, and define the notion of a User.cfc, and have some sort of validation service (see ValidateThis). Or something like that.

Lastly, this is not really the sort of question best asked on Stack Overflow, but would be good for Code Review. There's no one answer for this question, so people will be inclined to suggest closing it for being "primarily opinion-based".

I'm also gonna retag this as just "ColdFusion" rather than "ColdFusion 10", as it really has nothing specifically to do with CF10, it's just a CFML question. You'll get a bigger audience with it marked as just "ColdFusion".

Community
  • 1
  • 1
Adam Cameron
  • 29,677
  • 4
  • 37
  • 78
3

Instead of sharing code with you, I would like to introduce the concepts to you. The first thing you should do is read the OWASP recommendations for Data Validation. In it they suggest that there are four strategies for validating data, and they should be used in the following order. I will post some excerpts here but I strongly recommend you read the entire article.

  1. Accept known good
    This strategy is also known as "whitelist" or "positive" validation. The idea is that you should check that the data is one of a set of tightly constrained known good values. Any data that doesn't match should be rejected.

  2. Reject known bad
    This strategy, also known as "negative" or "blacklist" validation is a weak alternative to positive validation. Essentially, if you don't expect to see characters such as %3f or JavaScript or similar, reject strings containing them. This is a dangerous strategy, because the set of possible bad data is potentially infinite. Adopting this strategy means that you will have to maintain the list of "known bad" characters and patterns forever, and you will by definition have incomplete protection.

  3. Sanitize
    Rather than accept or reject input, another option is to change the user input into an acceptable format

  4. No validation
    This is inherently unsafe and strongly discouraged. The business must sign off each and every example of no validation as the lack of validation usually leads to direct obviation of application, host and network security controls.

The article goes on to discuss each of these in greater detail and much more.

Miguel-F
  • 13,450
  • 6
  • 38
  • 63
2

This is another way. You can decide for yourself whether or not it is better.

Step 1 - create an error message variable.

<cfset ErrorMessage = "">

Step 2 - Do your checks. If you see something you don't like, append text to your variable.

<cfif len(trim(form.LoginName)) gt 0>
<cfset ErrorMessage &= "<h3>Login Name is required</h3>">
</cfif>
more checks

Step 3 - Check the length of your error message variable

<cfif len(ErrorMessage) gt 0>
display it
<cfelse>
code for no errors
</cfif>

In addition to all this, you probably want to check to see if the page request actually came from your form page. You can use cgi.http_referrer for that.

One more thing. Instead of an anchor tag back to the form page like this,

<p><a href="users.cfm">Return to users list</a>.</p>

You can use javascript so that page does not have to reload in the browser.

<p><a href="javascript:history.back()">Return to users list</a>.</p>
Dan Bracuk
  • 20,699
  • 4
  • 26
  • 43