What we have at the moment isn't intuitive. I would probably load the
User class as default and add a field authorized true/false. Also don't
forget to remove the intial "User user = request.getUser() " as its
currently redundant.
Mark
Any catching of exceptions from user defined FtpLets should be logged,
debug makes sense.
Niklas Gustavsson wrote:
Mark Proctor wrote:
The onLogin request.getUser() returns null. So I started looking at
the PASS class:
String userName = request.getUserArgument()
User user = request.getUser()
So here you are requesting user, which is always null, but even if it
did return a value you never use it anyway as user is later nulled as
it's actually set by the authentication code. Further to this you
call the onLogin method before you do the request.setUser(user), so
I guess I'm asking should User be available from onLogin or not?
As it works right now, the Ftplet is given the possibility of denying
the login. Maybe we should change this so that we first set the status
as it would be logged in, and then if the Ftplet returns RET_SKIP we
reset the status so that the user is now set as denied access. I would
be happy to make this change if you think it makes sense.
If not, this should probably be documented. Either way the fist user
assigment and null check is redundant. I also notied that you are
swollowing Exceptions thrown by the FtpLets, even null pointers,
these should be atleast logged, as it makes tracking down bugs that
are occuring in them difficult.
By this you mean for example:
try{
ftpletRet = ftpletContainer.onLogin(request, out);
} catch(Exception e) {
ftpletRet = FtpletEnum.RET_DISCONNECT;
}
Adding a log there would make sense, possibly at DEBUG level?
/niklas
