Problem Creating a User Object

[chromis]

Hi,

I've been trying to implement a more OOP oriented approach to dealing with user security on one of my websites, and I am trying to validate the user against an array of roles, however I am struggling with a type error:

 The argument ROLES passed to function setRoles() is not of type array.
If the component name is specified as a type of this argument, the reason for this error might be that a definition file for such component cannot be found or is not accessible.
 
The error occurred in D:\***\***\AppTest\com\user.cfc: line 50

48 :     <cffunction name="getRoles" access="public" output="true" returntype="array">
49 : 		<cfreturn variables.roles />
50 : 	</cffunction>
51 : 
52 : 	<cffunction name="setRoles" access="public" output="false" returntype="void">

I am definately passing in an array, or at least a list which I think are basically the same thing? Here is my code for the instantiating the User object (contained in /index.cfm):

    if(NOT isDefined("Session.user")) {
        Request.Ses.user = createObject("component","com.user").init(Request.App.Dsn);
		Request.Ses.user.setUsername("eddy");
		Request.Ses.user.setPassword("eddy");			
		Request.Ses.user.setRoles("staff, user");					
	}

and here is the User.cfc object:

<!--- security component --->
<cfcomponent displayname="user" output="false" hint="Provides authentication for users">
	<cffunction name="init" access="public" output="false" returntype="user" hint="Constructor for this CFC">
		<!--- take DSN as argument --->
		<cfargument name="dsn" type="string" required="true" hint="The datasource name" />       		
		<!--- put dsn in variables scope so we can use it throughout the CFC --->
		<cfscript>
            // datasource
            variables.dsn = arguments.dsn;
            // user details
            variables.username = "";			
            variables.password = "";
            variables.loginStatus = 0;
            variables.roles = ArrayNew(1);
        </cfscript>
		<!--- return this CFC --->
		<cfreturn this />
	</cffunction>
    
    <!--- Object data functions --->
	<cffunction name="getUsername" access="public" output="false" returntype="string">
		<cfreturn variables.username />
	</cffunction>

	<cffunction name="setUsername" access="public" output="false" returntype="void">
		<cfargument name="username" type="string" required="true" />
		<cfset variables.username = arguments.username />
	</cffunction>
	
    <cffunction name="getPassword" access="private" output="false" returntype="string">
		<cfreturn variables.password />
	</cffunction>

	<cffunction name="setPassword" access="public" output="false" returntype="void">
		<cfargument name="password" type="string" required="true" />
		<cfset variables.password = arguments.password />
	</cffunction>

    <cffunction name="getLoginStatus" access="private" output="false" returntype="string">
		<cfreturn variables.loginStatus />
	</cffunction>

	<cffunction name="setLoginStatus" access="public" output="false" returntype="void">
		<cfargument name="loginStatus" type="string" required="true" />
		<cfset variables.loginStatus = arguments.loginStatus />
	</cffunction>
    
    <cffunction name="getRoles" access="public" output="true" returntype="array">
		<cfreturn variables.roles />
	</cffunction>

	<cffunction name="setRoles" access="public" output="false" returntype="void">
		<cfargument name="roles" type="array" required="true" />
		<cfset variables.roles = arguments.roles />
	</cffunction>         
       
    <!--- C,R,U,D Methods --->
	<cffunction name="read" access="public" output="false" returntype="Void" hint="CRUD Method"> 
        <cfargument name="user" type="user" required="yes" />
        <cfargument name="username" type="string" required="yes" />
        
    	<cfset var qread = 0 />
        
        <cfquery name="qRead" datasource="#variables.dsn#">
            SELECT
            username, roles
            FROM cms_security
            WHERE
            username = <cfqueryparam value="#variables.getUsername()#" 
                            cfsqltype="cf_sql_varchar" />
		</cfquery>
        <cfif qRead.RecordCount>
        	<cfset arguments.User.setRoles(qRead.roles) />
        	<cfelse>
            	<cfthrow type="emptyRecordset" errorcode="User.read.emptyRecordset" message="User with name #arguments.username# not found" />
		</cfif>
    </cffunction>
        
    <!--- Persistent data functions --->
	<cffunction name="checkRoles" access="public" output="false" returntype="boolean" hint="Checks user roles against a supplied array or list">
            
    	<!--- Initialise variables --->
		<cfset var results = StructNew() />
		<cfset var qCheckRoles = 0 />
        
		<!--- defaults --->
		<cfset results.success = true />
		<cfset results.message = "The user has been validated." />        
        
		<cftry>
			<cfquery name="qCheckRoles" datasource="#variables.dsn#">
				SELECT roles
				FROM cms_security
				WHERE username = <cfqueryparam value="#variables.getUsername()#" 
						cfsqltype="cf_sql_varchar" />
			</cfquery>
			<cfcatch type="database">
				<cfset results.success = false />
				<cfset results.message = "User role check failed.  The error details if available 
					are as follows: " & CFCATCH.Detail />
			</cfcatch>
		</cftry>
        
		<!--- if we got data back, initialize the object --->
		<cfif IsQuery(qCheckRoles) AND qCheckRoles.RecordCount EQ 1>
        	<cfdump var="#qCheckRoles#">
            <cfreturn true>
        <cfelse>
        	<cfreturn false>
		</cfif>
        
	</cffunction>     
	<cffunction name="validate" access="public" output="false" returntype="boolean" hint="validates username and password">
        
    	<!--- Initialise variables --->
		<cfset var results = StructNew() />
		<cfset var qValidate = 0 />
        
		<!--- defaults --->
		<cfset results.success = true />
		<cfset results.message = "The user has been validated." />        
        
		<cftry>
			<cfquery name="qValidate" datasource="#variables.dsn#">
				SELECT username,password
				FROM cms_security
				WHERE username = <cfqueryparam value="#variables.getUsername()#" 
						cfsqltype="cf_sql_varchar" />
				AND password = '#hash(variables.password)#'
			</cfquery>
			<cfcatch type="database">
				<cfset results.success = false />
				<cfset results.message = "The person insert failed.  The error details if available 
					are as follows: " & CFCATCH.Detail />
			</cfcatch>
		</cftry>
        
		<!--- if we got data back, initialize the object --->
		<cfif IsQuery(qValidate) AND qValidate.RecordCount EQ 1>        
			<cfset setLoginStatus(true) />
            <cfreturn true>
        <cfelse>
        	<cfreturn false>
		</cfif>
        
	</cffunction>     
</cfcomponent>

Could someone point out where I am going wrong?

Thanks,

Chromis

[acoder]

You're passing a list rather than an array. What you could do is accept a string as the input and convert to an array in the function, or create the array with cfset and pass that to setRoles.

[chromis]

You're passing a list rather than an array. What you could do is accept a string as the input and convert to an array in the function, or create the array with cfset and pass that to setRoles.

Ah right ok I'll do that. Thanks alot for your help, I'll no doubt have some questions shortly!

[acoder]

No problem, if you do, you know where to come :)

[chromis]

No problem, if you do, you know where to come :)

Ok I'm getting a little confused, basically I want to create an OOP method for validating a user, the user must have a valid username, password and also have one of the roles for the particular page that they are viewing.

I've got the fundamentals of the validation working now, though i'm sure i'm not getting the structure of the User.cfc quite right. I've read about OOP and it talks about DAOs and Gateways I have tryed to implement this sort of structure. Tho I have struggled to separate the database calls into a Gateway file.
At the moment i seem to have a kind of mis-match of the two in one file. Could you tell me how i could improve this code?

Instantiation and data output (/index.cfm):

<cfscript>
    if(NOT isDefined("Session.user")) {
        Request.Ses.user = createObject("component","com.user").init(Request.App.Dsn);
		Request.Ses.user.setUsername("eddy");
		Request.Ses.user.setPassword("eddy");									
	}
</cfscript>

<cfdump var="#Request#">

<cfoutput>Validate result: #Request.Ses.user.validate()#</cfoutput>
Roles: <cfoutput>#Request.Ses.user.getRoles()#</cfoutput>
Actual Roles:<cfoutput>#Request.Ses.user.getActualRoles()#</cfoutput>
Validate roles: <cfoutput>#Request.Ses.user.checkRoles("staff,user")#</cfoutput>

User DAO / Gateway (/com/User.cfc):

<!--- security component --->
<cfcomponent displayname="user" output="false" hint="Provides authentication for users">
	<cffunction name="init" access="public" output="false" returntype="user" hint="Constructor for this CFC">
		<!--- take DSN as argument --->
		<cfargument name="dsn" type="string" required="true" hint="The datasource name" />       		
		<!--- put dsn in variables scope so we can use it throughout the CFC --->
		<cfscript>
            // datasource
            variables.dsn = arguments.dsn;
            // user details
            variables.username = "";			
            variables.password = "";
            variables.loginStatus = 0;
            variables.roles = "";
            variables.actualRoles = "";			
        </cfscript>
		<!--- return this CFC --->
		<cfreturn this />
	</cffunction>
    
    <!--- Object data functions --->
	<cffunction name="getUsername" access="public" output="false" returntype="string">
		<cfreturn variables.username />
	</cffunction>

	<cffunction name="setUsername" access="public" output="false" returntype="void">
		<cfargument name="username" type="string" required="true" />
		<cfset variables.username = arguments.username />
	</cffunction>
	
    <cffunction name="getPassword" access="private" output="false" returntype="string">
		<cfreturn variables.password />
	</cffunction>

	<cffunction name="setPassword" access="public" output="false" returntype="void">
		<cfargument name="password" type="string" required="true" />
		<cfset variables.password = arguments.password />
	</cffunction>

    <cffunction name="getLoginStatus" access="private" output="false" returntype="string">
		<cfreturn variables.loginStatus />
	</cffunction>

	<cffunction name="setLoginStatus" access="public" output="false" returntype="void">
		<cfargument name="loginStatus" type="string" required="true" />
		<cfset variables.loginStatus = arguments.loginStatus />
	</cffunction>
    
    <cffunction name="getRoles" access="public" output="true" returntype="string">
		<cfreturn variables.roles />
	</cffunction>
     
	<cffunction name="setRoles" access="public" output="false" returntype="void">
		<cfargument name="roles" type="string" required="true" />
		<cfset variables.roles = arguments.roles />
	</cffunction>         
        
    <!--- Persistent data functions --->
	<cffunction name="checkRoles" access="public" output="false" returntype="boolean" hint="Checks user roles against a supplied array or list">
		<cfargument name="roles" type="string" required="true" />
        
    	<!--- Initialise variables --->
		<cfset var results = StructNew() />
		<cfset var qCheckRoles = 0 />
        
		<!--- defaults --->
		<cfset results.success = true />
		<cfset results.message = "The user has been validated." />        
        
		<cftry>
			<cfquery name="qCheckRoles" datasource="#variables.dsn#">
				SELECT roles
				FROM cms_security
				WHERE username = <cfqueryparam value="#variables.getUsername()#" 
						cfsqltype="cf_sql_varchar" />
			</cfquery>
			<cfcatch type="database">
				<cfset results.success = false />
				<cfset results.message = "User role check failed.  The error details if available 
					are as follows: " & CFCATCH.Detail />
			</cfcatch>
		</cftry>
        
		<!--- if we got data back, check roles --->
		<cfif IsQuery(qCheckRoles) AND qCheckRoles.RecordCount EQ 1>
        
        	<cfset variables.userRoles = ValueList(qCheckRoles.roles,",")>
            
            <cfloop list="#variables.userRoles#" index="role">
                <cfif ListFindNoCase(arguments.roles,role) GT 0>
                    <cfset roleFound = true>
                </cfif>
            </cfloop>
            
            <cfif roleFound EQ true>
				<cfreturn true>
			<cfelse>
                <cfreturn false>
            </cfif>
            
        <cfelse>
        	<cfreturn false>
		</cfif>
        
	</cffunction>     
	<cffunction name="validate" access="public" output="false" returntype="boolean" hint="validates username and password">
        
    	<!--- Initialise variables --->
		<cfset var results = StructNew() />
		<cfset var qValidate = 0 />
        
		<!--- defaults --->
		<cfset results.success = true />
		<cfset results.message = "The user has been validated." />        
        
		<cftry>
			<cfquery name="qValidate" datasource="#variables.dsn#">
				SELECT username,password
				FROM cms_security
				WHERE username = <cfqueryparam value="#variables.getUsername()#" 
						cfsqltype="cf_sql_varchar" />
				AND password = '#hash(variables.password)#'
			</cfquery>
			<cfcatch type="database">
				<cfset results.success = false />
				<cfset results.message = "The person insert failed.  The error details if available 
					are as follows: " & CFCATCH.Detail />
			</cfcatch>
		</cftry>
        
		<!--- if we got data back, initialize the object --->
		<cfif IsQuery(qValidate) AND qValidate.RecordCount EQ 1>        
			<cfset setLoginStatus(true) />
            <cfreturn true>
        <cfelse>
        	<cfreturn false>
		</cfif>
        
	</cffunction>     
</cfcomponent>

The parts I want to look at specifically are the checkRoles and validate methods. Should I put them into one method or attempt to separate the checkRoles and validate database calls into two different methods and have one validate method? I'm a bit confused :)

Thanks,

Chromis
[code]

[acoder]

It probably would be a good idea to put the database calls in its own object which you can instantiate/call when the user object is initialised. Then it'd smply be a function call to make the query which would return true/false or a result struct. As for the validate() and checkRoles() functions, it depends on whether you ever need only the checkRoles function. If you do, you can call checkRoles within validate should validate() also validate roles.

[chromis]

It probably would be a good idea to put the database calls in its own object which you can instantiate/call when the user object is initialised. Then it'd smply be a function call to make the query which would return true/false or a result struct. As for the validate() and checkRoles() functions, it depends on whether you ever need only the checkRoles function. If you do, you can call checkRoles within validate should validate() also validate roles.

Thanks acoder, I'll have a go and post back what i come up with.

[chromis]

Hi acoder,

Ok, I'm had some time to look at this code again and I think what I need to do is create a UserDAO.cfc for my persistent data functions and then pass the user.cfc object to the UserDAO.cfc object upon initialisation, I can then authenticate and check the roles using UserDAO, is this right?

On various pages I'll need to the check the user roles against the role for that page and then redirect the user to another page, do I need to create another object for this, security.cfc for instance? My thinking is that the user objects shouldn't really be used for this sort of thing, as it's not a user objects responsibility to perform the business logic.

Thanks,

Chromis

[acoder]

That's right, though you could have a method in user.cfc which performs this logic, i.e. checks that the user has the role for that page; if not, redirect.

[chromis]

Ok, I started going down the route of creating a separate object (security.cfc) to deal with the business logic of redirecting the user, but that got me into the realm of keeping that object persistent aswell which seemed really overkill. In the end I took your advice and put a protect function in the user.cfc to redirect the user to a different page if the roles for the user were not found. This is definately the easiest method and for this project not one that's going to cause any problems so I guess I'll stick with it.

I guess what I'm trying to find out whilst learning OOP is when to separate logic and when not to, alot of what I have read so far talks about making one object do one thing well and what felt wrong about the user.cfc having a protect function and also the database interaction functions, was that it was no longer just defining a user, but providing functions to deal with it within the application.
Could you give me any advice regarding this?

Anyway here's my protect statement which I'm putting in my index.cfm's, if the user does not have the specified role record in the database, the user will be redirected to the specified URL.

index.cfm

	Request.Ses.User.protect("staff","../index.cfm");

User.cfc - protect function.

	<cffunction name="protect" access="public" output="true" returntype="boolean" hint="Checks user roles against a supplied array or list">
		<cfargument name="roles" type="string" required="true" />
		<cfargument name="redirectURL" type="string" required="true" />    
    
        <cfif variables.checkRoles(arguments.roles) EQ true>
         <cflocation url="#arguments.redirectURL#">
        	<cfreturn true>
        <cfelse>
            <cflocation url="#arguments.redirectURL#">
            <cfreturn false>
		</cfif>
       
	</cffunction>

Again thanks for your help!

[acoder]

I see what you mean. Coldfusion doesn't place any restrictions in terms of OOP, though this may change in future versions.

You could look at frameworks such as Fusebox/Mach-II which might help. You could also look into the MVC (Model-View-Controller) pattern for separation.

[chromis]

Ok well that's put me at ease a bit. I have looked into machII and the MVC pattern, I've only developed a couple of tests so far. I have a few questions regarding this though:

1. At what scale of application does the machII begin to be useful?
2. Is it good for rapid application development?
3. Does it lend itself well to OOP techniques?
4. What do you develop with?

[acoder]

Mach-II is probably better suited for large, complex applications. Another alternative is Model-Glue which can be simpler. Both enforce MVC whereas Fusebox doesn't. You may find this article/slideshow useful which compares the three.

What do I use? I don't use any, though I probably should. If I was developing from the ground up, I would and I'd change my programming style too.

[chromis]

Thanks I'll take a look at that. I guess for the app i'm developing at the moment I won't be needing a framework. But from what I have seen so far I think MachII would be my choice.

Again thanks for you help i'll no doubt be asking more questions soon!

[acoder]

No problem. This is an interesting discussion probably not covered in this forum before. Fire away when the need comes and I'll see what I can do :)