Fix redirect to login page when using single open id provider authentication #579
Conversation
| /** | ||
| * | ||
| */ | ||
| public String goToLoginPage() { |
There was a problem hiding this comment.
This method does not actually go to a page, right? It just returns a string representing the page to go to. Could it be named something like getLoginPageId or getLoginPagePath?
There was a problem hiding this comment.
This could use a non-empty doc comment, particularly if it keeps the strange name.
There was a problem hiding this comment.
This method is executed as soon as someone enters the login page. So, if they are already logged in, they are redirected to the dashboard, if there's only a single open id provider then they are redirected to the provider, otherwise, they are shown the usual login page.
Maybe something like onLoginPageAccessed
|
Code structure looks fine. Could use some documentation or a name change where indicated. Names that work well in the xml seem inappropriate in Java, so it is probably futile looking for ideal names. |
|
✅ Tested. |
carlosmunoz
force-pushed
the
login-redirect-fix
branch
from
f8c4938
to
48da2ea
Compare
| * This method is executed when accessing the login page. Depending on | ||
| * current session state, it might indicate to redirect to a different | ||
| * location. For example, if the user is already logged in, it will indicate | ||
| * to redirect to the dashboard. |
There was a problem hiding this comment.
I think the first line should give a reasonable summary, before giving all the details. e.g.
/**
* Indicates which page the user should be directed to when trying to log in.
* ...
|
@davidmason Made the changes you indicated. Can you please take another look before I send it for QA? |
|
👍 Passed review. |
|
@djansen-redhat Could you please give this another QA pass? Just to make sure nothing has been borken after code review changes. |
|
@carlosmunoz on it |
|
✅ Retested. |
…ication Pull request #579 Squashed commit of the following: commit a9ba559 Author: Carlos A. Munoz <camunoz@redhat.com> Date: Thu Sep 4 16:56:35 2014 +1000 Rename methods and change comments. commit 48da2ea Author: Carlos A. Munoz <camunoz@redhat.com> Date: Thu Sep 4 15:03:47 2014 +1000 Code review updates. commit 8a3d899 Author: Carlos A. Munoz <camunoz@redhat.com> Date: Thu Sep 4 12:04:30 2014 +1000 Fix redirect to login page when using single open id provider authentication.
|
Merged in commit 48d12c3 |
This pull request ensures that when accessing a link directly (e.g. going to the editor from a bookmarked link) under single open id provider authentication, Zanata will not go to the usual login page, but instead will go directly to the provider.
Things to test: