Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

This issue:

https://issues.apache.org/jira/browse/SHIRO-120

raises a good point.  We shouldn't be swallowing exceptions from
Realms anywhere.  We should instead offer a nice Exception hierarchy
to allow end-users to react to problems.

The best exception hierarchy I've seen exemplifying this is Spring's
DataAccessException and subclasses.

Should we incorporate something similar?  If so, any recommendations?

- Les

Re: Data Access Exceptions

[Brian Demers <br...@gmail.com>]

Agreed, Realm implementers should handle these cases.

On Wed, Jan 6, 2010 at 5:31 PM, Jeremy Haile <jh...@gmail.com> wrote:

> It does if you create a subclass of AuthenticationException called
> SystemErrorAuthenticationException ;-) (or whatever)
>
>
> Les Hazlewood wrote:
>
>> I think of those two exceptions and their hierarchies as representing
>> user-attributed "expected but not frequent" exceptions related to an
>> end-user's failure to authenticate or authorize properly - locked
>> account, incorrect password, etc.  These kind of exceptions are a
>> totally different class of exceptions compared to data source failure.
>>
>> Then there is the issue that, if say a NamingException occurs in
>> either during authentication or authorization, it appears that we'd
>> have to create two separate Exceptions under each hierarchy
>> representing the same system-level problem.  Not too ideal I don't
>> think.
>>
>> I don't think it is nice for example to just catch the problem and
>> throw as a generic AuthenticationException - this doesn't allow the
>> developer catching these exceptions to differentiate between
>> user-level problems and system-level problems.
>>
>> Thoughts?
>>
>> - Les
>>
>> On Wed, Jan 6, 2010 at 4:45 PM, Jeremy Haile<jh...@gmail.com>  wrote:
>>
>>
>>> Don't we already offer two hierarchies for this purpose?
>>>  AuthenticationException and AuthorizationException
>>>
>>>
>>> Les Hazlewood wrote:
>>>
>>>
>>>> This issue:
>>>>
>>>> https://issues.apache.org/jira/browse/SHIRO-120
>>>>
>>>> raises a good point.  We shouldn't be swallowing exceptions from
>>>> Realms anywhere.  We should instead offer a nice Exception hierarchy
>>>> to allow end-users to react to problems.
>>>>
>>>> The best exception hierarchy I've seen exemplifying this is Spring's
>>>> DataAccessException and subclasses.
>>>>
>>>> Should we incorporate something similar?  If so, any recommendations?
>>>>
>>>> - Les
>>>>
>>>>
>>>>
>>>

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

On Thu, Jan 14, 2010 at 8:51 AM, Brian Demers <br...@gmail.com> wrote:
> I wouldn't go the road of a RuntimeException,  If you have more then one
> Realm configured in a ModularRealmAuthenticator you would need to make sure
> it was catching throwable, as to not effect any other configured realms.

The ModularRealmAuthenticator already does this.  It allows the
AuthenticationStrategy.afterAttempt method implementation to decide
what ultimately happens if a Realm throws anything.

> I would stick with the realm only throwing a AthenticationException for now,
> at least if/unless the method signature changes.

I agree until the community decides otherwise (i.e. if people decide
that we should provide a rich exception hierarchy).  We should
definitely not be swallowing an exception to a log.

> Just a few thoughts,
> -Brian

Your feedback is much appreciated!

Thanks,

Les

Re: Data Access Exceptions

[Brian Demers <br...@gmail.com>]

I wouldn't go the road of a RuntimeException,  If you have more then one
Realm configured in a ModularRealmAuthenticator you would need to make sure
it was catching throwable, as to not effect any other configured realms.

I would stick with the realm only throwing a AthenticationException for now,
at least if/unless the method signature changes.

In the case of any sort of remote system, connection problems should be
expected, someone might be doing something as simple as restarting the
machine, or its under heavy load.  Either way it all comes down one of two
facts a.) the user cannot authenticate or b.) the the user cannot authorize.


I am still working with the jsecurity code base (will be changing soon i
hope) so please forgive me if my class name references are a bit off.

Just a few thoughts,
-Brian


On Thu, Jan 14, 2010 at 2:42 AM, Peter Ledbrook <pe...@cacoethes.co.uk>wrote:

> > NamingException is a checked exception.  The Realm interface does not
> > permit checked exceptions since they can vary dramatically across
> > implementations. :/
>
> I would wrap it with a runtime exception then. It's been a while since
> I played with LDAP, so do all cases of NamingException imply either a
> system problem or programmer error?
>
> I certainly think that AuthenticationException and
> AuthorizationException shouldn't be used in such cases where the realm
> should deal with the issue.
>
> Cheers,
>
> Peter
>

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

On Thu, Jan 14, 2010 at 2:42 AM, Peter Ledbrook <pe...@cacoethes.co.uk> wrote:
>> NamingException is a checked exception.
>
> I would wrap it with a runtime exception then. It's been a while since
> I played with LDAP, so do all cases of NamingException imply either a
> system problem or programmer error?

I believe so.  If the system is configured properly, there are no
network/lookup problems, and the name corresponds to a valid ojbect,
you should never get a NamingException.

Currently our other out-of-the-box Realms wrap any checked exceptions
(SQLException, etc) in an AuthenticationException or
AuthorizationException and just let that propagate.

I can change the LdapRealm to do this also, and I bet people wouldn't
complain much with that solution, but it isn't as clean as a
clear/meaningful exception indicating _why_ there is a problem.
Developers would have to catch either exception, get the cause, then
do instanceof checks to react accordingly if they wanted to.  If they
don't want to do that, they must subclass our out-of-the-box Realm (or
write their own) and create custom exceptions.  Is this OK?

The big benefit of just keeping the current behavior (and wrapping in
an AuthenticationException) is that we don't have to concern ourselves
with creating a rich hierarchy that would try to suit most EIS
environments, as Spring's DataAccessException hierarchy does.  Instead
we just let the Shiro end-user worry about this stuff by subclassing
or via instanceof checks.

What's better in this case?

- Les

Re: Data Access Exceptions

[Peter Ledbrook <pe...@cacoethes.co.uk>]

> NamingException is a checked exception.  The Realm interface does not
> permit checked exceptions since they can vary dramatically across
> implementations. :/

I would wrap it with a runtime exception then. It's been a while since
I played with LDAP, so do all cases of NamingException imply either a
system problem or programmer error?

I certainly think that AuthenticationException and
AuthorizationException shouldn't be used in such cases where the realm
should deal with the issue.

Cheers,

Peter

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

NamingException is a checked exception.  The Realm interface does not
permit checked exceptions since they can vary dramatically across
implementations. :/

On Wed, Jan 13, 2010 at 7:24 AM, Peter Ledbrook <pe...@cacoethes.co.uk> wrote:
>> Bump.
>>
>> How should we handle SHIRO-120?
>
> Why not let NamingException propagate? Or can ActiveDirectoryRealm be
> used without a subclass? In which case, you need a
> handleSystemException() method or similar that the subclass can
> override.
>
> Peter
>

Re: Data Access Exceptions

[Peter Ledbrook <pe...@cacoethes.co.uk>]

> Bump.
>
> How should we handle SHIRO-120?

Why not let NamingException propagate? Or can ActiveDirectoryRealm be
used without a subclass? In which case, you need a
handleSystemException() method or similar that the subclass can
override.

Peter

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

Bump.

How should we handle SHIRO-120?

Les

https://issues.apache.org/jira/browse/SHIRO-120

On Thu, Jan 7, 2010 at 9:27 AM, Les Hazlewood <lh...@apache.org> wrote:
> To my 2nd point earlier:
>
> So now I should also create a SystemErrorAuthorizationException if
> anything happens during authorization instead of authentication?  How
> about also creating a SystemErrorSessionException too?
>
> Doesn't that just sound awful?  Replicating system hierarchies just be
> just because of two or three different root parent classes?
>
> I think that system/data exceptions should be handled differently than
> exceptions that occur due to a user action and should be represented
> in a different hierarchy.
>
> - Les
>
> On Wed, Jan 6, 2010 at 5:31 PM, Jeremy Haile <jh...@gmail.com> wrote:
>> It does if you create a subclass of AuthenticationException called
>> SystemErrorAuthenticationException ;-) (or whatever)
>>
>> Les Hazlewood wrote:
>>>
>>> I think of those two exceptions and their hierarchies as representing
>>> user-attributed "expected but not frequent" exceptions related to an
>>> end-user's failure to authenticate or authorize properly - locked
>>> account, incorrect password, etc.  These kind of exceptions are a
>>> totally different class of exceptions compared to data source failure.
>>>
>>> Then there is the issue that, if say a NamingException occurs in
>>> either during authentication or authorization, it appears that we'd
>>> have to create two separate Exceptions under each hierarchy
>>> representing the same system-level problem.  Not too ideal I don't
>>> think.
>>>
>>> I don't think it is nice for example to just catch the problem and
>>> throw as a generic AuthenticationException - this doesn't allow the
>>> developer catching these exceptions to differentiate between
>>> user-level problems and system-level problems.
>>>
>>> Thoughts?
>>>
>>> - Les
>>>
>>> On Wed, Jan 6, 2010 at 4:45 PM, Jeremy Haile<jh...@gmail.com>  wrote:
>>>
>>>>
>>>> Don't we already offer two hierarchies for this purpose?
>>>>  AuthenticationException and AuthorizationException
>>>>
>>>>
>>>> Les Hazlewood wrote:
>>>>
>>>>>
>>>>> This issue:
>>>>>
>>>>> https://issues.apache.org/jira/browse/SHIRO-120
>>>>>
>>>>> raises a good point.  We shouldn't be swallowing exceptions from
>>>>> Realms anywhere.  We should instead offer a nice Exception hierarchy
>>>>> to allow end-users to react to problems.
>>>>>
>>>>> The best exception hierarchy I've seen exemplifying this is Spring's
>>>>> DataAccessException and subclasses.
>>>>>
>>>>> Should we incorporate something similar?  If so, any recommendations?
>>>>>
>>>>> - Les
>>>>>
>>>>>
>>
>

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

To my 2nd point earlier:

So now I should also create a SystemErrorAuthorizationException if
anything happens during authorization instead of authentication?  How
about also creating a SystemErrorSessionException too?

Doesn't that just sound awful?  Replicating system hierarchies just be
just because of two or three different root parent classes?

I think that system/data exceptions should be handled differently than
exceptions that occur due to a user action and should be represented
in a different hierarchy.

- Les

On Wed, Jan 6, 2010 at 5:31 PM, Jeremy Haile <jh...@gmail.com> wrote:
> It does if you create a subclass of AuthenticationException called
> SystemErrorAuthenticationException ;-) (or whatever)
>
> Les Hazlewood wrote:
>>
>> I think of those two exceptions and their hierarchies as representing
>> user-attributed "expected but not frequent" exceptions related to an
>> end-user's failure to authenticate or authorize properly - locked
>> account, incorrect password, etc.  These kind of exceptions are a
>> totally different class of exceptions compared to data source failure.
>>
>> Then there is the issue that, if say a NamingException occurs in
>> either during authentication or authorization, it appears that we'd
>> have to create two separate Exceptions under each hierarchy
>> representing the same system-level problem.  Not too ideal I don't
>> think.
>>
>> I don't think it is nice for example to just catch the problem and
>> throw as a generic AuthenticationException - this doesn't allow the
>> developer catching these exceptions to differentiate between
>> user-level problems and system-level problems.
>>
>> Thoughts?
>>
>> - Les
>>
>> On Wed, Jan 6, 2010 at 4:45 PM, Jeremy Haile<jh...@gmail.com>  wrote:
>>
>>>
>>> Don't we already offer two hierarchies for this purpose?
>>>  AuthenticationException and AuthorizationException
>>>
>>>
>>> Les Hazlewood wrote:
>>>
>>>>
>>>> This issue:
>>>>
>>>> https://issues.apache.org/jira/browse/SHIRO-120
>>>>
>>>> raises a good point.  We shouldn't be swallowing exceptions from
>>>> Realms anywhere.  We should instead offer a nice Exception hierarchy
>>>> to allow end-users to react to problems.
>>>>
>>>> The best exception hierarchy I've seen exemplifying this is Spring's
>>>> DataAccessException and subclasses.
>>>>
>>>> Should we incorporate something similar?  If so, any recommendations?
>>>>
>>>> - Les
>>>>
>>>>
>

Re: Data Access Exceptions

[Jeremy Haile <jh...@gmail.com>]

It does if you create a subclass of AuthenticationException called 
SystemErrorAuthenticationException ;-) (or whatever)

Les Hazlewood wrote:
> I think of those two exceptions and their hierarchies as representing
> user-attributed "expected but not frequent" exceptions related to an
> end-user's failure to authenticate or authorize properly - locked
> account, incorrect password, etc.  These kind of exceptions are a
> totally different class of exceptions compared to data source failure.
>
> Then there is the issue that, if say a NamingException occurs in
> either during authentication or authorization, it appears that we'd
> have to create two separate Exceptions under each hierarchy
> representing the same system-level problem.  Not too ideal I don't
> think.
>
> I don't think it is nice for example to just catch the problem and
> throw as a generic AuthenticationException - this doesn't allow the
> developer catching these exceptions to differentiate between
> user-level problems and system-level problems.
>
> Thoughts?
>
> - Les
>
> On Wed, Jan 6, 2010 at 4:45 PM, Jeremy Haile<jh...@gmail.com>  wrote:
>    
>> Don't we already offer two hierarchies for this purpose?
>>   AuthenticationException and AuthorizationException
>>
>>
>> Les Hazlewood wrote:
>>      
>>> This issue:
>>>
>>> https://issues.apache.org/jira/browse/SHIRO-120
>>>
>>> raises a good point.  We shouldn't be swallowing exceptions from
>>> Realms anywhere.  We should instead offer a nice Exception hierarchy
>>> to allow end-users to react to problems.
>>>
>>> The best exception hierarchy I've seen exemplifying this is Spring's
>>> DataAccessException and subclasses.
>>>
>>> Should we incorporate something similar?  If so, any recommendations?
>>>
>>> - Les
>>>
>>>        

Re: Data Access Exceptions

[Les Hazlewood <lh...@apache.org>]

I think of those two exceptions and their hierarchies as representing
user-attributed "expected but not frequent" exceptions related to an
end-user's failure to authenticate or authorize properly - locked
account, incorrect password, etc.  These kind of exceptions are a
totally different class of exceptions compared to data source failure.

Then there is the issue that, if say a NamingException occurs in
either during authentication or authorization, it appears that we'd
have to create two separate Exceptions under each hierarchy
representing the same system-level problem.  Not too ideal I don't
think.

I don't think it is nice for example to just catch the problem and
throw as a generic AuthenticationException - this doesn't allow the
developer catching these exceptions to differentiate between
user-level problems and system-level problems.

Thoughts?

- Les

On Wed, Jan 6, 2010 at 4:45 PM, Jeremy Haile <jh...@gmail.com> wrote:
> Don't we already offer two hierarchies for this purpose?
>  AuthenticationException and AuthorizationException
>
>
> Les Hazlewood wrote:
>>
>> This issue:
>>
>> https://issues.apache.org/jira/browse/SHIRO-120
>>
>> raises a good point.  We shouldn't be swallowing exceptions from
>> Realms anywhere.  We should instead offer a nice Exception hierarchy
>> to allow end-users to react to problems.
>>
>> The best exception hierarchy I've seen exemplifying this is Spring's
>> DataAccessException and subclasses.
>>
>> Should we incorporate something similar?  If so, any recommendations?
>>
>> - Les
>>
>

Re: Data Access Exceptions

[Jeremy Haile <jh...@gmail.com>]

Don't we already offer two hierarchies for this purpose?  
AuthenticationException and AuthorizationException


Les Hazlewood wrote:
> This issue:
>
> https://issues.apache.org/jira/browse/SHIRO-120
>
> raises a good point.  We shouldn't be swallowing exceptions from
> Realms anywhere.  We should instead offer a nice Exception hierarchy
> to allow end-users to react to problems.
>
> The best exception hierarchy I've seen exemplifying this is Spring's
> DataAccessException and subclasses.
>
> Should we incorporate something similar?  If so, any recommendations?
>
> - Les
>