FTPSERVER - Make User interface more basic

Niklas,

What do you think about making User more basic, ie, more
abstract, 
having only one method such as getEnabled(). Then push
everything else 
up higher, like the Authority[].  This way we can allow
other User-code 
styles that may not use the those mappings or approaches. 
In other 
words keep the interface very basic.

There are many different styles or flavors of
"User" implementations, 
one for example could be based on nss/pam or another which
uses a 
database where these current default User can cause a
problem(s). 
Some-what related is the concept of how different file
systems handle 
home directories, the native one wants one, but many virtual
file 
systems do not. Unfortunately that "baggage" comes
along with the base 
User interface, regardless if it's implemented or not.

The other benefit of making the User class more basic is
that we can 
clearly split out authentication and authorization. These
are related 
but not the same, and the default User should not define
them.  Leave 
this up to a higher class such as BaseUser or a user's
custom User 
implementation.  Keeping in mind that authentication asks
the question 
"Who are you?" and authorization asks the question
"Okay, I know who you 
are, now what are you allowed to do?". So keep the
authentication and 
authorization in separate interfaces, and do not make them
mandatory in 
the base User class.

Also keep in mind that even the concept of a user name is
different. For 
some user name means the login name, for others the persons
full name, 
and yet for others it may only be the first or last name. So
specifying 
any type of user name in the base User class is probably not
a good 
idea. Leave the user name definition to a higher level
class.

While it is possible to create a "super" User
interface that extends 
other interfaces, this is not really a good idea for other
reasons. It 
too would eventually lock the code to an implementation that
may not be 
desirable in the future. Best to keep things separate, then
join them at 
the implementation class.

Proposed Basic User Interface
--------------------

public interface User {

    boolean getEnabled();
}


Example: Users Implementation
-----------------------------------

public class MyVirtualUser implements
    User,MyAuthentication,MyAuthorization {

    ...

    MySession getSession() {
       return this.mySession;
    }

    int getUid() {
       return this.uid;
    }

    boolean getEnabled() {
       return this.isEnabled;
    }
    ...
}


Example: In the FileSystemManager
------------------------

public FileSystemView createFileSystemView(User user) {

       MyVirtualUser myUser (MyVirtualUser) user;

       if (myUser.getEnabled) {

          MySession session = myUser.getSession();

          int uid = myUser.getUid();
          ...
       }
}


What do you think, just keep the User interface very basic,
one method only?

Andy Thomson

On Tue, Jun 3, 2008 at 10:55 PM, Andy Thomson
<a10008051gmail.com> wrote:
> Niklas,
>
> What do you think about making User more basic, ie,
more abstract, having
> only one method such as getEnabled(). Then push
everything else up higher,
> like the Authority[].  This way we can allow other
User-code styles that may
> not use the those mappings or approaches.  In other
words keep the interface
> very basic.

Not to detract from this discussion, but my plan is to
propose that we
rip out our home-grown security design and replace it with
JSecurity
(http://www.jsecurity.org
). The project is currently being voted for
incubation at Apache and I think it would be a perfect fit
for
FtpServer. In that case, the current User interface would be
replaced
by the Subject interface:
http://www.jsecurity.org/api/org/jsecurity/subjec
t/Subject.html

What's your thoughts on this?

/niklas

Niklas,

The jsecurity.org is very nice, it's that type of framework
that I was 
thinking about for making the User "less" or more
abstract. This is far 
superior though to what I had in mind.

The multiple Realm(s), Authentication, Authorization and
especially the 
way the Session is handled is nice.  I can see where you
could 
immediately enhance it by adding compliance checks among
other things. 
The nice thing is it [security] can be completely outside of
the file 
system, but available to it.

This has some dependencies on the asm-xxx libraries, watch
out for these 
with Hibernate, and Open-Ejb.  If you get class not found
errors, pull 
down the asm-all-version jar and use it instead of the 3-4
asm jar files 
currently used.  I had already dumped the home-grown User
and replaced 
it with a more abstract one, one of the interfaces was with
Hibernate, 
along with Open-Ejb, the asm stuff was a slight annoyance
sorting out.

---

Might be best to make the "security" a separate
package, removing the 
old User and replacing it with the Subject. The Realm and
all the other 
components stay outside of the core ftp server that way.

Perhaps start a new project/package

   ftp-security

with the base class of:

   org.apache.ftpserver.security

  to handle the Realms, configuration, and other related
matters.

FileSystemManager interface would change to be:

FileSystemView createFileSystemView(Subject subject) throws
FTPException

The above is likely one of the most important changes.  Most
of the 
others are really attributes or permissions and can be
attached to the 
user-session.

The User and UserManager can be removed completely from the
code.  No 
user reference would be needed for FtpSession{*}, so User
does not need 
to be replaced with Subject there.  The PASS, USER, SITE_WHO
command can 
access the Subject via the SecurityUtils call.  The
UserMetadata could 
go away, move the CertificateChain into the user session
using 
AuthenticationToken [... or maybe not ...].

{*} -- unless people don't like the singleton access, but
then this 
would start to tie the user stuff back into the core code.
Core code 
should focus on transport and actions, not authc/authz,
doing certs and 
managing users.

All the other odd-ball things like max idle time are moved
into the user 
session using the session.setAttribute( key, value) as these
are both 
Objects.

All of the classes in org.apache.ftpserver.usermanager can
go away. The 
Concurrent, write, and other classes are re-coded to match
the jsecurity 
versions of them [authz]. subject.isPermitted(Permission )
ie, 
Permission ConcurrentLogin ...

They use "authc" package names to designate
"authentication" and "authz" 
package names to designate "authorization". Just
noting this, makes it 
easier to scan down or through when looking for something.

I can take a first pass at this since I already ripped out
the existing 
User/Manager code. I would also like to use this, the faster
it's 
implemented and tested the better.

---

I think I saw a kitchen sink or two in the code too, looks
like they did 
not miss much.  If it's not supported directly, it's easy
enough to add 
it. I see they forgot the free beer, oh wait, we're suppose
to give that 
to them for the nice security package .


Andy


























Niklas Gustavsson wrote:
> On Tue, Jun 3, 2008 at 10:55 PM, Andy Thomson
<a10008051gmail.com> wrote:
>> Niklas,
>>
>> What do you think about making User more basic, ie,
more abstract, having
>> only one method such as getEnabled(). Then push
everything else up higher,
>> like the Authority[].  This way we can allow other
User-code styles that may
>> not use the those mappings or approaches.  In other
words keep the interface
>> very basic.
> 
> Not to detract from this discussion, but my plan is to
propose that we
> rip out our home-grown security design and replace it
with JSecurity
> (http://www.jsecurity.org
). The project is currently being voted for
> incubation at Apache and I think it would be a perfect
fit for
> FtpServer. In that case, the current User interface
would be replaced
> by the Subject interface:
> http://www.jsecurity.org/api/org/jsecurity/subjec
t/Subject.html
> 
> What's your thoughts on this?
> 
> /niklas
> 

On Mon, Jun 9, 2008 at 6:42 AM, Andy Thomson
<a10008051gmail.com> wrote:
> I can take a first pass at this since I already ripped
out the existing
> User/Manager code. I would also like to use this, the
faster it's
> implemented and tested the better.

Please do! If you can break it down into fairly small
patches that
would be great. Makes them much easier to review.

/niklas