You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by nakomis <gi...@git.apache.org> on 2015/02/25 09:56:51 UTC
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
GitHub user nakomis opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/529
Redirects web gui to root on logout to prevent login failure after logout
Previously, clicking logout would leave the page at /logout. Trying to log in would then navigate to a URL such as /logout/#v1 etc
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nakomis/incubator-brooklyn fix/logout
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/529.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #529
----
commit 562470f93a78026cf20f4b4ca98a3656a0aa9bf4
Author: Martin Harris <gi...@nakomis.com>
Date: 2015-02-25T08:55:14Z
Redirects web gui to root on logout to prevent login failure after logout
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/529#issuecomment-77142830
Thanks for pointing me at the at `originalRequest` in `BrooklynPropertiesSecurityFilter`. My initial concern is invalid but there are a few usability issues with the changes here:
* The `onClick` event is not fired when I right click the link and click "Open link in a new tab", so I'm not logged out.
* Every so often - I haven't worked out the cause - clicking Logout navigates me to the dashboard screen but doesn't actually log me out.
* Clicking logout normally makes it impossible to log back in without refreshing the page. Each time I enter my username and password I'm immediately logged out again. The log shows:
```
2015-03-04 10:55:33,823 INFO REST logging sam out of session 1xf18cj3jx94x59y5dt4kahdp
2015-03-04 10:55:47,359 INFO REST logging sam out of session 1tb4mq85kwddh199yigwsrwtrw
2015-03-04 10:55:52,173 INFO REST logging sam out of session 4afpbliikh5i1w29exmvrw534
```
* You can break logout on someone's web console by altering the logout function to either do something different or nothing at all (i.e. open the Javascript console and enter `logout = undefined`). It's broken until they refresh, at least.
* Having the function in the global namespace means there's a chance it will clash with another library that also defines `logout` globally. I'd suggest moving the function to `brooklyn.js` or `brooklyn-util.js`, adding an `id` to the anchor element and binding an `onClick` event there rather than putting it on the element directly.
@nakomis and @aledsage up to you for the first few comments. The last two should be fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/529
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/529#issuecomment-76949112
@sjcorbett The user isn't immediately logged back in due to `originalRequest` logic in https://github.com/apache/incubator-brooklyn/blob/master/usage/rest-server/src/main/java/brooklyn/rest/filter/BrooklynPropertiesSecurityFilter.java, the logic of which hasn't been changed. This PR simply changes the behaviour in the browser, which I'm not sure how to write a test for
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/529#issuecomment-77357925
PR comments (mostly) addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Redirects web gui to root on logo...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/529#issuecomment-75938647
Isn't the problem with this that after clicking logout the user will immediately be logged back in? The browser will just resend their credentials and they will be reauthenticated. Can you include a test case to prove me wrong?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---