I know it's a bit too late to send my opinion, but I thought we reached an agreement. I'm just a bit surprised that the agreement are so easily destroyed. on 2014/06/13 15:12, 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.

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.

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.

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.

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.

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?

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.

on 2014/06/13 05:50, Aline Manera wrote:

...

Applied. Thanks.

Regards,

Aline Manera

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

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. 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

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.

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.

...

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.

I do not think brower history will be a reliable solution. http://www.w3schools.com/js/js_window_history.asp 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

on 2014/06/13 17:57, Yu Xin Huo 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

On 6/13/2014 5:45 PM, Zhou Zheng Sheng wrote: 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.

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.

...

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.

...

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.

I do not think brower history will be a reliable solution. http://www.w3schools.com/js/js_window_history.asp 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

on 2014/06/13 18:47, Yu Xin Huo wrote:

...

on 2014/06/13 17:57, Yu Xin Huo 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

On 6/13/2014 5:45 PM, Zhou Zheng Sheng wrote: 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.

On 6/13/2014 6:07 PM, Zhou Zheng Sheng wrote: 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. 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. 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. 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. 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.

...

...

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.

...

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.

I do not think brower history will be a reliable solution. http://www.w3schools.com/js/js_window_history.asp 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@ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >

-- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

on 2014/06/16 15:53, Yu Xin Huo wrote:
> 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.

I tried it myself before I sent the previous mail, it works. The problem
is that intermediate proxy may change it, or the user can configure
firefox not sending it. It's not reliable, but it can served as the last
means to get the related page.

>>
>>   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?

There is no https://xxx.xxx.xxx.xxx:8001/login.html, it is only served
through internal redirect. The user can not directly access it.

>>
>>   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?

Before internal redirection, back-end can check if it is first time
login or timeout, and sets an variable/title/label in the dynamic
rendered "login.html".

>>
>> 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.

What do you think of softlayer.com ?

>>
>> 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.

The event notification is in the front-end, how do you send it to back-end?

>>
>> 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.

Yes. However I didn't said we should use the same mechanism as other
sites, but I said exactly we can refer them. The problem is that the
authors didn't referenced and analyzed other sites' solutions at all. At
least I didn't see them in the commit message, email, wiki or offline
discussion. Would you please tell me how many sites you referenced and
why none of the existing solutions can not be used? Or you can just send
me a link of you previous analysis.

>>
>>>>> 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@ovirt.org
>>>>>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>>>>>
>>>
>>
> 
> 

on 2014/06/16 19:46, Yu Xin Huo wrote:
> On 6/16/2014 4:23 PM, Zhou Zheng Sheng wrote:
>> on 2014/06/16 15:53, Yu Xin Huo wrote:
>>> 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.
>> I tried it myself before I sent the previous mail, it works. The problem
>> is that intermediate proxy may change it, or the user can configure
>> firefox not sending it. It's not reliable, but it can served as the last
>> means to get the related page.
> 
> Checked all types of requests, "referer" is always below in my env.
> 
> 
> If "referer" can work, I truly like it very much, that will save us a lot of 
> complexity, can you paste a screen-shot of your env?
> 

What exactly environment do you want? Firefox about:config or the debug
screen?

>>
>>>>    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?
>> There is nohttps://xxx.xxx.xxx.xxx:8001/login.html, it is only served
>> through internal redirect. The user can not directly access it.
> For web resources at server side, static, dynamic, protected, unprotected, there 
> will always be a url addressable way to load it.
> For your design, it completely relies on browser url which is quite easy for 
> user to input anything.
> For fuzzing security testing or robustness testing, any malformed url maybe 
> tried to check server response.
> If user does input below, for your option 2.2, it will be a dead loop that user 
> will always see login page. Tester will be quite happy to found it.
> 
> "https://xxx.xxx.xxx.xxx:8001/login.html"
>

There are some possible methods to solve this problem.

Firstly, in the above method, there is a hidden input field
"PreviousPage", which is filled by JS with the current URI. Then in the
back-end, after successful /login handling, just before it redirects the
user back, it checks if "PreviousPage" is "login.html", if yes,
redirects to the front page, otherwise redirects to "PreviousPage".

Actually in back-end, it's also possible to restrict a page to be
accessed only fron internal redirection. There are two possible ways to
do this.
  a) Usually we use internal redirect as
internal_redirect("login.html"). Actually we can use
internal_redirect("login.html?from=internal"), then in login.html.tmpl,
it checks $data.from, if it is not from internal, it could hide the
content or uses JS to redirect the user to the front-page.
  b) A more elegant way is to configure "login.html" as follow.
  '/login.html':  {tools.kimchi_restrict_internal.on: True},
Then in the function kimchi_restrict_internal(), it checks if
kimchi.request.prev is empty. If it's empty, it is accessed directly,
and we can raise 404. If it's not empty, it is accessed from
InternalRedirect.

>>
>>>>    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?
>> Before internal redirection, back-end can check if it is first time
>> login or timeout, and sets an variable/title/label in the dynamic
>> rendered "login.html".
> I do not think there is a way to differentiate "user first time login" and 
> "session timeout" by internal redirect.
> Can you clarity?

If it's the first time user accesses Kimchi, back-end would create a new
session cookie. And Kimchi does not set cherrypy session timout,
instead, it expire the session by itself. This is because it needs to
differentiate robot requests from normal requests. So the back-end
actually knows if a session is created the first time or it's expired.

>>
>>>> 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.
>> What do you think of softlayer.com ?
> We have designed a complete, end to end "login/logout/user, pass wrong/session 
> timeout/preserve meaningful user context" experience.
> And figured out a technical solution to support it.
> If you do see any better user experience that softlayer delivered than our 
> current design. Feel free to point out.

If you really checked softlayer.com . It implements the things you
required. For example, if the user does not take action for some time,
it redirects the browser to the login page and says session timeout.

>>
>>>> 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.
>> The event notification is in the front-end, how do you send it to back-end?
> front end can also set cookie.

Exactly, so why you have the back-end record the previous page? If you
are really staying with the cookie approach, we can create an common
html page to be included by other pages, sets the previous page cookie
whenever it gets loaded. It seems kimchi-ui.tmpl already serves as this
purpose. I'm not a front-end expert, you can correct me if I didn't
understand it right.

In fact the redirection can be done solely in the front-end. The
back-end has no interests in the previous page. The front-end can use
the HTM5 local storage feature. The front-end can record the current
page URI in the local storage (of cause except login.html), then in
login.html, it uses login API call to authenticate, then it fetches the
previous page from the local storage and go back to it. In this way we
avoid the complexity in back-end and avoid extra cookie in each AJAX
request and response.

>>
>>>> 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.
>> Yes. However I didn't said we should use the same mechanism as other
>> sites, but I said exactly we can refer them. The problem is that the
>> authors didn't referenced and analyzed other sites' solutions at all. At
>> least I didn't see them in the commit message, email, wiki or offline
>> discussion. Would you please tell me how many sites you referenced and
>> why none of the existing solutions can not be used? Or you can just send
>> me a link of you previous analysis.
> I do not believe all web sites use internal-redirect to do login, why you only 
> reference a website that use internal-redirect?
> If I do want to reference others, I will list the top 10 web sites in this world 
> and check how they implement it.
> 
> Now, only hotmail is available, checked that it is not internal-redirect
> 
> 
> In an enterprise environment, all applications, systems need to be integrated to 
> enable single access control, single sign on.
> The whole login process of all apps is delegated to a central access manager, 
> how can internal-redirect work in this case?
> 

Then you can state your investigation on how top 10 sites does login,
and checks if they uses the "previous page" cookie solution. I don't
believe top 10 sites uses "previous page" cookie solution for just a
login handling.

Furthermore, there is nothing prevent internal redirect solution to be
integrated with single sign on or ldap.

For example, whether or not the login.html is served by internal
redirect, the actual auth handling is in Kimchi back-end. The Kimchi
login API call or login POST handler can pass the auth to an LDAP server.

I also has done some work on single sign on before. Usually the single
sign on is on the different URI of the login.html. When the browser gets
a token from the central access manager, it can be redirected to
https://kimchi:8001/sso?token=XXXX. Then the sso handler of Kimchi can
check the token with the central manager or check the token in the
back-end which is sets by the central manager in other means.

>>
>>>>>>> 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@ovirt.org
>>>>>>>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>>>>>>>
>>>

On 6/13/2014 6:09 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.

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. I wonder can client redirect by browser history without cookies. I have ever already stated that browser history is unreliable which definitely can not be adopted, please stop thinking about browser history.

Again, Brower history is at brower level, not application level. Read below, If session timeout, kimchi login page is presented. At this time, user go to www.google.com, searched something. Then user input kimchi url to login. If leverage brower history, then back to www.google.com.

...

...

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.

...

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.

I do not think brower history will be a reliable solution. http://www.w3schools.com/js/js_window_history.asp 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 06/13/2014 04:12 AM, 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.

I am not aware about ANY discuss you have just mentioned. ALL discussion related to patches/features **MUST BE DONE IN THE MAILING LIST** to the whole team be aware about it! And in a V5 patch, you have more than enough time to do it.

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.

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.

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.

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.

If I applied the patches I reviewed it!

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?

An RFC named as V5? A V5 patch set is to "be so hurry" for you??

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.

If you think you solution is better, send the patch!

on 2014/06/13 05:50, Aline Manera wrote:

...

Applied. Thanks.

Regards,

Aline Manera

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

on 2014/06/15 00:18, Aline Manera wrote:

On 06/13/2014 04:12 AM, 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.

I am not aware about ANY discuss you have just mentioned. ALL discussion related to patches/features **MUST BE DONE IN THE MAILING LIST** to the whole team be aware about it!

And in a V5 patch, you have more than enough time to do it.

Yes. The problem is that after a local discussion we reached an agreement and I trusted Shao He and Yu Xin who were actively work on this should have reflected the discussion result either in the patch or in the mail. Sorry for not sending the discussion to the mail list. I'd send my opinion to mail list every from now on.

...

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.

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.

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.

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.

If I applied the patches I reviewed it!

You are the boss ;-). However even if you thought the solution is acceptable, there was some new code in the v5 patch and it's worth getting reviewed by other developers.

...

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?

An RFC named as V5? A V5 patch set is to "be so hurry" for you??

Yes. It's so hurry. The fact is the solution of redirecting user back to previous page is not mature and I was strongly against it. As I said, Yu Xin and Shao He also thought it's bad. The v5 patch does not improve this at all. In fact I'd be curious on how it didn't get improved through 5 series of patches.There was also no investigation on how other sites does this. In my opinion, if it's not mature, we can split out the redirection part, and apply the rest part, so that it can be improved. By the way, I think the version count does not directly related to the completeness of a patch series. If it does not reach a certain point of maturity, we should improve it in new version. In other projects it's usual to see a patch gets updated to v8 v10 or even more. For example: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector [libvirt] [PATCH v8 0/4] Handling of undefine and r e define snapshots with VirtualBox 4.2 or higher http://gerrit.ovirt.org/#/c/28088/ v12 Final separation of IOProcess and RFH For this particular patch series, I think the redirecting part is very ugly, the rest part is good. At least we can split out and merge the rest, and improve the redirection mechanism. Consider this patch series contains lots of mechanism changes, it's normal to update more than 5 or 6 times to get it ready.

...

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.

If you think you solution is better, send the patch!

It's always cheap to change things in the design phase, then development phase. Once it's merged, new code starts to rely on it, and things would get harder to changes. I already expressed my opinion to the developers, and the redirection does not get improved even the patch was updated through 5 series. I'd always happy to help, but I'm lack of some front-end knowledge to do the UI part. However, don't knowing how to make a film does not mean that the audience can not give comments on the quality of the film. The same applies to patches. If you always say "If you think you solution is better, send the patch!", that's destroying the basis of code review. If your logic takes hold, there should be no code review at all, and to do one thing right, we just submit patches by patches and go in circles. The problem is that the authors of this patch series neither did any investigation on how other sites do login redirection nor tried any of the proposed alternatives. If there's something we all know it's important, but the current solution is evil, and the same thing has been solved by other WEB sites already, why we persisted on this solution and merged it so quickly? If we consider an open source project as a product, the code quality is an key point of this product, since its internal opens to everyone. Merging a patch is a declaration of admitting that the code quality reaches the common quality of this project. Yes, we can always improve, but, shouldn't we have some bottom line?

...

on 2014/06/13 05:50, Aline Manera wrote:

...

Applied. Thanks.

Regards,

Aline Manera

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

on 2014/06/16 16:34, Yu Xin Huo wrote:

On 6/16/2014 12:12 PM, Zhou Zheng Sheng wrote:

...

on 2014/06/15 00:18, Aline Manera wrote:

...

On 06/13/2014 04:12 AM, 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. I am not aware about ANY discuss you have just mentioned. ALL discussion related to patches/features **MUST BE DONE IN THE MAILING LIST** to the whole team be aware about it!

And in a V5 patch, you have more than enough time to do it.

Yes. The problem is that after a local discussion we reached an agreement and I trusted Shao He and Yu Xin who were actively work on this should have reflected the discussion result either in the patch or in the mail. Sorry for not sending the discussion to the mail list. I'd send my opinion to mail list every from now on. The agreement is that it worth a try, not as a final design.

If you didn't try, how do you know which is better? If I remembered correctly, you agreed to try it and agreed that the current solution was too dirty.

...

...

...

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.

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.

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.

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. If I applied the patches I reviewed it!

You are the boss ;-). However even if you thought the solution is acceptable, there was some new code in the v5 patch and it's worth getting reviewed by other developers.

...

...

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? An RFC named as V5? A V5 patch set is to "be so hurry" for you??

Yes. It's so hurry. The fact is the solution of redirecting user back to previous page is not mature and I was strongly against it. As I said, Yu Xin and Shao He also thought it's bad. The v5 patch does not improve this at all. In fact I'd be curious on how it didn't get improved through 5 series of patches.There was also no investigation on how other sites does this. In my opinion, if it's not mature, we can split out the redirection part, and apply the rest part, so that it can be improved.

I am not thinking it is bad, till now, I am still thinking cookie is the only way to support all the scenarios we want to support.

...

By the way, I think the version count does not directly related to the completeness of a patch series. If it does not reach a certain point of maturity, we should improve it in new version. In other projects it's usual to see a patch gets updated to v8 v10 or even more. For example:

[Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target [Qemu-devel] [PATCH v10 1/6] block: round up file size to nearest sector [libvirt] [PATCH v8 0/4] Handling of undefine and r e define snapshots with VirtualBox 4.2 or higher http://gerrit.ovirt.org/#/c/28088/ v12 Final separation of IOProcess and RFH

If you would like to reference engineering statistics from other projects, please get their average patch version or the whole open source field's average patch versions. You intentionally to reference their several big version patches, it will mislead the whole team.

No, my point is not that we should update so many version for each patch, instead, the point is for big mechanism change, a large version count is not rare. Just v5 does not gives it the justification to be mature, only the test discussion and code review on the code and solution does.

...

For this particular patch series, I think the redirecting part is very ugly, the rest part is good. At least we can split out and merge the rest, and improve the redirection mechanism. Consider this patch series contains lots of mechanism changes, it's normal to update more than 5 or 6 times to get it ready.

I did not get any justification at all to pursue to try to get a patch version as big as possible.

I did not said at all you should get a patch version as big as possible.

...

...

...

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. If you think you solution is better, send the patch!

It's always cheap to change things in the design phase, then development phase. Once it's merged, new code starts to rely on it, and things would get harder to changes. I already expressed my opinion to the developers, and the redirection does not get improved even the patch was updated through 5 series. I'd always happy to help, but I'm lack of some front-end knowledge to do the UI part. However, don't knowing how to make a film does not mean that the audience can not give comments on the quality of the film. The same applies to patches. If you always say "If you think you solution is better, send the patch!", that's destroying the basis of code review. If your logic takes hold, there should be no code review at all, and to do one thing right, we just submit patches by patches and go in circles.

Welcome your comment, please focus on the overall solution. Parts of the solution are inter-connected, pieces can not be separated. we always discuss pieces with the overall solution in mind, please also.

Current design is as below:

1. user first time access a protected resource, redirect login page. 2. when session timeout, redirect user to login page with a message "session timeout, please re-login". 3. if anytime a wrong user/pass is provided, redirect back to login page with a message "username/password wrong." 4. if user login successfully, always redirect to the url that he intend to go originally.

No. I don't see there is any trouble to split this piece out. You can always redirect the user back to the /host tab as a temp solution, then improve the redirection solution.

if you are truly interested to be involved, please think about the overall solution totally when you are challenging a piece. You do not need to code everything out so client side skill is not an excuse.

The fact is that I can only code the back-end, and my proposed solution involves front-end coding. If I didn't get help from the front-end, how can I submit patches? You didn't give a try on the alternative solution. If you did, I'm sure I can help with the back-end things.

...

The problem is that the authors of this patch series neither did any investigation on how other sites do login redirection nor tried any of the proposed alternatives. If there's something we all know it's important, but the current solution is evil, and the same thing has been solved by other WEB sites already, why we persisted on this solution and merged it so quickly?

Before layout your own overall solution, please do not speak with assumption.

I don't see any trouble of my proposed redirection solution to leave with the current one.

...

If we consider an open source project as a product, the code quality is an key point of this product, since its internal opens to everyone. Merging a patch is a declaration of admitting that the code quality reaches the common quality of this project. Yes, we can always improve, but, shouldn't we have some bottom line?

This is a design discussion, it has nothing to do with code quality, the current code support the design well.

Sorry for the misleading. By saying "code quality" I meant to say the design and code. I think my argument still takes hold if you consider the design.

...

...

...

on 2014/06/13 05:50, Aline Manera wrote:

...

Applied. Thanks.

Regards,

Aline Manera

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

On 6/15/2014 12:38 AM, Aline Manera wrote:

Sheldon, Yu Xin:

I identified 2 issues in this new flow

1) The username is not being displayed

in login.html.tmpl, add one line code. function updateBtnLabel() { document.getElementById("login").style.display = "none"; document.getElementById("logging").style.display = ""; kimchi.user.setUserName(document.getElementById("username").value); } this line of code will store a 'username' attribute into cookie, for the left, original popup login window code will handle to add user name to UI.

2) When I access "https://localhost:8001/vnc_auto.html?port=64667&path=?token=Fedora&encrypt=1" without be logged in, I am redirected to the login page and after it I got the Guests tabs instead of the VM console

After some investigation, backend has not handled the first time user paste a url(vnc_auto.html or /#tabs/templates), For current design, the brower url will always taken as 'previousPage' and stored in cookie. Shao he will add code to handle it.

The logic to do the proper redirect to the VM console is in kimchi.login_main() (ui/js/src/kimchi.login_window.js) I am wondering why we are not using it In the login.html.tmpl, we are using local functions to do the login, I think we need to call kimhi.login_main() instead have JS into login.html.tpml

Some code there are bind to UI structure and can only apply to login popup, can not reused. As there are too few css/js code in current login.html, so leave it there, but it is quite easy to separate those code to js, css files.

Please, send a patch for those issues AS SOON AS POSSIBLE as kimchi 1.2.1 will be released next Friday (June 20)

Thanks, Aline Manera

On 06/13/2014 04:12 AM, 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.

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.

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.

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.

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.

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?

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.

on 2014/06/13 05:50, Aline Manera wrote:

...

Applied. Thanks.

Regards,

Aline Manera

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 06/15/2014 12:38 AM, Aline Manera wrote:

2) When I access "https://localhost:8001/vnc_auto.html?port=64667&path=?token=Fedora&encrypt=1" without be logged in, I am redirected to the login page and after it I got the Guests tabs instead of the VM console

The logic to do the proper redirect to the VM console is in kimchi.login_main() (ui/js/src/kimchi.login_window.js) I am wondering why we are not using it

In the login.html.tmpl, we are using local functions to do the login, I think we need to call kimhi.login_main() instead have JS into login.html.tpml

Please, send a patch for those issues AS SOON AS POSSIBLE as kimchi 1.2.1 will be released next Friday (June 20)

Thanks, Aline Manera see patch: [PATCH V3 0/2] bug fix: redirect to the protected page after login

-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center