Hi Jérôme -

I have unfortunately not had a chance to pull, review and test it yet and
have intended to do that today.
I apologize for the delay.

I was actually thinking that this would go into a follow up release that we
would try and get done rapidly after the 0.7.0 release but we can discuss
the target and its chances of destabilizing 0.7.0.

I believe that it is rather self-contained with only a few changes to
external modules.

Opening the JIRAs is perfect and I was going to do that once I started the
review.

Let's continue review comments and collaboration on those JIRAs.
I will add you to the contributors list so that we can assign them to you.

Thank you for your contributions and your patience, Jérôme!

--larry

On Fri, Dec 11, 2015 at 3:00 AM, Jérôme LELEU <lel...@gmail.com> wrote:

> Hi,
>
> I didn't get any new feedback on the pull request so I assume everything is
> ok from your point of view.
>
> I released pac4j v1.8.1 and j2e-pac4j v1.2.1 so I updated the pull request
> to use them and successfully re-tested everything.
>
> I opened KNOX-641 and submitted the corresponding patch.
>
> I also wrote the documentation, opened KNOX-642 and submitted the
> corresponding patch (just to let you know that it doesn't work out of the
> box in Windows, I had to replace mvn.bat by mvn.cmd to make ant work).
>
> Even if the branch 0.7.0 has already been created, I assumed this new pac4j
> provider will go into this version 0.7.0 (dependency on the 0.7.0-SNAPSHOT
> parent version).
>
> Just let me know if everything is ok and when it's goind to be merged.
>
> Thanks.
> Best regards,
> Jérôme
>
>
>
>
> 2015-12-04 14:20 GMT+01:00 larry mccay <larry.mc...@gmail.com>:
>
> > That ability to configure multiple mechanisms based on clientName is
> really
> > interesting for Knox.
> > Currently, we require separate topologies per authentication mechanism.
> > The ability to configure them all in one is really great.
> >
> > We would need to think through the best way to provide the clientName
> > parameter.
> > Since this is targeting KnoxSSO it can actually be added to the
> providerURL
> > used to redirect from the participating application.
> > Regardless of the authentication mechanism used each application will
> still
> > get the same JWT based cookie.
> >
> > I think that should work really nicely.
> >
> >
> > On Fri, Dec 4, 2015 at 7:17 AM, larry mccay <larry.mc...@gmail.com>
> wrote:
> >
> > > Excellent, Jérôme.
> > > Thanks!
> > >
> > > On Fri, Dec 4, 2015 at 2:40 AM, Jérôme LELEU <lel...@gmail.com> wrote:
> > >
> > >> Hi,
> > >>
> > >> I will write how to configure the pac4j provider in the documentation,
> > but
> > >> I can already give you some insights.
> > >>
> > >> My main goal is always to respect the key design principles of pac4j
> > >> whatever the environment / framework in which it is implemented. For
> > Knox,
> > >> I'm pretty happy with the use of the j2e-pac4j library, which means
> that
> > >> almost all the pac4j features are available, especially both direct
> and
> > >> indirect clients. So it can do what Shiro already does but also, as we
> > >> agreed together, supports remote authentications.
> > >>
> > >> It is only limited by what you can currently configure. And even
> > >> configuration is a pac4j feature as the CAS server has the same need.
> > >> Everything happens in this class:
> > >>
> > >>
> >
> https://github.com/pac4j/pac4j/blob/master/pac4j-config/src/main/java/org/pac4j/config/client/ConfigPropertiesFactory.java
> > >> ,
> > >> which allows you to configure Facebook, Twitter, a CAS server, a SAML
> > IdP
> > >> or an OpenID Connect provider. All the provided parameters to the
> pac4j
> > >> provider are put into a Map and the ConfigPropertiesFactory is built
> > with
> > >> this Map to return the built client (= authentication mechanism).
> > >>
> > >> You have one more specific option for Knox as a basic authentication
> > popup
> > >> where the username must match the password, you can define that by:
> > >>
> > >> <param>
> > >>   <name>clientName</name>
> > >>   <value>testBasicAuth</value>
> > >> </param>
> > >>
> > >>
> > >> It's for testing only.
> > >>
> > >> For a CAS server:
> > >>
> > >> <param>
> > >>   <name>cas.loginUrl</name>
> > >>   <value>https://casserverpac4j.herokuapp.com/login</value>
> > >> </param>
> > >>
> > >>
> > >> Here are all the properties available for building clients (their
> > meaning
> > >> is obvious):
> > >>
> > >> facebook.id
> > >> facebook.secret
> > >> facebook.scope
> > >> facebook.fields
> > >> twitter.id
> > >> twitter.secret
> > >> saml.keystorePassword
> > >> saml.privateKeyPassword
> > >> saml.keystorePath
> > >> saml.identityProviderMetadataPath
> > >> saml.maximumAuthenticationLifetime
> > >> saml.serviceProviderEntityId
> > >> saml.serviceProviderMetadataPath
> > >> cas.loginUrl
> > >> cas.protocol
> > >> oidc.id
> > >> oidc.secret
> > >> oidc.discoveryUri
> > >> oidc.customParamKey1
> > >> oidc.customParamValue1
> > >>
> > >>
> > >> If you define multiple clients, the first one will be used for
> > >> authentication, but you can explicitly choose the client you want to
> use
> > >> via the clientName parameter, assuming you want to switch from client
> > >> depending on environment for example.
> > >>
> > >> So if you want to add some new authentication mechanism, you must
> first
> > >> check that it is available in pac4j (if it's not, it's another
> > discussion,
> > >> but generally, it is). Then, you'll need to upgrade the
> > >> ConfigPropertiesFactory by submitting a new pull request to the pac4j
> > >> project (I can do it myself, but I'm sure you could do that easily),
> > >> finally wait for the new pac4j release and switch pac4j versions in
> Knox
> > >> to
> > >> benefit from the new feature.
> > >> The good thing is that if someone related to the CAS server does the
> > same
> > >> thing for CAS (in pac4j), you will automatically get it when you'll
> > >> upgrade
> > >> pac4j.
> > >>
> > >> To go even further, replacing LDAP Shiro authentication is just a
> matter
> > >> of
> > >> making pac4j LDAP authentication available via configuration
> parameters.
> > >>
> > >> I hope it was clear enough.
> > >>
> > >> Thanks.
> > >> Best regards,
> > >> Jérôme
> > >>
> > >>
> > >>
> > >>
> > >> 2015-12-03 20:45 GMT+01:00 larry mccay <larry.mc...@gmail.com>:
> > >>
> > >> > Excellent!
> > >> >
> > >> > I will carve out some time to do code review.
> > >> > We will need to get some insights into how to go about testing:
> > >> >
> > >> > * is the CAS server going to be available for testing?
> > >> > * what are the specific and generic/standard (if any) authentication
> > >> > mechanisms available - for instance:
> > >> >     - Facebook, Google, LinkedIn and CAS are specifics
> > >> >     - OAuth 2, OpenID Connect, SAML are generic/standards - that may
> > be
> > >> > used for the above specifics...
> > >> > * how do we test things other than CAS - in terms of getting
> > >> credentials,
> > >> > configuration, etc
> > >> >
> > >> > We could certainly do this is phases as well.
> > >> >
> > >> > If you can enumerate the things that should work and provide some
> > >> testing
> > >> > details for CAS or as many as possible and OpenID Connect then we
> can
> > >> test
> > >> > the specific implementations that you provide and enable the testing
> > of
> > >> > another OpenID Connect effort that is in the works in the community.
> > >> >
> > >> > I'm not sure whether we want to commit contributions that are
> > dependent
> > >> on
> > >> > snapshots - we certainly can't release with any such dependencies.
> > >> > I would hate to add a cleanup task to a release to make sure there
> are
> > >> no
> > >> > snapshots in there.
> > >> > We will probably wait until after the pac4j releases to commit.
> > >> >
> > >> > I am really happy that this integration is happening and that it
> went
> > >> > rather smoothly.
> > >> > These sorts of authentication protocols are complex and I think we
> > >> lined up
> > >> > pretty well overall.
> > >> >
> > >> > Thanks for your work!
> > >> >
> > >> > On Thu, Dec 3, 2015 at 2:28 PM, Jérôme LELEU <lel...@gmail.com>
> > wrote:
> > >> >
> > >> > > Hi,
> > >> > >
> > >> > > I just sync'ed with master, cleaned dependencies and added missing
> > >> > > Javadocs. Everything works correctly now. Many thanks.
> > >> > >
> > >> > > The pull request is ready for a full code review:
> > >> > > https://github.com/apache/knox/pull/2
> > >> > >
> > >> > > I'll write the documentation after the pac4j releases (I hope next
> > >> week).
> > >> > >
> > >> > > Thanks.
> > >> > > Best regards,
> > >> > > Jérôme
> > >> > >
> > >> > >
> > >> > > 2015-12-02 19:18 GMT+01:00 larry mccay <larry.mc...@gmail.com>:
> > >> > >
> > >> > > > Fixed in https://issues.apache.org/jira/browse/KNOX-636.
> > >> > > >
> > >> > > > On Wed, Dec 2, 2015 at 12:42 PM, larry mccay <
> > larry.mc...@gmail.com
> > >> >
> > >> > > > wrote:
> > >> > > >
> > >> > > > > Sure - I can file a JIRA and commit a fix.
> > >> > > > >
> > >> > > > > The secret generation should be done in one instance and
> > >> replicated
> > >> > > > across
> > >> > > > > others.
> > >> > > > > This replication/management of the credential stores is
> outside
> > of
> > >> > the
> > >> > > > > scope of Knox itself as of now.
> > >> > > > >
> > >> > > > > Documentation is done in markdown and is contributing details
> > are
> > >> > > > > available at:
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process#ContributionProcess-DocumentationContributorWorkflow
> > >> > > > >
> > >> > > > > Which should give you a general idea.
> > >> > > > >
> > >> > > > > Find an example like: ./trunk/books/0.7.0/
> > >> > > config_preauth_sso_provider.md
> > >> > > > >
> > >> > > > > For an example of typical content and format.
> > >> > > > >
> > >> > > > > Here is how that example renders:
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> http://knox.apache.org/books/knox-0-7-0/user-guide.html#Preauthenticated+SSO+Provider
> > >> > > > >
> > >> > > > > You'll need to tie it into the rest of the book - just grep
> for
> > >> where
> > >> > > > that
> > >> > > > > filename is referenced.
> > >> > > > > To test how it renders build the site with: "ant" and note the
> > >> url to
> > >> > > the
> > >> > > > > 0.7.0 book.
> > >> > > > >
> > >> > > > >
> > >> > > > > On Wed, Dec 2, 2015 at 12:12 PM, Jérôme LELEU <
> lel...@gmail.com
> > >
> > >> > > wrote:
> > >> > > > >
> > >> > > > >> Hi,
> > >> > > > >>
> > >> > > > >> Why it doesn't work for pac4j while it works for others is a
> > bit
> > >> > > strange
> > >> > > > >> to
> > >> > > > >> me, but if you have the patch in front of your eyes, I'd
> rather
> > >> > prefer
> > >> > > > you
> > >> > > > >> to commit it. In all cases, I'll sync with the master.
> > >> > > > >>
> > >> > > > >> There was one question you didn't answer previously: is the
> > >> password
> > >> > > > >> generated for the pac4j provider the same across all gateway
> > >> > > instances?
> > >> > > > >> Because I expect to have the same value as I use it to
> encrypt
> > /
> > >> > > decrypt
> > >> > > > >> data.
> > >> > > > >>
> > >> > > > >> I will add the Javadoc. After that, you can review the pull
> > >> request
> > >> > > more
> > >> > > > >> completely.
> > >> > > > >>
> > >> > > > >> What do you expect for the documentation?
> > >> > > > >>
> > >> > > > >> Notice that pac4j dependencies are still snapshots, but they
> > >> will be
> > >> > > > >> released in a week or two.
> > >> > > > >>
> > >> > > > >> Thanks.
> > >> > > > >> Best regards,
> > >> > > > >> Jérôme
> > >> > > > >>
> > >> > > > >>
> > >> > > > >> 2015-12-02 17:51 GMT+01:00 larry mccay <
> larry.mc...@gmail.com
> > >:
> > >> > > > >>
> > >> > > > >> > Jérôme -
> > >> > > > >> >
> > >> > > > >> > If you would like to add that change as part of your patch
> or
> > >> as a
> > >> > > > >> > separately filed JIRA to fix a bug that would certainly be
> > >> > welcomed.
> > >> > > > >> > Otherwise, I can do it.
> > >> > > > >> >
> > >> > > > >> > Let me know.
> > >> > > > >> >
> > >> > > > >> > thanks,
> > >> > > > >> >
> > >> > > > >> > --larry
> > >> > > > >> >
> > >> > > > >> > On Wed, Dec 2, 2015 at 11:44 AM, larry mccay <
> > >> > larry.mc...@gmail.com
> > >> > > >
> > >> > > > >> > wrote:
> > >> > > > >> >
> > >> > > > >> > > Okay - I had to add an override of getUserPrincipal() to
> > the
> > >> > > > >> > > IdentityAsserterHttpServletRequestWrapper and return the
> > >> member
> > >> > > > >> variable
> > >> > > > >> > > username and it works like a charm.
> > >> > > > >> > >
> > >> > > > >> > > Why I haven't seen this same behavior with other
> providers
> > >> is a
> > >> > > bit
> > >> > > > >> of a
> > >> > > > >> > > mystery but they must be adding other wrappers that
> handle
> > >> it.
> > >> > > > >> > > This is quite cool, Jérôme!
> > >> > > > >> > >
> > >> > > > >> > > On Wed, Dec 2, 2015 at 10:41 AM, larry mccay <
> > >> > > larry.mc...@gmail.com
> > >> > > > >
> > >> > > > >> > > wrote:
> > >> > > > >> > >
> > >> > > > >> > >> That was it - thanks!
> > >> > > > >> > >>
> > >> > > > >> > >> On Wed, Dec 2, 2015 at 10:20 AM, Jérôme LELEU <
> > >> > lel...@gmail.com>
> > >> > > > >> wrote:
> > >> > > > >> > >>
> > >> > > > >> > >>> This is my exact command line: mvn -Prelease clean
> > install
> > >> > > > >> -DskipTests
> > >> > > > >> > >>>
> > >> > > > >> > >>> You use an internal Maven repository to fetch
> > dependencies
> > >> > from
> > >> > > > >> > internet:
> > >> > > > >> > >>>
> > >> > > http://nexus-private.hortonworks.com/nexus/content/groups/public/
> > >> > > > >> > >>>
> > >> > > > >> > >>> Does this repository have access to the remote
> Snapshots
> > >> > > Sonatype
> > >> > > > >> repo?
> > >> > > > >> > >>>
> > >> > > > >> > >>>
> > >> > > > >> > >>>
> > >> > > > >> > >>> 2015-12-02 16:16 GMT+01:00 larry mccay <
> > >> larry.mc...@gmail.com
> > >> > >:
> > >> > > > >> > >>>
> > >> > > > >> > >>> > hmmm - I used:
> > >> > > > >> > >>> >
> > >> > > > >> > >>> > mvn clean install -DskipTests=true -Prelease
> > >> > > > >> > >>> >
> > >> > > > >> > >>> > The repository entry is in there already.
> > >> > > > >> > >>> > No worky.
> > >> > > > >> > >>> >
> > >> > > > >> > >>> > On Wed, Dec 2, 2015 at 10:12 AM, Jérôme LELEU <
> > >> > > lel...@gmail.com
> > >> > > > >
> > >> > > > >> > >>> wrote:
> > >> > > > >> > >>> >
> > >> > > > >> > >>> > > Hi,
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > You need the j2e-pac4j dependencies as well as the
> > >> pac4j-*
> > >> > > > >> > >>> dependencies,
> > >> > > > >> > >>> > > but you don't need to build them locally
> (hopefully).
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > But you need a dependency on the Sonatype snapshots
> > >> > > repository
> > >> > > > >> > >>> (where the
> > >> > > > >> > >>> > > snapshot versions are hosted), which is added for
> > >> Maven in
> > >> > > the
> > >> > > > >> root
> > >> > > > >> > >>> > > pom.xml:
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> >
> > >> > > > >> > >>>
> > >> > > > >> >
> > >> > > > >>
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://github.com/apache/knox/pull/2/files#diff-600376dffeb79835ede4a0b285078036R123
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > If you use Ant for the build, there is maybe a
> glitch
> > >> to
> > >> > > find
> > >> > > > >> the
> > >> > > > >> > >>> > Sonatype
> > >> > > > >> > >>> > > Maven repo.
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > Thanks.
> > >> > > > >> > >>> > > Best regards,
> > >> > > > >> > >>> > > Jérôme
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > 2015-12-02 16:06 GMT+01:00 larry mccay <
> > >> > > larry.mc...@gmail.com
> > >> > > > >:
> > >> > > > >> > >>> > >
> > >> > > > >> > >>> > > > Oh - do I need to build j2e-pac4 locally in order
> > to
> > >> > > resolve
> > >> > > > >> the
> > >> > > > >> > >>> > > > dependencies?
> > >> > > > >> > >>> > > >
> > >> > > > >> > >>> > > > [ERROR] Failed to execute goal on project
> > >> > > > >> > >>> > > gateway-provider-security-pac4j:
> > >> > > > >> > >>> > > > Could not resolve dependencies for project
> > >> > > > >> > >>> > > >
> > >> > > > >> > >>>
> > >> > > >
> > org.apache.knox:gateway-provider-security-pac4j:jar:0.7.0-SNAPSHOT:
> > >> > > > >> The
> > >> > > > >> > >>> > > > following artifacts could not be resolved:
> > >> > > > >> > >>> > > > org.pac4j:j2e-pac4j:jar:1.2.1-SNAPSHOT,
> > >> > > > >> > >>> > > > org.pac4j:pac4j-http:jar:1.8.1-SNAPSHOT,
> > >> > > > >> > >>> > > > org.pac4j:pac4j-config:jar:1.8.1-SNAPSHOT: Could
> > not
> > >> > find
> > >> > > > >> > artifact
> > >> > > > >> > >>> > > > org.pac4j:j2e-pac4j:jar:1.2.1-SNAPSHOT in public
> (
> > >> > > > >> > >>> > > >
> > >> > > > >> >
> > >> http://nexus-private.hortonworks.com/nexus/content/groups/public/
> > >> > )
> > >> > > > >> > >>> ->
> > >> > > > >> > >>> > > > [Help
> > >> > > > >> > >>> > > > 1]
> > >> > > > >> > >>> > > >
> > >> > > > >> > >>> > > > On Wed, Dec 2, 2015 at 10:05 AM, larry mccay <
> > >> > > > >> > >>> larry.mc...@gmail.com>
> > >> > > > >> > >>> > > > wrote:
> > >> > > > >> > >>> > > >
> > >> > > > >> > >>> > > > > gateway-provider-security-pac4j doesn't build -
> > do
> > >> you
> > >> > > > have
> > >> > > > >> a
> > >> > > > >> > >>> pending
> > >> > > > >> > >>> > > > > change for your pom.xml or something?
> > >> > > > >> > >>> > > > >
> > >> > > > >> > >>> >
> > >> > > > >> > >>>
> > >> > > > >> > >>
> > >> > > > >> > >>
> > >> > > > >> > >
> > >> > > > >> >
> > >> > > > >>
> > >> > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

Reply via email to