OZ-683: Skip logout confirmation page #22
Conversation
OZ-683 - Skip logout confirmation if id_token exists in logout URL OZ-683 - Added and updated tests OZ-683 - Removed unused property OZ-683 - Fixed failing tests
There was a problem hiding this comment.
Two comments here:
- It would be nice if the ticket could be replicated in OpenMRS's JIRA so that details about why this is going in are part of the long-term public record
- I don't love storing security-sensitive information in userProperties for similar reasons that its not a good idea to store security-sensitive information in globalProperties. E.g., all userProperties are currently dumped as plain-text as part of the
Sessionobject returned from the REST API.
| if (provider.getRetired()) | ||
| ps.unretireProvider(provider); |
There was a problem hiding this comment.
| if (provider.getRetired()) | |
| ps.unretireProvider(provider); | |
| if (provider.getRetired()) { | |
| ps.unretireProvider(provider); | |
| } |
|
I sort of agree with you @ibacher about storing the id token in user properties and I wanted to store it in the database but this was going to be invasive since core does not support user attributes. I was wondering what is the worst case scenario if one got hold of another's id token? Remember that this is not the access token, so they can only log out another user as the worst case scenario that I could think of, which is why I found it ok to store it as a user property. |
|
@ibacher @wluyima , this is the O3 ticket I believe: https://openmrs.atlassian.net/browse/OA-37 |
| @Override | ||
| public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) | ||
| throws IOException, ServletException { | ||
| Properties properties = OAuth2BeanFactory.getProperties(OAuth2BeanFactory.getOAuth2PropertiesPath()); | ||
| String redirectPath = properties.getProperty("logoutUri"); | ||
| //the redirect path can contain a [token] that should be replaced by the aut token | ||
| if (StringUtils.isNoneBlank(redirectPath) && redirectPath.contains("[token]")) { | ||
| String token = getToken(); | ||
| String token = Context.getAuthenticatedUser().getUserProperty(USER_PROP_ID_TOKEN); |
There was a problem hiding this comment.
By this time, do we still have an authenticated user?
There was a problem hiding this comment.
Yes, because this is called on oauth2 logout success and not OpenMRS logout success which is later called on the next few lines.
I agree with this point, though another approach would be to encrypt the token before storing it in the userProperty as a work-around. |
https://mekomsolutions.atlassian.net/browse/OZ-683