From ea3595b6b60a983458481535a3a27fd44e96abb1 Mon Sep 17 00:00:00 2001 From: Juliana Corredor Date: Wed, 15 Aug 2012 18:01:41 -0500 Subject: [PATCH 1/3] IDEMPIERE-358 Login- how to make unique and safe - implement same for swing --- .../src/org/compiere/apps/ALogin.java | 218 ++++++++++-------- 1 file changed, 121 insertions(+), 97 deletions(-) diff --git a/org.adempiere.ui.swing/src/org/compiere/apps/ALogin.java b/org.adempiere.ui.swing/src/org/compiere/apps/ALogin.java index 8255566d0e..6fa830a646 100644 --- a/org.adempiere.ui.swing/src/org/compiere/apps/ALogin.java +++ b/org.adempiere.ui.swing/src/org/compiere/apps/ALogin.java @@ -275,22 +275,22 @@ public final class ALogin extends CDialog // DefaultTab defaultPanel.setLayout(defaultPanelLayout); // + clientLabel.setText("Client"); + clientLabel.setHorizontalAlignment(SwingConstants.RIGHT); + clientLabel.setLabelFor(clientCombo); + clientCombo.addActionListener(this); + defaultPanel.add(clientLabel, new GridBagConstraints(0, 0, 1, 1, 0.0, 0.0 + ,GridBagConstraints.EAST, GridBagConstraints.NONE, new Insets(12, 12, 5, 5), 0, 0)); + defaultPanel.add(clientCombo, new GridBagConstraints(1, 0, 1, 1, 1.0, 0.0 + ,GridBagConstraints.WEST, GridBagConstraints.HORIZONTAL, new Insets(12, 0, 5, 12), 0, 0)); roleLabel.setText("Role"); roleLabel.setHorizontalAlignment(SwingConstants.RIGHT); roleLabel.setLabelFor(roleCombo); roleCombo.addActionListener(this); - defaultPanel.add(roleLabel, new GridBagConstraints(0, 0, 1, 1, 0.0, 0.0 - ,GridBagConstraints.EAST, GridBagConstraints.NONE, new Insets(12, 12, 5, 5), 0, 0)); - defaultPanel.add(roleCombo, new GridBagConstraints(1, 0, 1, 1, 1.0, 0.0 - ,GridBagConstraints.WEST, GridBagConstraints.HORIZONTAL, new Insets(12, 0, 5, 12), 0, 0)); - clientLabel.setText("Client"); - clientLabel.setHorizontalAlignment(SwingConstants.RIGHT); - clientLabel.setLabelFor(clientCombo); - defaultPanel.add(clientLabel, new GridBagConstraints(0, 1, 1, 1, 0.0, 0.0 - ,GridBagConstraints.EAST, GridBagConstraints.NONE, new Insets(5, 12, 5, 5), 0, 0)); - clientCombo.addActionListener(this); - defaultPanel.add(clientCombo, new GridBagConstraints(1, 1, 1, 1, 1.0, 0.0 - ,GridBagConstraints.WEST, GridBagConstraints.HORIZONTAL, new Insets(5, 0, 5, 12), 0, 0)); + defaultPanel.add(roleLabel, new GridBagConstraints(0, 1, 1, 1, 0.0, 0.0 + ,GridBagConstraints.EAST, GridBagConstraints.NONE, new Insets(5, 12, 5, 5), 0, 0)); + defaultPanel.add(roleCombo, new GridBagConstraints(1, 1, 1, 1, 1.0, 0.0 + ,GridBagConstraints.WEST, GridBagConstraints.HORIZONTAL, new Insets(5, 0, 5, 12), 0, 0)); orgLabel.setText("Organization"); orgLabel.setHorizontalAlignment(SwingConstants.RIGHT); orgLabel.setLabelFor(orgCombo); @@ -500,10 +500,10 @@ public final class ALogin extends CDialog else if (e.getSource() == languageCombo) languageComboChanged(); // - else if (e.getSource() == roleCombo) - roleComboChanged(); else if (e.getSource() == clientCombo) clientComboChanged(); + else if (e.getSource() == roleCombo) + roleComboChanged(); else if (e.getSource() == orgCombo) orgComboChanged(); else if ("onlineLoginHelp".equals(e.getActionCommand())) @@ -645,13 +645,13 @@ public final class ALogin extends CDialog // Reference check Ini.setProperty(Ini.P_LOGMIGRATIONSCRIPT, "Reference".equalsIgnoreCase(CConnection.get().getDbUid())); - // Get Roles + // Get Clients m_login = new Login(m_ctx); - KeyNamePair[] roles = null; + KeyNamePair[] clients = null; try { - roles = m_login.getRoles(m_user, new String(m_pwd)); - if (roles == null || roles.length == 0) + clients = m_login.getClients(m_user, new String(m_pwd)); + if (clients == null || clients.length == 0) { statusBar.setStatusLine(txt_UserPwdError, true); userTextField.setBackground(AdempierePLAF.getFieldBackground_Error()); @@ -679,11 +679,85 @@ public final class ALogin extends CDialog // Delete existing role items m_comboActive = true; - if (roleCombo.getItemCount() > 0) - roleCombo.removeAllItems(); + if (clientCombo.getItemCount() > 0) + clientCombo.removeAllItems(); // Initial role KeyNamePair iniValue = null; + String iniDefault = Ini.getProperty(Ini.P_CLIENT); + + // fill roles + for (int i = 0; i < clients.length; i++) + { + clientCombo.addItem(clients[i]); + if (clients[i].getName().equals(iniDefault)) + iniValue = clients[i]; + } + if (iniValue != null){ + clientCombo.setSelectedItem(iniValue); + } else { + clientCombo.setSelectedItem(clients[0]); + } + + + if (clientCombo.getItemCount() == 1) + { + clientCombo.setSelectedIndex(0); + clientCombo.setVisible(false); + clientCombo.setVisible(false); + clientLabel.setVisible(false); + } + else + { + clientCombo.setVisible(true); + clientCombo.setVisible(true); + } + + userTextField.setBackground(AdempierePLAF.getFieldBackground_Normal()); + passwordField.setBackground(AdempierePLAF.getFieldBackground_Normal()); + // + this.setTitle(hostField.getDisplay()); + statusBar.setStatusLine(txt_LoggedIn); + m_comboActive = false; + clientComboChanged(); + return true; + } // tryConnection + + + /** + * Client changed - fill Role List + */ + private void clientComboChanged () + { + KeyNamePair client = (KeyNamePair)clientCombo.getSelectedItem(); + if (client == null || m_comboActive) + return; + log.config(": " + client); + m_comboActive = true; + // @Trifon - Set Proper "#AD_Client_ID", #AD_User_ID and "#SalesRep_ID" + // https://sourceforge.net/tracker/?func=detail&aid=2957215&group_id=176962&atid=879332 + Env.setContext(m_ctx, "#AD_Client_ID", client.getKey()); + MUser user = MUser.get (m_ctx, userTextField.getText()); + if (user != null) { + Env.setContext(m_ctx, "#AD_User_ID", user.getAD_User_ID() ); + Env.setContext(m_ctx, "#SalesRep_ID", user.getAD_User_ID() ); + } + // + KeyNamePair[] roles = m_login.getRoles(userTextField.getText(), client); + // delete existing rol/org items + if (roleCombo.getItemCount() > 0) + roleCombo.removeAllItems(); + if (orgCombo.getItemCount() > 0) + orgCombo.removeAllItems(); + // No Clients + if (roles == null || roles.length == 0) + { + statusBar.setStatusLine(txt_RoleError, true); + m_comboActive = false; + return; + } + // initial rol + KeyNamePair iniValue = null; String iniDefault = Ini.getProperty(Ini.P_ROLE); // fill roles @@ -693,97 +767,47 @@ public final class ALogin extends CDialog if (roles[i].getName().equals(iniDefault)) iniValue = roles[i]; } - if (iniValue != null) - roleCombo.setSelectedItem(iniValue); - - // If we have only one role, we can hide the combobox - metas-2009_0021_AP1_G94 - if (roleCombo.getItemCount() == 1 && ! MSysConfig.getBooleanValue(MSysConfig.ALogin_ShowOneRole, true)) - { - roleCombo.setSelectedIndex(0); - roleLabel.setVisible(false); - roleCombo.setVisible(false); - } - else - { - roleLabel.setVisible(true); - roleCombo.setVisible(true); - } - - userTextField.setBackground(AdempierePLAF.getFieldBackground_Normal()); - passwordField.setBackground(AdempierePLAF.getFieldBackground_Normal()); - // - this.setTitle(hostField.getDisplay()); - statusBar.setStatusLine(txt_LoggedIn); - m_comboActive = false; - roleComboChanged(); - return true; - } // tryConnection - - - /** - * Role changed - fill Client List - */ - private void roleComboChanged () - { - KeyNamePair role = (KeyNamePair)roleCombo.getSelectedItem(); - if (role == null || m_comboActive) - return; - log.config(": " + role); - m_comboActive = true; - // - KeyNamePair[] clients = m_login.getClients(role); - // delete existing client/org items - if (clientCombo.getItemCount() > 0) - clientCombo.removeAllItems(); - if (orgCombo.getItemCount() > 0) - orgCombo.removeAllItems(); - // No Clients - if (clients == null || clients.length == 0) - { - statusBar.setStatusLine(txt_RoleError, true); - m_comboActive = false; - return; - } - // initial client - KeyNamePair iniValue = null; - String iniDefault = Ini.getProperty(Ini.P_CLIENT); - - // fill clients - for (int i = 0; i < clients.length; i++) - { - clientCombo.addItem(clients[i]); - if (clients[i].getName().equals(iniDefault)) - iniValue = clients[i]; - } // fini if (iniValue != null) - clientCombo.setSelectedItem(iniValue); + roleCombo.setSelectedItem(iniValue); // + // If we have only one role, we can hide the combobox - metas-2009_0021_AP1_G94 + if(roleCombo.getItemCount()==1 && ! MSysConfig.getBooleanValue(MSysConfig.ALogin_ShowOneRole, true)) + { + roleCombo.setSelectedIndex(0); + roleCombo.setVisible(false); + roleCombo.setVisible(false); + roleLabel.setVisible(false); + + } + m_comboActive = false; - clientComboChanged(); + roleComboChanged(); } // roleComboChanged /** - * Client changed - fill Org & Warehouse List + * role changed - fill Org & Warehouse List */ - private void clientComboChanged() + private void roleComboChanged() { - KeyNamePair client = (KeyNamePair)clientCombo.getSelectedItem(); - if (client == null || m_comboActive) + KeyNamePair rol = (KeyNamePair)roleCombo.getSelectedItem(); + if (rol == null || m_comboActive) return; - log.config(": " + client); + log.config(": " + rol); m_comboActive = true; - // @Trifon - Set Proper "#AD_Client_ID", #AD_User_ID and "#SalesRep_ID" - // https://sourceforge.net/tracker/?func=detail&aid=2957215&group_id=176962&atid=879332 - Env.setContext(m_ctx, "#AD_Client_ID", client.getKey()); - MUser user = MUser.get (m_ctx, userTextField.getText(), new String (passwordField.getPassword()) ); - if (user != null) { - Env.setContext(m_ctx, "#AD_User_ID", user.getAD_User_ID() ); - Env.setContext(m_ctx, "#SalesRep_ID", user.getAD_User_ID() ); + + if( Env.getContextAsInt(m_ctx, "#AD_Client_ID") > 0 ) + { + MUser user = MUser.get (m_ctx, userTextField.getText()); + if (user != null) { + Env.setContext(m_ctx, "#AD_User_ID", user.getAD_User_ID() ); + Env.setContext(m_ctx, "#SalesRep_ID", user.getAD_User_ID() ); + } } + // - KeyNamePair[] orgs = m_login.getOrgs(client); + KeyNamePair[] orgs = m_login.getOrgs(rol); // delete existing cleint items if (orgCombo.getItemCount() > 0) orgCombo.removeAllItems(); From e0b518164e963075e0164ebe03ef5616f2c36e02 Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Wed, 15 Aug 2012 18:58:53 -0500 Subject: [PATCH 2/3] IDEMPIERE-177 Complete Window Customization functionality / found issues with cache vs the window customization implementation --- .../org/adempiere/webui/AdempiereWebUI.java | 5 +---- .../src/org/adempiere/webui/apps/AEnv.java | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/AdempiereWebUI.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/AdempiereWebUI.java index 6b395ec42d..81ba7d7f17 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/AdempiereWebUI.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/AdempiereWebUI.java @@ -333,10 +333,7 @@ public class AdempiereWebUI extends Window implements EventListener, IWeb appDesktop.logout(); Executions.getCurrent().getDesktop().getSession().getAttributes().clear(); - MSession mSession = MSession.get(Env.getCtx(), false); - if (mSession != null) { - mSession.logout(); - } + AEnv.logout(); SessionManager.clearSession(); super.getChildren().clear(); diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/AEnv.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/AEnv.java index b698d361bd..72c7a90ce6 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/AEnv.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/AEnv.java @@ -49,6 +49,7 @@ import org.compiere.model.Lookup; import org.compiere.model.MAcctSchema; import org.compiere.model.MLookup; import org.compiere.model.MQuery; +import org.compiere.model.MSession; import org.compiere.util.CCache; import org.compiere.util.CLogger; import org.compiere.util.CacheMgt; @@ -189,9 +190,13 @@ public final class AEnv public static void logout() { - Env.logout(); - - + String sessionID = Env.getContext(Env.getCtx(), "#AD_Session_ID"); + windowCache.remove(sessionID); + // End Session + MSession session = MSession.get(Env.getCtx(), false); // finish + if (session != null) + session.logout(); + // } /** @@ -240,12 +245,12 @@ public final class AEnv log.config("Window=" + WindowNo + ", AD_Window_ID=" + AD_Window_ID); GridWindowVO mWindowVO = null; - String locale = Env.getLanguage(Env.getCtx()).getLocale().toString(); + String sessionID = Env.getContext(Env.getCtx(), "#AD_Session_ID"); if (AD_Window_ID != 0 && Ini.isCacheWindow()) // try cache { synchronized (windowCache) { - CCache cache = windowCache.get(locale); + CCache cache = windowCache.get(sessionID); if (cache != null) { mWindowVO = cache.get(AD_Window_ID); @@ -267,11 +272,11 @@ public final class AEnv { synchronized (windowCache) { - CCache cache = windowCache.get(locale); + CCache cache = windowCache.get(sessionID); if (cache == null) { cache = new CCache("AD_Window", 10); - windowCache.put(locale, cache); + windowCache.put(sessionID, cache); } cache.put(AD_Window_ID, mWindowVO); } From ca94ad667005bd756c02547299082fffe8282581 Mon Sep 17 00:00:00 2001 From: Juliana Corredor Date: Thu, 16 Aug 2012 15:34:37 -0500 Subject: [PATCH 3/3] IDEMPIERE-358 Login- how to make unique and safe - implement email login for monitor --- .../src/org/compiere/model/MUser.java | 121 ++++++++---------- 1 file changed, 52 insertions(+), 69 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/model/MUser.java b/org.adempiere.base/src/org/compiere/model/MUser.java index 1c71102eaa..964247d077 100644 --- a/org.adempiere.base/src/org/compiere/model/MUser.java +++ b/org.adempiere.base/src/org/compiere/model/MUser.java @@ -36,6 +36,7 @@ import org.compiere.util.CCache; import org.compiere.util.CLogger; import org.compiere.util.DB; import org.compiere.util.Env; +import org.compiere.util.KeyNamePair; import org.compiere.util.Msg; import org.compiere.util.Secure; import org.compiere.util.SecureEngine; @@ -173,80 +174,62 @@ public class MUser extends X_AD_User return null; } boolean hash_password = MSysConfig.getBooleanValue(MSysConfig.USER_PASSWORD_HASH, false); + boolean email_login = MSysConfig.getBooleanValue(MSysConfig.USE_EMAIL_FOR_LOGIN, false); + ArrayList clientList = new ArrayList(); + ArrayList clientsValidated = new ArrayList(); MUser retValue = null; - if (!hash_password) - { - int AD_Client_ID = Env.getAD_Client_ID(ctx); - - - String sql = "SELECT * FROM AD_User " - + "WHERE COALESCE(LDAPUser, Name)=? " // #1 - + " AND ((Password=? AND (SELECT IsEncrypted FROM AD_Column WHERE AD_Column_ID=417)='N') " // #2 - + "OR (Password=? AND (SELECT IsEncrypted FROM AD_Column WHERE AD_Column_ID=417)='Y'))" // #3 - + " AND IsActive='Y' AND AD_Client_ID=?" // #4 - ; - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - pstmt.setString (1, name); - pstmt.setString (2, password); - pstmt.setString (3, SecureEngine.encrypt(password)); - pstmt.setInt(4, AD_Client_ID); - rs = pstmt.executeQuery (); - if (rs.next ()) - { - retValue = new MUser (ctx, rs, null); - if (rs.next()) - s_log.warning ("More then one user with Name/Password = " + name); - } - else - s_log.fine("No record"); + + StringBuffer where = new StringBuffer("Password IS NOT NULL AND "); + if (email_login) + where.append("EMail=?"); + else + where.append("COALESCE(LDAPUser,Name)=?"); + where.append(" AND") + .append(" EXISTS (SELECT * FROM AD_User_Roles ur") + .append(" INNER JOIN AD_Role r ON (ur.AD_Role_ID=r.AD_Role_ID)") + .append(" WHERE ur.AD_User_ID=AD_User.AD_User_ID AND ur.IsActive='Y' AND r.IsActive='Y') AND ") + .append(" EXISTS (SELECT * FROM AD_Client c") + .append(" WHERE c.AD_Client_ID=AD_User.AD_Client_ID") + .append(" AND c.IsActive='Y') AND ") + .append(" AD_User.IsActive='Y'"); + + List users = new Query(ctx, MUser.Table_Name, where.toString(), null) + .setParameters(name) + .setOrderBy(MUser.COLUMNNAME_AD_User_ID) + .list(); + + if (users.size() == 0) { + s_log.saveError("UserPwdError", name, false); + return null; + } + + for (MUser user : users) { + if (clientsValidated.contains(user.getAD_Client_ID())) { + s_log.severe("Two users with password with the same name/email combination on same tenant: " + name); + return null; } - catch (Exception e) - { - s_log.log(Level.SEVERE, sql, e); + + clientsValidated.add(user.getAD_Client_ID()); + boolean valid = false; + if (hash_password) { + String hash = user.getPassword(); + String salt = user.getSalt(); + // always do calculation to confuse timing based attacks + if ( hash == null ) + hash = "0000000000000000"; + if ( salt == null ) + salt = "0000000000000000"; + valid = user.authenticateHash(password); + } else { + // password not hashed + valid = user.getPassword().equals(password); } - finally - { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } - } else { - String where = " COALESCE(LDAPUser,Name) = ? AND" + - " EXISTS (SELECT * FROM AD_User_Roles ur" + - " INNER JOIN AD_Role r ON (ur.AD_Role_ID=r.AD_Role_ID)" + - " WHERE ur.AD_User_ID=AD_User.AD_User_ID AND ur.IsActive='Y' AND r.IsActive='Y') AND " + - " EXISTS (SELECT * FROM AD_Client c" + - " WHERE c.AD_Client_ID=AD_User.AD_Client_ID" + - " AND c.IsActive='Y') AND " + - " AD_User.IsActive='Y'"; - - MUser user = MTable.get(ctx, MUser.Table_ID).createQuery( where, null).setParameters(name).firstOnly(); // throws error if username collision occurs - - String hash = null; - String salt = null; - - if (user != null ) - { - hash = user.getPassword(); - salt = user.getSalt(); - } - - // always do calculation to confuse timing based attacks - if ( user == null ) - user = MUser.get(ctx, 0); - if ( hash == null ) - hash = "0000000000000000"; - if ( salt == null ) - salt = "0000000000000000"; - - if ( user.authenticateHash(password) ) - { + + if (valid){ retValue=user; } - } + } + return retValue; } // get