[Kimchi-devel] [PATCH V5 0/5] Switch to a traditional login flow

[Yu Xin Huo]

On 6/16/2014 11:33 AM, Zhou Zheng Sheng wrote:
> on 2014/06/13 18:47, Yu Xin Huo wrote:
>> On 6/13/2014 6:07 PM, Zhou Zheng Sheng wrote:
>>> on 2014/06/13 17:57, Yu Xin Huo wrote:
>>>> On 6/13/2014 5:45 PM, Zhou Zheng Sheng wrote:
>>>>> So your point is the proposed alternatives is uglier than the current
>>>>> one, and the current one can be improved.
>>>>>
>>>>> Firstly from the JS accessing browser history link you gave me, I don't
>>>>> see anywhere it says it's unreliable. The solution doesn't need to
>>>>> inspect the history, it just back needs to go back.
>>>>>
>>>>> Secondly you think "refresh.html" is uglier than cookie. It's actually
>>>>> not. A "refresh.html" only affects the login process, but the current
>>>>> cookie solution affects all requests and responses that is not related
>>>>> to login. The current solution works, but it is far from works well,
>>>>> and
>>>>> if you want to improve it by suppress sending the previous page cookie
>>>>> in AJAX requests and response, more hacks would be added in both
>>>>> front-end and back-end. This is not improving step by step. To improve
>>>>> step by step, the original direction should be correct and effective,
>>>>> but this is not.
>>>> Again, there will always cookie back and forth between browser and web
>>>> container. It is handled automatically by browser and web container.
>>>> What make you uncomfortable is that piece of code at server side to get
>>>> previousPage.
>>> Yes, but the previousPage cookie is unnecessary regardless if it's
>>> handled automatically or not. The alternative solutions we discussed can
>>> avoid this unnecessary handling, and restrict the login handling logic
>>> only to a small region of code and runtime process.
>> there are 2 alternative solutions you proposed:
>>
>> 1. brower history
>>                  -- unreliable as I replied.
> 1. You are correct
>
>> 2. internal redirect
>>                  -- 1. make browser url mismatch its content, it is
>> desirable to make browser url truly address it intended content.
>>                     2. after login, server response back a refresh.html
>> with javascript to reload browser, this refresh.html need to be pasted
>> to browser to get that javascript to run.
>>                          user will definitely perceive this page switch,
>> make it a bad experience.
>>                      3. Compare with a single cookie attribute, there are
>> must more side effects.
> 2. There are plenty of ways to improve internal redirect method and
> eliminate the need for "refresh.html".
>
>   Option 2.1. Upon unauthenticated access, back-end internal redirects to
> "login.html". The POST action of the form send the current page URI to
> the back-end as well, then after the /login URI handler authenticates
> the user, it fetches the "Referer" header of the request, and this
> header contains the original page URL, then back-ends uses internal
> redirect or HTTP 3xx redirect to present the original page content.
"Referer" does not work. Please google search or try it yourself.
>
>   Option 2.2. Upon unauthenticated access, back-end internal redirects to
> "login.html". The form in the "login.html" contains a hidden input
> indicating the current URI. Then the JS in "login.html" fills this
> hidden input upon successfully loading the page. So the POST action of
> the form send the current page URI to the back-end as well, then after
> the /login URI handler authenticates the user, it uses internal or HTTP
> 3xx redirect to lead the processing flow back to the original URI.
If the first time I try to access kimchi with 
"https://xxx.xxx.xxx.xxx:8001/login.html", what will happen?
>
>   Option 2.3. Upon unauthenticated access, back-end internal redirects to
> "login.html". "login.html" does not use traditional HTTP POST form,
> instead, it uses /login API call to login using AJAX invocation, upon
> successful login, "login.html" refreshes the page.
how to differentiate first time login and session timeout?
>
> Actually "softlayer.com" is using some means of internal redirection to
> guide user to the previous page. You can try to open the following links.
>    https://control.softlayer.com/account/invoices
>    https://control.softlayer.com/account/orders
> You'll notice it returns a plain login page, after login, it triggers a
> refresh that the user doesn't notice at all.
>
> The page switch only happens in login process. Though you state cookie
> approach as "single cookie attribute", but it gets transferred in every
> request and response even the AJAX ones. It's obvious which is more
> evil. There is also an inevitable bug in cookie approach. Say the user
> opens two firefox tabs, one is for VMs, the other is for StoragePool.
> Once the session is expired, and the user login in VMs, it may lead the
> user to StoragePools if StoragePools happens to be opened after VMs.
Quite easy to be solved, I believe there will be an event notification 
when switching tab.
>
> Moreover, I didn't see any investigation on how other WEB sites does
> this. Though this is not my task, I just investigated softlayer to prove
> my point. I suggest anyone who works on this firstly do some research on
> the existing WEB sites and avoid re-inventing the wheel.
Kimchi has its own problem and solution, others can be referenced, but 
not defined by them.
>
>>>> Both client javascript and web container can set cookie.
>>>> the previsousPage can also be set with javascript in client side,
>>>> each time user go to a new tab, there is a callback, in the callback,
>>>> previousPage can be set into cookie.
>>> But the previousPage cookie is still carried forth and back in normal
>>> requests, which is absolute redundant information, not to mention the
>>> troubles it causes when the user opens two firefox tabs accessing
>>> different page of Kimchi.
>> Cookie will always be sent back and forth regardless of our previousPage
>> url.
>> What trouble you mean when multiple browser tabs, please clarify.
>>
>>>>> on 2014/06/13 17:24, Yu Xin Huo wrote:
>>>>>> First of all, what you are complaining about is a pretty, pretty small
>>>>>> fraction of the overall login solution.
>>>>>> For that fraction, the design is to back to the original url after
>>>>>> user
>>>>>> login from a session timeout.
>>>>>> Shaohe's current implementation works well. what zhengsheng means is
>>>>>> about how to improve it.
>>>>>>
>>>>>> On 6/13/2014 3:12 PM, Zhou Zheng Sheng wrote:
>>>>>>> If I remembered correctly, Shao He, Yu Xin and me had several long
>>>>>>> talks
>>>>>>> on the login design. This solution is not the solution we agreed, and
>>>>>>> all of us thought that this solution is ugly and obviously should be
>>>>>>> improved.
>>>>>> We have ever discussed several options, but we have not converged to a
>>>>>> certain one.
>>>>>> Let me continue to read to see what the 'ugly' you mean.
>>>>>>
>>>>>> This solution works very well, it can always be improved.
>>>>>>> The problem is on how it redirects the user back to the previous page
>>>>>>> after login. In this patch, the back-end has to intercepts each
>>>>>>> access
>>>>>>> to any of the ".html" page, and sets
>>>>>>>       cookie['lastPage'] = current page URI,
>>>>>>> and return it to front-end, then the front-end sends this cookie to
>>>>>>> back-end in every query, including AJAX query. When the session
>>>>>>> expires,
>>>>>>> the back-end redirects the front-end to a login page, after login
>>>>>>> successfully, the back-end gets cookie['lastPage'], at last, redirect
>>>>>>> the user to the last page.
>>>>>>>
>>>>>>> Just to implement returning to the previous page afte login, the
>>>>>>> back-end has to intercept each '.html' access and sets cookie, and
>>>>>>> the
>>>>>>> front-end has to send the cookie in each request including AJAX ones.
>>>>>> Handling cookie back and forth is done by browser and web container.
>>>>>> There will be always cookie, this is not an additional overhead.
>>>>>> What shaohe does is only to get the current tab url. That is a quite
>>>>>> small amount of code.
>>>>>>> So in the last talk we agreed that a simpler and more effective
>>>>>>> solution
>>>>>>> should be used. We at least have two alternative solutions.
>>>>>>>
>>>>>>> 1. When session is expired, we redirect the user to login.html. After
>>>>>>> login successfully, the JS script in the front-end asks the
>>>>>>> browser to
>>>>>>> go back to the previous page. Since the browser keeps a stack of page
>>>>>>> histories, it should be easy to do this.
>>>>>> This is server side redirect after form is submitted, at that time,
>>>>>> client side has no code to run.
>>>>>> Server side redirect browser directly to last page in login response,
>>>>>> quite straight-forward.
>>>>>>
>>>>>> I do not think brower history will be a reliable solution.
>>>>>> http://www.w3schools.com/js/js_window_history.asp
>>>>>>> 2. When the back-end detects the session is expired or the user
>>>>>>> hasn't
>>>>>>> login yet, it uses internal redirect to present the "login.html".
>>>>>>> From
>>>>>>> the front-end point of view, an unauthenticated access to "GET
>>>>>>> #tabs/vms.html" returns "login.html". After the user input his/her
>>>>>>> password in the "login.html" and click "login", the back-end receives
>>>>>>> the request, if the password is correct, it returns a
>>>>>>> "refresh.html". In
>>>>>>> "refresh.html" there is actually a small JS code to ask the
>>>>>>> browser to
>>>>>>> refresh the page. Since we are using internal redirect all the time,
>>>>>>> the
>>>>>>> page URI in the browser remains "#tabs/vms.html", so after the login,
>>>>>>> just refreshing the page would lead user to the real "vms.html.tmpl".
>>>>>>>
>>>>>>> In the above two solutions, no ugly cookie is needed for each request
>>>>>>> and response, and the back-end doesn't have to intercept each ".html"
>>>>>>> access, but just has to intercept each unauthenticated access.
>>>>>> internal redirect will make browser url mismatch with the content.
>>>>>> the current design will always keep url address its intended content,
>>>>>> this is a virtue over internal redirect we should pursue.
>>>>>>
>>>>>> The ugly cookie is removed, but introduced *a much uglier*
>>>>>> "refresh.html" and code in that html.
>>>>>> To me, it is much more complicated.
>>>>>>> I don't know why Yu Xin and Shao He sent the to-be-abandoned solution
>>>>>>> again to the mailing list. Patches were sent on 20:00 Chinese local
>>>>>>> time, the patches got merged in 05:00 Chinese local time in the next
>>>>>>> day. There is no other developer gets CCed. There is no reviewed-by.
>>>>>> This is not to-be-abandoned solution. Adam, Aline, Shao He and I have
>>>>>> discussed this in a team meeting.
>>>>>> It is sent to mail list. all people subscribed to mail list should
>>>>>> get it.
>>>>>> It is already V5, it is reviewed enough.
>>>>>>> After talked to Shao He this morning, he told me that we
>>>>>>> determined to
>>>>>>> defer this feature/task to seek a better solution. Shao He told me
>>>>>>> that
>>>>>>> they sent the patch as RFC, not aim to be a final solution.
>>>>>>> However it
>>>>>>> is a big misleading to other developers because there is no RFC in
>>>>>>> the
>>>>>>> patch title. There is even no reviewed-by. Is there any reason to
>>>>>>> merge
>>>>>>> it so hurry?
>>>>>> This is already V5, how can it be an RFC?
>>>>>>> If there was any time and task pressure, I think as an open source
>>>>>>> project, the progress should have some flexibility. We should not
>>>>>>> write
>>>>>>> code for a known broken solution, while there is obvious alternative
>>>>>>> solutions. This is very different from incremental development. In
>>>>>>> incremental development, the direction and the solution is
>>>>>>> correct, we
>>>>>>> just completes the missing pieces step by step. In this case, the
>>>>>>> solution and the framework itself is not so effective. Once it's
>>>>>>> merged,
>>>>>>> we started to rely on this, and changing and improving it would be
>>>>>>> much
>>>>>>> harder.
>>>>>> Speed will always be most key to succeed for any organization.
>>>>>> There is nothing broken, it works well.
>>>>>> Improvement will never stop.
>>>>>>
>>>>>> Again, the overall solution is discussed across whole team in one
>>>>>> of the
>>>>>> Wed's team meeting.
>>>>>> Cookie is the best way to store previousPage to reserve user
>>>>>> context. If
>>>>>> anyone see a better way than cookie for this issue, welcome to discuss
>>>>>> with me.
>>>>>>
>>>>>> As zhengsheng point out, we need to try to improve that small bit of
>>>>>> code to store previousPage into cookie. but again, it works well
>>>>>> currently.
>>>>>> But obviously, that is far from a justification to stop this patch
>>>>>> from
>>>>>> being merged.
>>>>>>> on 2014/06/13 05:50, Aline Manera wrote:
>>>>>>>> Applied. Thanks.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>>
>>>>>>>> Aline Manera
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Kimchi-devel mailing list
>>>>>>>> Kimchi-devel at ovirt.org
>>>>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>>>>
>>
>