[
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)
