gandres / pwm

Automatically exported from code.google.com/p/pwm
0 stars 0 forks source link

logout issue behind NAM #437

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Configure PWM to use the logout URL of NAM (AGlogout)
1. Use Forgotten Password or Activate user module
2. Upon completion (after questions, c/r set etc.) set password
3. logout

What is the expected output? What do you see instead?
I would expect that when authenticated by Forgotten Password or Activate User 
PWM would use it own logout.jsp. Instead PWM always calls the configured logut 
URL (NAM AGlogout). This leads to an error page on the IDP, because that user 
was never logged in into NAM: they were logged in by PWM only.

The  implementation of the LogOut Servlet needs to check whether the user is 
authenticated via the Forgotten Password and Activate User module. If so, it 
needs to call the PWM logout JSP and not the configured logout URL.

Original issue reported on code.google.com by sebastia...@gmail.com on 2 Aug 2013 at 7:11

GoogleCodeExporter commented 9 years ago
Attached a proposed patch to fix this issue. I've extended the SessionStateBean 
with setLogoutLocal/ isLogoutLocal methods. Both the ActivateUser and 
ForgottenPassword servlet use the setLogoutLocal method. the Logout servlet 
uses the method isLogoutLocal to forward the User to the PWM logout JSP.

I initially tried to reuse the existing setAuthenticationType and 
getAuthenticationType of the UserInfoBean, but AUTH_FROM_FORGOTTEN is being 
overruled by AUTHENTICATED when the users gets authenticated by PWM, after 
supplying the token. 

Original comment by sebastia...@gmail.com on 6 Aug 2013 at 1:54

Attachments:

GoogleCodeExporter commented 9 years ago
So two points:

1) I'm not understanding the original issue.  If your logout url is set to the 
AG Logout url such as "http://app1.example.org/AGLogout", the IDP isn't 
directly involved with the browser.  The ESP backchannel takes care of the IDP 
logout.  The AGLogout page  displays the same logoutSuccess.jsp regardless of 
the authentication state.

2) The patch approach is nam-specific.  The logout url is used for other use 
cases besides NAM so I'm not sure this is a good approach, at the very least we 
would need a way to enable/disable, or better yet set a specific url for "local 
auth", but then that needs another http param, etc. etc...

Original comment by jrivard on 7 Aug 2013 at 6:34

GoogleCodeExporter commented 9 years ago
Thanks for your feedback. I didn't know that the default AGLogout page displays 
the same logoutSuccess.jsp regardless of the authentication state. This 
actually pointed me in the right direction: we have some customizations running 
on the logout pages, which actually throws an error when the AGlogout is called 
without being authenticated on the IDP. Thanks for the hint, I'll get that 
fixed ;)

Still, It would be cleaner if PWM wouldn't send the user to the AGlogout: the 
user is not authenticated on the IDP, so no reason to call the AGlogout.

My patch is indeed specific to fix this issue with NAM (or any other proxy). A 
specific url for local auth would be a cleaner approach, but also requires a 
lot of work ;)

Original comment by sebastia...@gmail.com on 7 Aug 2013 at 7:29

GoogleCodeExporter commented 9 years ago
So another approach to your issue would be to redirect to a custom public JSP 
behind NAM.  You could then figure out your authentication state and do what 
you need to from there - redirect based on auth, or build iframes to do 
whatever is needed.  

We could add a parameter to the called logout url like "authState" and set to 
true/false when the logout url is called.  

I'm weary to do the local/non local as different URLs or to add more settings 
as it will just confuse the average admin in most cases...  thoughts?

Original comment by jrivard on 7 Aug 2013 at 7:34

GoogleCodeExporter commented 9 years ago
Yupz, I'm already using a custom public JSP behind NAM that acts as a warning 
screen: you can cancel your logout by returning to the app you requested the 
logout from. This is because the average Joe does not understand SSO.

I will take a look at that JSP to see whether I can fix this on the NAM side 
and report back. If not, an extra param would be a good enhancement.

I agree that anymore settings will confuse the average admin (read MCSE). 
Especially a seperate logout url for a reverse proxy makes it hard understand/ 
configure.

Lets put this issue on hold for now and see whether I can make my custom public 
JSP more intelligent.

Original comment by sebastia...@gmail.com on 7 Aug 2013 at 7:48

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Ok had some look at the code and two parameters would be very handy:

- a parameter authState (boolean) and set to true/false (based on local login 
via ActivateUser or ForgottenPassword) when the logout url is called.

- a parameter forceAuth (boolean) and set to true/false when the logout url is 
called.

The extra param forceAuth is set based on whether the password is changed of 
the user. This extra param makes it possible to auto relogin that user on NAM 
after he/ she has changed his password. This could be easily implemented by 
looking at SessionStateBean.getFinishAction() == 
SessionStateBean.FINISH_ACTION.LOGOUT.

Your thoughts are appreciated :)

Original comment by sebastia...@gmail.com on 9 Aug 2013 at 1:17

GoogleCodeExporter commented 9 years ago
I adjusted my patch so that two params are added to the configuredLogoutURL:

- authLocal (whether PWM authenticated the user by itself)
- forceAuth (whether the password has been changed for the authenticated User )

SessionLogoutURL and the PWM Logout JSP URL are unaffected. I've extended the 
SessionStateBean to store the local auth state. No more settings are necessary.

Original comment by sebastia...@gmail.com on 14 Aug 2013 at 9:42

Attachments:

GoogleCodeExporter commented 9 years ago
Please take a look at the changes in revision 592.  The updates are based on 
your patch with a few differences.

Changed "authLocal" to publicOnly to indicate if private or public pages have 
been accessed, and then watched for actual requests to /private/ to trigger the 
privateUrlAccessed flag in SessionStateBean.

Changed "forceAuth" to passwordModified, and switched flag in SessionStateBean 
to same boolean name instead of unneeded FinishState enum.

Added idle flag to show if logout is due to idle timeout.

Please update bug if this satisfies your requirements, based on comment history 
I think this should do it.  The attribute names changed to reflect the state of 
the pwm session as opposed to what your using them for to try to make them more 
generic.

Original comment by jrivard on 17 Aug 2013 at 12:51

GoogleCodeExporter commented 9 years ago
Ok, sounds good to me. Only thing is that authLocal is not necessarily equal to 
publicOny. In the current build they are equivalent, but what if in the future 
we call authUsrWithUnknownPassword and redirect to a private page? Im' not sure 
this will ever happen, but it is something to be aware of.

Again, thanks for accepting this ER.

Original comment by sebastia...@gmail.com on 17 Aug 2013 at 8:36

GoogleCodeExporter commented 9 years ago
I think the publicOnly flag should be correct, even if a future change modifies 
behavior.  If the user ever accesses a page under /private, this flag is 
tripped, and that would only happen (in case of NAM) if the user had 
authenticated to NAM.  In any case, if it breaks in future just open another 
issue.

Original comment by jrivard on 24 Aug 2013 at 10:56