FtpServer
  1. FtpServer
  2. FTPSERVER-235

Documentation and code do not match for db user manager

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0.0-M3, 1.0.0-M4
    • Fix Version/s: 1.0.0-RC1
    • Component/s: Core
    • Labels:
      None

      Description

      In the examples on the website(http://cwiki.apache.org/FTPSERVER/database-user-manager.html) it shows:

      <authenticate>SELECT uid from FTP_USER WHERE uid='

      {uid}

      ' AND
      userpassword='

      {userpassword}

      '</authenticate>

      (uid is wrong, is actually userid in all three places)

      but the code will never set userpassword

      in DbUserManager.authenticate

      it does

      HashMap<String, Object> map = new HashMap<String, Object>();
      map.put(ATTR_LOGIN, escapeString(user));
      String sql = StringUtils.replaceString(authenticateStmt, map);
      LOG.info(sql);

      and after it compares the stored password with the one the user entered.

      is this designed to be this way or the way described in the documentation, i think allowing it the way it is in the documentation allows for greater flexibility.

      if it is not a bug and is a design feature I will make a custom user manager.

      a fix that would match the documentation would be

      public User authenticate(Authentication authentication) throws AuthenticationFailedException {
      if (authentication instanceof UsernamePasswordAuthentication) {
      UsernamePasswordAuthentication upauth = (UsernamePasswordAuthentication) authentication;
      String user = upauth.getUsername();
      String password = upauth.getPassword();
      if (user == null)

      { throw new AuthenticationFailedException("Authentication failed"); }

      if (password == null)

      { password = ""; }

      Statement stmt = null;
      ResultSet rs = null;
      try {
      // create the sql query
      HashMap<String, Object> map = new HashMap<String, Object>();
      map.put(ATTR_LOGIN, escapeString(user));
      map.put(ATTR_PASSWORD, escapeString(password));
      String sql = StringUtils.replaceString(authenticateStmt, map);
      LOG.info(sql);
      // execute query
      stmt = createConnection().createStatement();
      rs = stmt.executeQuery(sql);
      if (rs.next()) {
      try

      { return getUserByName(user); }

      catch (FtpException e)

      { throw new AuthenticationFailedException("Authentication failed", e); }

      } else

      { throw new AuthenticationFailedException("Authentication failed"); }
      } catch (SQLException ex) { LOG.error("DbUserManager.authenticate()", ex); throw new AuthenticationFailedException("Authentication failed", ex); } finally { closeQuitely(rs); closeQuitely(stmt); }
      } else if (authentication instanceof AnonymousAuthentication) {
      try {
      if (doesExist("anonymous")) { return getUserByName("anonymous"); } else { throw new AuthenticationFailedException("Authentication failed"); }

      } catch (AuthenticationFailedException e)

      { throw e; }

      catch (FtpException e)

      { throw new AuthenticationFailedException("Authentication failed", e); }

      } else

      { throw new IllegalArgumentException("Authentication not supported by this user manager"); }

      }

        Activity

        nathan longley created issue -
        Niklas Gustavsson made changes -
        Field Original Value New Value
        Assignee Niklas Gustavsson [ niklas ]
        Fix Version/s 1.0.0-M4 [ 12313395 ]
        Niklas Gustavsson made changes -
        Fix Version/s 1.0.0-RC1 [ 12313542 ]
        Affects Version/s 1.0.0-M4 [ 12313395 ]
        Fix Version/s 1.0.0-M4 [ 12313395 ]
        Hide
        Niklas Gustavsson added a comment -

        I fixed the uid->userid in the documentation (this was missed when we changed the column name due to problems in Oracle). I also changed the documentation for the SQL statement for the authentication statement. The reason this works as it does is that we can not know the salt before we have done the select. Also, since the password encryptor is pluggable, we leave it to its implementation to verify the password, rather then the database.

        Show
        Niklas Gustavsson added a comment - I fixed the uid->userid in the documentation (this was missed when we changed the column name due to problems in Oracle). I also changed the documentation for the SQL statement for the authentication statement. The reason this works as it does is that we can not know the salt before we have done the select. Also, since the password encryptor is pluggable, we leave it to its implementation to verify the password, rather then the database.
        Niklas Gustavsson made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Closed [ 6 ]

          People

          • Assignee:
            Niklas Gustavsson
            Reporter:
            nathan longley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development