User authentication in Mvc

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

piccoloprincipe
Forum Newbie
Posts: 12
Joined: Wed Jan 16, 2008 4:11 pm

User authentication in Mvc

Post by piccoloprincipe »

I have to control if an user is authenticated, so there's a registered variable in a singleton registry that I have to look up.
I have written a method in the Controller base class that return the User object if is authenticated or throws a NotAuthenticatedUserException if not. So if user shouldn't be there, I not catch the exception, while if he could have clicked a link that redirect there I will catch the exception and redirect him to the authentication form.
Maybe it's better to redirect to authentication in any case? And what about this sequence of redirect? After loading a view I think I have to call exit() to prevent any code from being executed after my redirect...
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Re: User authentication in Mvc

Post by Kieran Huggins »

my first instinct would be to return from the action instead.

What framework is this? Cake? Symfony?
piccoloprincipe
Forum Newbie
Posts: 12
Joined: Wed Jan 16, 2008 4:11 pm

Re: User authentication in Mvc

Post by piccoloprincipe »

My own framework. So it's a design question..
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: User authentication in Mvc

Post by Mordred »

Thou shalt not use exceptions for code logic.
An unauthenticated user is hardly an exception.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Re: User authentication in Mvc

Post by Chris Corbyn »

Mordred wrote:Thou shalt not use exceptions for code logic.
An unauthenticated user is hardly an exception.
Disagree. We have things like this all over the code in SitePoint's apps.

Code: Select all

if (!$user = $this->getUser()) {
  throw new AccessException('You must be logged in to view this page');
}
I think people's use of exceptions differs vastly. They have a unique way of controlling program flow compared with if/else structures.

You don't strictly need to do a HTTP redirect for authentication demands however; you can just display a login template which POSTs to the login controller.
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: User authentication in Mvc

Post by Mordred »

I'm sorry, but how does the fact that you're using it prove that it's good? ;)

Exceptions should be used for exceptional situations where the module in question cannot handle them properly.
Your case, and the OP's case is not an exceptional situation, and they can handle the situation quite well. If you want to delegate the handling to another module, call it specifically and explicitly with a callback.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Re: User authentication in Mvc

Post by Chris Corbyn »

I didn't say I was right... I just disagreed ;)

I'm not really up for getting into yet another "is it an exceptional scenario" debate though 8)
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Re: User authentication in Mvc

Post by Kieran Huggins »

I know we're not getting into a big debate, but I sort of lie in the middle. I think reacting to a failed login is reasonable program flow issue and definitely lies in the pervue of the controller action. However, I would be all for raising a "not logged in" exception before a a method that expects a logged in user. I implement them all the time in before filters and it just feels right to me.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: User authentication in Mvc

Post by Christopher »

Mordred wrote:Exceptions should be used for exceptional situations where the module in question cannot handle them properly.
We had a discussion awhile back about this ... and I have to say that I generally disagree with your position. I think your ban of exceptions from "code logic" may sound good but in reality makes no sense -- what part of code does not have logic? Likewise, your rule about "exceptional situations" suffers from a serious lack of specificity as what exactly "exceptional situations" means.

If you instead think of exceptions as simply a control structure like if(), but that does not require return values to be passed up through layers -- then you have a more practical rule. That is, if you don't want to or can't require intervening layers to know how to pass a return value, then using an exception may make sense.
(#10850)
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: User authentication in Mvc

Post by alex.barylski »

arborint wrote:If you instead think of exceptions as simply a control structure like if(), but that does not require return values to be passed up through layers -- then you have a more practical rule.
Disagree with the first statement, agree with the latter -- sorta.

Your latter rule has one small issue -- it's dependent on the throw'er of the exception to throw unique Exception or set a globally unique return code.

Code: Select all

doSomething() -> doSomethingAgain() -> oneMoreThing();
Assume each of those functions call each other, they are not sibling functions.

oneMoreThing() throws an exception, which bubbles up into the calling code for the first function doSomething().

The try-catch handler looks something like:

Code: Select all

try{
  doSomething();
}
catch(Exception $e){
  // TODO: Determine which exception was raised???
}
You have two choices in determining what the exception is:
1) Set the error code - similar to using return codes
2) Derive a custom Exception class and throw that

Or I guess you could just handle the exception as a generic Exception - but that likely wouldn't be ideal.

This same effect can be accomplished using return codes as you've noted:

Code: Select all

doSomething() -> doSomethingAgain() -> oneMoreThing();

Code: Select all

function doSomethingAgain()
{
  if($a == 0) return false; // Notify calling code about error
  // TODO: Do stuff...
}
 
function doSomething()
{
  if(doSomethingAgain() === false) return false; // Let calling code know about error
}
Personally, returning an error code or throwing an exception -- there is little difference in this regard so I think the argument is moot -- although more practical that being vague, there difference is not significant.

Using exceptions, one needs to devise a mechanism to unique identify what that exception is all about - at least when handling the error in a context other than where the error occurs -- which is the strength of your argument (avoiding bubbling up return codes).

On the other hand, using return codes are less work in terms of indicating an error (throw versus return - the latter doesn't need custom classes) but becomes more complex than exceptions as you have to explicitly bubble error codes up to original calling code. Which is not a lot of work - probably less than introducing yet another CustomException class.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: User authentication in Mvc

Post by Christopher »

Two things, first you can't have return values with a fluent interface because the methods return an object. If the methods are all in one class then they could share a property.

Second, the point I was making was, given placeWhereConditionIs(), intermediateLayer(), and placeWhereConditionIsDealtWith() -- the difference between exceptions and return values is that with return values intermediateLayer() needs to know how to pass the return value, with exceptions it does not. That implies some possible code reduction and looser coupling with exceptions in some circumstances.
(#10850)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: User authentication in Mvc

Post by Mordred »

Exceptions break the code flow. Remember "goto statement considered harmful"? throws are gotos that are outside of the current scope, and in fact you can't easily follow where the corresponding catch is. The catch-es are like "come from"-s, with no easier way to know who threw the exception. This is why for code logic it is better to use the regular code flow mechanism which is function calls. With exceptional situations both the throwing and the catching sides are allowed not to know where the corresponding parts are.

A practical rule that one can follow with exceptions is this: can we deal with the situation here, in this scope, or do we "see"/"know" someone who can deal with it (with a function call). If so - use a function call; if not - throw exception.

Imagine a program reading data from a file, which gets a data read error. This is not a condition it can deal with, so it should throw an exception. Now imagine a disk checking utility that gets a data read error while reading a file. For it, this is clearly not an exceptional matter: "Yeah, yeah, good ol' data read error. Calling up the routine to mark it as a bad sector."

With the OP's example, an analogy would be to call a function do deal with the "user is not authenticated" case, and throw an exception if the database query returns an error (the database server is down?) The first event is definitely in the area of interest for this module, while the second is foreign, so we just raise the alarm and let the authority on that type of alarm deal with it.
Hockey wrote:You have two choices in determining what the exception is:
1) Set the error code - similar to using return codes
2) Derive a custom Exception class and throw that
Well, you seem to have given the answer yourself, so I don't know why you raise the question at all. The type of error is "encoded" in the type of exception being thrown/catched. Passing an error code with the exception is redundant and wrong. Moreover, a good hierarchy of exception classes will allow for a fine-grained control over what to catch in "upper" modules.

(btw I don't think Hockey meant fluent interfaces by using those arrows, just functions that call each other)
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: User authentication in Mvc

Post by alex.barylski »

The type of error is "encoded" in the type of exception being thrown/catched. Passing an error code with the exception is redundant and wrong
Redundant and wrong? Explain please. :)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: User authentication in Mvc

Post by Mordred »

You should use the type-hinting properties of the catch statements.
Instead of throwing and catching one exception class, and then do a switch in the catch to see the actual error code, you should extend the base Exception class, throw a specific subclass of it, and catch the class directly. Thus in any specific catch statement you know precisely what went wrong, instead of checking additonal error codes.

Since a subclass represents a concrete error condition, passing an error type code when throwing is redundant.
Since a type-hinted catch is able to process an exact error condition, checking for an error type code is wrong.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: User authentication in Mvc

Post by Christopher »

Well I am glad to see that we agree that try/catch() can be a replacement for if() or switch(). And I think that when you say, "can we deal with the situation here, in this scope, or do we "see"/"know" someone who can deal with it (with a function call)" you are saying the same thing that I am saying above in my second point. That is that there is in-between code to convey the data. The good thing about those statements is that they are clear descriptions of actual things that happen in programs.

But where you are not clear is on this idea of "exceptional situations." Because, not all programming problems where there is no in-between code to convey data are "exceptional situations". Many are quite expected and some have nothing to do with errors. That is to problem with arguments based on this idea of "exceptional."
(#10850)
Post Reply