Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/308#issuecomment-106818248
Hi Rohit, thank you for your view on this :)
Here's my comment on it:
1. No use on protecting the sessionkey from MITM attacks, as the
username/password are sent in plain text, and so are all the other commands, so
anyone can pickup your credentials if they're listening on the wire ;). For
MITM protection SSL should be used. I did notice that
isSecureSessionCookieEnabled i actually placed the cookie in raw to be exactly
the same as the sessioncookie. I did use the cookie class for reading though
:). So.. both cookies should be inserted in the same fashion.
2. Will take care of that :)
3. I miss the purpose.. session cleanup? Can you explain a bit more? :)
4. If you don't return it in the loginResponse, regular (non-browser) calls
to the API will fail, because i guess all integrations with the API depend on
it, that's why i didn't remove it. It can be in the response, as long as
javascript doesn't store it for later access.
Regarding the non-bulleted stuff:
Secure cookie:
I don't agree with this one, that will make it just not work at all in
HTTP. We aim to fix a browser vulnerability here, not a communication channel
one. If someone wants on the wire security, they should just switch to SSL.
Not depend on sessionkey as API argument:
Will that not break some API implementations? I think supporting both
methods is the best for now.
About SAML and redirect based auth not working:
Are you sure? Did you test? I left it in a way it would likely work...
cookie is still read an put into the javascript variable if it was created on
redirection. Since the SAML plugin just puts it back in a cookie so i didn't
bother destroying it in that scenario, the plugin should be modded for that.
Either way.. it *should* work :)
Thanks for reviewing.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
