Alon Bar-Lev has posted comments on this change. Change subject: aaa : Add engine sso ......................................................................
Patch Set 71: (4 comments) looks really good! 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. 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: 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. 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. you should store the result in database and compare it to the result on the incoming. 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? 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 <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
