[jira] [Comment Edited] (KNOX-641) Support CAS / OAuth / OpenID C / SAML protocols using pac4j

["Larry McCay (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KNOX-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15053409#comment-15053409 ] 

Larry McCay edited comment on KNOX-641 at 12/11/15 7:27 PM:
------------------------------------------------------------

Okay...

I applied the patch to the 0.7.0 branch and changed the pom version to remove the -SNAPSHOT so that it would build.

I'm impressed with what you have been able to leverage of the gateway services and other mostly undocumented aspects!
We need to document some of that in the developers guide.

A couple minor things and then an observation and follow up concern:

1. @author tags need to be removed they are generally not used. Credit will be provided in the JIRA and the commit itself.
2. the sso provider url should be given a different name. let's make it a pac4j specific parameter name like pac4j.sso.authentication.provider.url
3. the cookie domain is being set to the servername for the pac4j session. Trying to think about whether this is sufficient. In a cluster of Knox instances, there would be multiple KnoxSSO instances as well. I think that the cookies will need to be sent to every instance. Assuming that all of the instances are in the same domain, we should probably consider making the cookie domain reflect support for all subdomains. See KNOX-640 where I just introduced what I think will suffice for the KnoxSSO cookie.
4. We need to have some sort of unit tests of some of the assumptions made in the provider.
5. OBSERVATION: when I authenticated using the testBasicAuth clientName and then changed the knoxsso.xml topology to not have that parameter, I expected to be challenged by the cas server - at least after removing the hadoop-jwt cookie. It continued to use the established session.
6. CONCERN: I'm not sure whether we should have the testBasicAuth mechanism in the actual implementation. I would be interested in others' thoughts on this.

Regarding the OBSERVATION above, I think this may be fine given the nature of an SSO session. That is that I am assuming that this is due to some cookies that are being presented to pac4j that indicate the session is still valid and that authentication is not needed again yet. We need to be sure that this is the case and I will likely need to do some debugging to ensure that.

Regarding the CONCERN, given the above observation, I think it may be possible for an admin to add the testBasicAuth param, authenticate as someone else, change the topology back and continue to spoof the identity for as long as the pac4j session lasts. Which AFAICT is for the life of the browser session. Granted, a malicious admin could also change the federation provider altogether as well but I think that existing sessions would likely be invalidated in that case.

thoughts?


was (Author: lmccay):
Okay...

I applied the patch to the 0.7.0 branch and changed the pom version to remove the -SNAPSHOT so that it would build.

I'm impressed with what you have been able to leverage of the gateway services and other mostly undocumented aspects!
We need to document some of that in the developers guide.

A couple minor things and then an observation and follow up concern:

1. @author tags need to be removed they are generally not used. Credit will be provided in the JIRA and the commit itself.
2. the sso provider url should be given a different name. let's make it a pac4j specific parameter name like pac4j.sso.authentication.provider.url
3. the cookie domain is being set to the servername for the pac4j session. Trying to think about whether this is sufficient. In a cluster of Knox instances, there would be multiple KnoxSSO instances as well. I think that the cookies will need to be sent to every instance. Assuming that all of the instances are in the same domain, we should probably consider making the cookie domain reflect support for all subdomains. See KNOX-640 where I just introduced what I think will suffice for the KnoxSSO cookie.
4. OBSERVATION: when I authenticated using the testBasicAuth clientName and then changed the knoxsso.xml topology to not have that parameter, I expected to be challenged by the cas server - at least after removing the hadoop-jwt cookie. It continued to use the established session.
5. CONCERN: I'm not sure whether we should have the testBasicAuth mechanism in the actual implementation. I would be interested in others' thoughts on this.

Regarding the OBSERVATION above, I think this may be fine given the nature of an SSO session. That is that I am assuming that this is due to some cookies that are being presented to pac4j that indicate the session is still valid and that authentication is not needed again yet. We need to be sure that this is the case and I will likely need to do some debugging to ensure that.

Regarding the CONCERN, given the above observation, I think it may be possible for an admin to add the testBasicAuth param, authenticate as someone else, change the topology back and continue to spoof the identity for as long as the pac4j session lasts. Which AFAICT is for the life of the browser session. Granted, a malicious admin could also change the federation provider altogether as well but I think that existing sessions would likely be invalidated in that case.

thoughts?

> Support CAS / OAuth / OpenID C / SAML protocols using pac4j
> -----------------------------------------------------------
>
>                 Key: KNOX-641
>                 URL: https://issues.apache.org/jira/browse/KNOX-641
>             Project: Apache Knox
>          Issue Type: New Feature
>            Reporter: Jérôme Leleu
>            Assignee: Jérôme Leleu
>             Fix For: 0.7.0
>
>         Attachments: KNOX-641.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)