Hi, ftpserver-dev,
New patch variant is in attachment. No credentials class. Only replacing BaseUser with User from UserManager and replcaing fake BaseUser between USER and PASS with tmpUserName in FTPRequest.
Sergey
2006/5/5, Sergey Vladimirov <[EMAIL PROTECTED]>:
Anton, Rana,It is possible that I miss this point from Rana letter, so I don't quite understand what is his viewpoint about.But anyway, with current implementation FileSystemManager will must expect User to be instance of BaseUser. So, I agree, that such checks should have place, but I want to get FileSystemManager chanses to have application-specific User classes.
"Dividing the credentials (as a term, not a class) into User and Credentials where they are used separately, depending on implementation and the state the request is at doesn't sound as a good idea to me."I don't fully understand the idea of using Credentials for anything, except storing username between USER and PASS command. Ok, ok, I can just replace Credentials class with tmpUserName field, if it is more understandable. I never said that Credentials should be used instead of User anywere, except USER & PASS commands.
Implementing JAAS is intresting, but it is more complex for simple FTPServer. And I don't remember what is license terms of jaas.jar from Sun(r).Sergey2006/5/5, Anton Goldberg <[EMAIL PROTECTED]>:Hi,
I agree with the Rana's viewpoint from the previous letter. If any
component depends upon specific implementation/functionality of User
implementation, it's a configuration problem. FtpServer implementation
in such a case must be configured to use an appropriate UserManager
implementation and the component in question (FileSystemManager)
should check the class of User and take an action if the User is not
what it expected (fail fast).
Dividing the credentials (as a term, not a class) into User and
Credentials where they are used separately, depending on
implementation and the state the request is at doesn't sound as a good
idea to me.
In general, if the general opinion is that security implementation
needs to be more standard/powerful/complex we should consider
implementing JAAS or more modern frameworks.
P.S. Chances are this email will not show up in the list bcs for some
reason all emails from me to the list are going to the big hard drive
in the sky. I'm working on a solution for this problem (sending emails
to apache@ and infrastructure@) but that's the state things are right
now.
--
Anton
--
Sergey Vladimirov
--
Sergey Vladimirov
Index:
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
===================================================================
---
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
(revision 399733)
+++
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
(working copy)
@@ -26,12 +26,8 @@
import java.util.zip.DeflaterOutputStream;
import java.util.zip.InflaterInputStream;
-import org.apache.ftpserver.ftplet.FileObject;
-import org.apache.ftpserver.ftplet.FileSystemView;
-import org.apache.ftpserver.ftplet.FtpRequest;
-import org.apache.ftpserver.ftplet.User;
+import org.apache.ftpserver.ftplet.*;
import org.apache.ftpserver.interfaces.ConnectionObserver;
-import org.apache.ftpserver.usermanager.BaseUser;
/**
* FTP request object.
@@ -45,6 +41,10 @@
private String command;
private String argument;
+ /**
+ * Contains user name between USER and PASS commands
+ */
+ private String tmpUserName;
private User user;
private HashMap attributeMap;
private InetAddress remoteAddr;
@@ -66,7 +66,8 @@
*/
public FtpRequestImpl() {
attributeMap = new HashMap();
- user = new BaseUser();
+ tmpUserName = null;
+ user = null;
connectionTime = System.currentTimeMillis();
}
@@ -103,7 +104,8 @@
* Reinitialize request.
*/
public void reinitialize() {
- user = new BaseUser();
+ tmpUserName = null;
+ user = null;
loginTime = 0L;
fileSystemView = null;
renameFrom = null;
@@ -240,8 +242,24 @@
public void setRenameFrom(FileObject file) {
renameFrom = file;
}
-
+
/**
+ * Returns user name entered in USER command
+ *
+ * @return user name entered in USER command
+ */
+ public String getTmpUserName() {
+ return tmpUserName;
+ }
+
+ /**
+ * Set user name entered from USER command
+ */
+ public void setTmpUserName(String tmpUserName) {
+ this.tmpUserName = tmpUserName;
+ }
+
+ /**
* Get the ftp command.
*/
public String getCommand() {
@@ -290,6 +308,10 @@
return user;
}
+ public void setUser(User user) {
+ this.user = user;
+ }
+
/**
* Get remote address
*/
Index:
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
===================================================================
---
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
(revision 399733)
+++
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
(working copy)
@@ -23,18 +23,11 @@
import org.apache.ftpserver.FtpRequestImpl;
import org.apache.ftpserver.FtpWriter;
import org.apache.ftpserver.RequestHandler;
-import org.apache.ftpserver.ftplet.FileSystemManager;
-import org.apache.ftpserver.ftplet.FileSystemView;
-import org.apache.ftpserver.ftplet.FtpException;
-import org.apache.ftpserver.ftplet.Ftplet;
-import org.apache.ftpserver.ftplet.FtpletEnum;
-import org.apache.ftpserver.ftplet.User;
-import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.ftplet.*;
import org.apache.ftpserver.interfaces.ICommand;
import org.apache.ftpserver.interfaces.IConnectionManager;
import org.apache.ftpserver.interfaces.IFtpConfig;
import org.apache.ftpserver.interfaces.IFtpStatistics;
-import org.apache.ftpserver.usermanager.BaseUser;
/**
* <code>PASS <SP> <password> <CRLF></code><br>
@@ -73,8 +66,7 @@
}
// check user name
- User user = request.getUser();
- String userName = user.getName();
+ String userName = request.getTmpUserName();
if(userName == null) {
out.send(503, "PASS", null);
return;
@@ -106,36 +98,32 @@
// authenticate user
UserManager userManager = fconfig.getUserManager();
+ User user = null;
try {
if(bAnonymous) {
bSuccess = userManager.doesExist("anonymous");
+ if (bSuccess)
+ user = userManager.getUserByName("anonymous");
}
else {
bSuccess = userManager.authenticate(userName, password);
+ if (bSuccess)
+ user = userManager.getUserByName(userName);
}
}
catch(Exception ex) {
bSuccess = false;
log.warn("PASS.execute()", ex);
}
- if(!bSuccess) {
+ if(bSuccess) {
+ request.setUser(user);
+ request.setTmpUserName(null);
+ } else {
log.warn("Login failure - " + userName);
out.send(530, "PASS", userName);
return;
}
- // populate user information
- try {
- populateUser(handler, userName);
- }
- catch(FtpException ex) {
- bSuccess = false;
- }
- if(!bSuccess) {
- out.send(530, "PASS", userName);
- return;
- }
-
// everything is fine - send login ok message
out.send(230, "PASS", userName);
if(bAnonymous) {
@@ -168,25 +156,4 @@
}
}
}
-
- /**
- * Populate user.
- */
- private void populateUser(RequestHandler handler,
- String login) throws FtpException {
-
- User user = handler.getConfig().getUserManager().getUserByName(login);
- if(user == null) {
- throw new FtpException(login + " : does not exist.");
- }
-
- BaseUser reqUser = (BaseUser)handler.getRequest().getUser();
- reqUser.setName(user.getName());
- reqUser.setEnabled(user.getEnabled());
- reqUser.setHomeDirectory(user.getHomeDirectory());
- reqUser.setMaxUploadRate(user.getMaxUploadRate());
- reqUser.setMaxDownloadRate(user.getMaxDownloadRate());
- reqUser.setMaxIdleTime(user.getMaxIdleTime());
- reqUser.setWritePermission(user.getWritePermission());
- }
}
Index:
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/USER.java
===================================================================
---
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/USER.java
(revision 399733)
+++
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/USER.java
(working copy)
@@ -21,12 +21,11 @@
import org.apache.ftpserver.FtpRequestImpl;
import org.apache.ftpserver.FtpWriter;
import org.apache.ftpserver.RequestHandler;
-import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.User;
import org.apache.ftpserver.interfaces.ICommand;
import org.apache.ftpserver.interfaces.IConnectionManager;
import org.apache.ftpserver.interfaces.IFtpConfig;
import org.apache.ftpserver.interfaces.IFtpStatistics;
-import org.apache.ftpserver.usermanager.BaseUser;
/**
* <code>USER <SP> <username> <CRLF></code><br>
@@ -47,7 +46,7 @@
*/
public void execute(RequestHandler handler,
FtpRequestImpl request,
- FtpWriter out) throws IOException, FtpException {
+ FtpWriter out) throws IOException {
boolean bSuccess = false;
IFtpConfig fconfig = handler.getConfig();
@@ -66,8 +65,8 @@
}
// already logged-in
- BaseUser user = (BaseUser)request.getUser();
if(request.isLoggedIn()) {
+ User user = request.getUser();
if( userName.equals(user.getName()) ) {
out.send(230, "USER", null);
bSuccess = true;
@@ -103,7 +102,7 @@
// finally set the user name
bSuccess = true;
- user.setName(userName);
+ request.setTmpUserName(userName);
if(bAnonymous) {
out.send(331, "USER.anonymous", userName);
}
