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

[Yu Xin Huo]

On 6/13/2014 6:05 PM, Sheldon wrote:
> On 06/13/2014 05:57 PM, 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.
> I think browser to redirect seems more easy.  But I not familiar with JS.
It is not browser redirect, it is still server side redirect, browser 
only get current tab url and store it into cookie.
>
> We have said let others to try it first as back-end redirect.
>
>>
>> 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.
>>>
>>> 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
>>>>>>
>>>>
>>>
>>
>>
>>
>>
>
>