Ravi Nori has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 71: (5 comments) https://gerrit.ovirt.org/#/c/36119/71/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/OAuthTokenServlet.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/OAuthTokenServlet.java: Line 41: break; Line 42: case "password": Line 43: issueTokenForPasswd(request, response); Line 44: break; Line 45: case "": > still empty. Done Line 46: String redirectUri = SSOUtils.getRequestParameter(request, SSOUtils.REDIRECT_URI, SSOUtils.REDIRECT_URI); Line 47: issueTokenUsingHttpHeaders(request, response, redirectUri); Line 48: break; Line 49: default: Line 96: throw new AuthenticationException(String.format("Cannot authenticate user '%s': %s", credentials == null ? "N/A" : credentials.getUsername(), ex.getMessage())); Line 97: } Line 98: } Line 99: Line 100: private static void issueTokenUsingHttpHeaders(HttpServletRequest request, HttpServletResponse response, String redirectUri) throws Exception { > this should be iterative, similar to what we have in nego. calling all exte ExternalAuthUtils.doAuth already loops through the list of extensions until authenticated. Do we still need another loop here? Line 101: log.debug("Entered issueTokenUsingHttpHeaders"); Line 102: Credentials credentials = null; Line 103: try { Line 104: String code = ExternalAuthUtils.doAuth(request, response, false); https://gerrit.ovirt.org/#/c/36119/71/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/DBUtils.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/DBUtils.java: Line 36: Connection connection = ds.getConnection(); Line 37: PreparedStatement ps = connection.prepareStatement(sql); Line 38: ) { Line 39: try (ResultSet rs = ps.executeQuery();) { Line 40: PasswordStore ph = new PasswordStore("PBKDF2WithHmacSHA1", 256, 2000, null); > all these should be parameters that can be customized. Done Line 41: while (rs.next()) { Line 42: String clientId = rs.getString("client_id"); Line 43: Map<String, String> clientInfo = new HashMap<>(); Line 44: clientInfo.put(SSOUtils.CLIENT_SECRET, ph.encode(rs.getString("client_secret"))); Line 40: PasswordStore ph = new PasswordStore("PBKDF2WithHmacSHA1", 256, 2000, null); Line 41: while (rs.next()) { Line 42: String clientId = rs.getString("client_id"); Line 43: Map<String, String> clientInfo = new HashMap<>(); Line 44: clientInfo.put(SSOUtils.CLIENT_SECRET, ph.encode(rs.getString("client_secret"))); > not sure I understand... it should be the opposite. we need to discuss, will ping you Line 45: clientInfo.put(SSOUtils.SCOPE, rs.getString("scope")); Line 46: clientInfo.put(SSOUtils.TRUSTED, ((Boolean) rs.getBoolean("trusted")).toString()); Line 47: map.put(clientId, clientInfo); Line 48: } https://gerrit.ovirt.org/#/c/36119/71/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/TokenCleanupUtility.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/TokenCleanupUtility.java: Line 20: return; Line 21: } Line 22: lastCleanup = currentTime; Line 23: log.debug("Cleaning up expired tokens"); Line 24: Map<String, Map> sessionData = ((Map<String, Map>) ctx.getAttribute(SSOUtils.SSO_SESSION_DATA)); > should be this concurrent map to enable that or should we sync? Done Line 25: Line 26: for (Map.Entry<String, Map> entry : sessionData.entrySet()) { Line 27: if ((currentTime - (long) entry.getValue().get(SSOUtils.TOKEN_LAST_ACCESS)) > ssoConfig.getTokenTimeout()) { Line 28: log.debug("Cleaning up expired token for user: {}, last accessed {}", entry.getValue().get(SSOUtils.USER_ID), entry.getValue().get(SSOUtils.TOKEN_LAST_ACCESS)); -- To view, visit https://gerrit.ovirt.org/36119 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4894fc12653027271b6abd4dd5313b10593703fa Gerrit-PatchSet: 71 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
