Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributing to a new nuxeo login plugin with Keycloak SSO #9

Closed
wants to merge 1 commit into from
Closed

Conversation

fmaturel
Copy link
Contributor

@fmaturel fmaturel commented Jul 3, 2015

Dear nuxeo community developpers and reviewers,
I just developped a new nuxeo plugin to allow users already logged on Keycloak SSO (a brand new Open Source JBoss project). This plugin will be used by a Startup in Rennes, France that uses Nuxeo community to store thousands of media.
We'd be glad that you integrate this plugin to your main stream community offer, and are ready to cross review this plugin code to see if it meets your requirements.
Hope to hear from you soon.

@efge
Copy link
Collaborator

efge commented Jul 10, 2015

Thanks François. We'll try to review this ASAP.

@tiry
Copy link

tiry commented Jul 10, 2015

Hi Francois,

Before we can integrate any code from you, we would need you to sign the contributor agreement.

https://doc.nuxeo.com/download/attachments/4687444/Nuxeo-Contributor-Agreement.pdf

Thank you !

@tiry
Copy link

tiry commented Jul 10, 2015

First feedback after a quick review :

  • this is good thank you
  • I may prefer removing the NuxeoUserService and either use a real Nuxeo service (i.e. use Nuxeo Runtime to make a singleton), this may be also an opportunity to integrate the nuxeo-usermapper
  • for the JSON config file, we may prefer to package it inside an XML extension to make it more consistent with the rest of the framework, but this is not really required

If you are ok and provide the contributor aggrement, I could try to find the time to :

  • integrate the usermapper inside Nuxeo
  • align you code to use it (as well as SAML plugin)
  • merge your code

Tiry

@fmaturel
Copy link
Contributor Author

Hi,

Just sent the contributor agreement signed to the nuxeo contact email address.

  • I'm ok to remove the NuxeoUserService, and rather use the nuxeo-usermapper that I did not know about.
  • Regarging the JSON config file, this is how keycloak handles the SSO configuration (you can produce the JSON from UI), and It makes sense to keep the file as is.
  • I had a conflict on the Jackson JSON marshaller between Nuxeo and Keycloak (Nuxeo is 1.8.x where Keycloak is 1.9.x). The bad news is that Keycloak is using a method that is not in 1.8.x Jackson... So I had to tweek keycloak a little bit to avoid using this method (ObjectMapper.setSerializationInclusion)

Thanks for your quick review.
Please do whatever needs to be done so that this can me merged.

@tiry
Copy link

tiry commented Aug 27, 2015

Hi,

Just FYI, I started the infrastructure work on https://github.com/nuxeo/nuxeo-platform-login/commits/feature-NXP-15701-IDM-integration : the idea being to integrate the UserMapper inside the default code.

Next step will be to integrate your module.

@tiry
Copy link

tiry commented Aug 27, 2015

Code has been integrated in a branch :

I will close the PR as soon as the branch is merged.

@fmaturel
Copy link
Contributor Author

Thanks!

@efge efge closed this Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants