Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/308#issuecomment-106800322
Hi @rsafonseca thanks for the update and patch. It's a good idea to use
HTTPOnly flag on cookie to ensure we have the authentication going on and at
the same time not expose the cookie value to javascript. But the current patch
will break SAML and any redirected authentication which relies on cookies being
read by JS and then destroyed. As an alternative, let's discuss how we can
improve this:
1. When a user is authenticated, we set the sessionkey cookie (httponly).
This btw, only protects it from being read by JavaScript and not MITM attacks.
For this, we can use a global setting I had added (isSecureSessionCookieEnabled
method in ApiServer can be used for this) to enable secure flag on it as well.
Please use Cookie class for this instead of adding raw strings like:
resp.addCookie(new cookie(with flags etc enabled.)
2. In HttpUtils, add helper method:
+ public static String findCookie(final Cookie[] cookies, String key) {
+ for (Cookie cookie: cookies) {
+ if (cookie.getName().equals(key)) {
+ return cookie.getValue();
+ }
+ }
+ return null;
+ }
3. In ApiServlet, use the above method to find the cookie value (if
available) when there is no sessionkey or empty sessionkey passed in the API
params, check against session's sessionkey value to invalidate session.
4. When users use default login method (login command), the login response
still has the sessionkey. So, we can discuss to remove this as a security
measure as well. If users are using an external logging system such as SAML
based, usually the plugin would set the cookie. If the cookie becomes httponly
(so JavaScript won't be able to read it), we will need to fix the JavaScript
code such that it does not rely on sessionkey parameter at all.
Please review the above ideas and comment/share on them.
TLDR? To fix the login/UI issue, I think we need to not depend on passing a
sessionkey as API argument but instead use HTTPOnly (and enable secure flag, so
cookies are passed on HTTPs if enabled) "sessionkey" cookie along with the
JSESSIONID cookie. Doing this would make sure we can run ACS UI across tabs and
let the UI work when a external auth system such as those based on SAML,
2-factor or OAuth can work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
