github-oauth-plugin
github-oauth-plugin copied to clipboard
WIP: Add email domain filter
This is useful for operators who have users whose Github accounts have multiple email addresses and their default address is a personal address. The current implementation leads to Jenkins storing the default github address, which in some cases is a personal account. This change allows an operator to specify the domain of their org, and the plugin will look at all the emails for a user and if it finds one that matches the specified domain it will prefer that email over the primary. If the setting is empty it will behave as before.
I also added a checkbox to always override the stored value with what is found in Github.
This is very much a work in progress. I'm not an experienced Java developer and I did this in one working day. It probably also needs some tests for the new functionality. I'd appreciate feedback.
This makes sense. I'll take a look.
@jenkinsci/code-reviewers Please check out this change.
@jdmulloy mind if I make a feature request? A CSV list of domains in order of preference. Some companies have contractors who would be using contractor emails. If not, that's fine, I could try adding it later.
@samrocketman I've added in some logging and added the csv feature.
Thanks for humoring my request @jdmulloy. I'll test running and upgrading to your version to ensure stability. If all goes well I'll merge it. I think this is a pretty cool feature.
Once it's approved I can squash the commits to make the history cleaner.
I have not forgotten about this change. I was going to look at it over the weekend and did not get a chance. I'll see about looking at it during the evenings this week.
@samrocketman I'll work on overloading the method and fixing the tests. How do I run the tests that are failing? When I build the plugin I don't get any test failures, so there is probably something I'm missing. I can't find any failing tests via this PR, but I may not know where to look.
Thanks
Actually I think I figured out what you were saying. You were talking about the tests that were failing until I changed them. Correct?
@samrocketman I added the deprecated method, but it fails to compile with the following error. I can't figure out why it can't find the method. I've tried googling.
[INFO]
[INFO] --- maven-resources-plugin:2.7:resources (default-resources) @ github-oauth ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 4 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @ github-oauth ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 11 source files to /home/jdmulloy/git/github-oauth-plugin/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/jdmulloy/git/github-oauth-plugin/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java:[164,9] cannot find symbol
symbol: method GithubSecurityRealm(java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.Boolean)
location: class org.jenkinsci.plugins.GithubSecurityRealm
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 8.973 s
[INFO] Finished at: 2018-02-14T12:36:43-05:00
[INFO] Final Memory: 41M/155M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project github-oauth: Compilation failure
[ERROR] /home/jdmulloy/git/github-oauth-plugin/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java:[164,9] cannot find symbol
[ERROR] symbol: method GithubSecurityRealm(java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.Boolean)
[ERROR] location: class org.jenkinsci.plugins.GithubSecurityRealm
[ERROR]
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Let me know when your'e ready for me to review/test this change for merging.
Please squash and rebase on master.
@samrocketman I've squashed and rebased on master. I tested upgrading the plugin and it works now. I had to fix handling of the forceGithubEmail setting being missing in config.xml. The unmarshaling code was setting it to Null instead of False.
The todo comment for handling the boolean forceGithubEmail is still there. The useSecurity setting in core uses "true" in config.xml to store true, but I couldn't find the relevant marshaling and unmarshaling code for that. So I couldn't figure out how that's done in core.
Thanks I'll look it over and test it.
Upgraded by uploading via the Advanced
and checking the restart Jenkins checkbox. Jenkins threw a stacktrace upon upgrade.
Click here for stack trace
java.lang.NullPointerException
at com.thoughtworks.xstream.io.xml.PrettyPrintWriter.writeText(PrettyPrintWriter.java:235)
at com.thoughtworks.xstream.io.xml.PrettyPrintWriter.writeText(PrettyPrintWriter.java:231)
at com.thoughtworks.xstream.io.xml.PrettyPrintWriter.setValue(PrettyPrintWriter.java:214)
at com.thoughtworks.xstream.io.WriterWrapper.setValue(WriterWrapper.java:45)
at org.jenkinsci.plugins.GithubSecurityRealm$ConverterImpl.marshal(GithubSecurityRealm.java:285)
at hudson.util.XStream2$AssociatedConverterImpl.marshal(XStream2.java:370)
at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:69)
at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:58)
at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:84)
at hudson.util.RobustReflectionConverter.marshallField(RobustReflectionConverter.java:265)
at hudson.util.RobustReflectionConverter$2.writeField(RobustReflectionConverter.java:252)
Caused: java.lang.RuntimeException: Failed to serialize jenkins.model.Jenkins#securityRealm for class hudson.model.Hudson
at hudson.util.RobustReflectionConverter$2.writeField(RobustReflectionConverter.java:256)
at hudson.util.RobustReflectionConverter$2.visit(RobustReflectionConverter.java:224)
at com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider.visitSerializableFields(PureJavaReflectionProvider.java:138)
at hudson.util.RobustReflectionConverter.doMarshal(RobustReflectionConverter.java:209)
at hudson.util.RobustReflectionConverter.marshal(RobustReflectionConverter.java:150)
at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:69)
at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:58)
at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:43)
at com.thoughtworks.xstream.core.TreeMarshaller.start(TreeMarshaller.java:82)
at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.marshal(AbstractTreeMarshallingStrategy.java:37)
at com.thoughtworks.xstream.XStream.marshal(XStream.java:1026)
at com.thoughtworks.xstream.XStream.marshal(XStream.java:1015)
at com.thoughtworks.xstream.XStream.toXML(XStream.java:988)
at hudson.XmlFile.write(XmlFile.java:181)
at jenkins.model.Jenkins.save(Jenkins.java:3198)
at jenkins.model.Jenkins.saveQuietly(Jenkins.java:3204)
at jenkins.model.Jenkins.setSecurityRealm(Jenkins.java:2576)
at jenkins.model.Jenkins$17.run(Jenkins.java:3168)
at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:169)
at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:282)
at jenkins.model.Jenkins$5.runTask(Jenkins.java:1068)
at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:210)
at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
Caused: org.jvnet.hudson.reactor.ReactorException
at org.jvnet.hudson.reactor.Reactor.execute(Reactor.java:269)
at jenkins.InitReactorRunner.run(InitReactorRunner.java:47)
at jenkins.model.Jenkins.executeReactor(Jenkins.java:1102)
at jenkins.model.Jenkins.<init>(Jenkins.java:904)
at hudson.model.Hudson.<init>(Hudson.java:86)
at hudson.model.Hudson.<init>(Hudson.java:82)
at hudson.WebAppMain$3.run(WebAppMain.java:233)
Caused: hudson.util.HudsonFailedToLoad
at hudson.WebAppMain$3.run(WebAppMain.java:250)
To reproduce this issue I used the my jenkins-bootstrap-jervis project which you can use settings.groovy
to configure the GitHub OAuth plugin. Uncomment the bottom of settings.groovy
to configure github-oauth.
dependencies.gradle
of jenkins-bootstrap-jervis will show you what versions of plugins I tested.
However, when I restarted Jenkins the stacktrace went away.
I'm going to investigate this more before merging.
Also note, if you wanted to test this yourself without the complication of learning the scripts I publish packages of the version I tested. https://github.com/samrocketman/jenkins-bootstrap-jervis/releases/tag/jervis-bootstrap-2.89.4.3
@jdmulloy I just released a bunch of security fixes and a critical bug fix. Unfortunately, I didn't get a chance to test your change as part of the release. Would you mind merging in the latest master and resolving the merge conflicts?
If not, then I can take a look and update it after I move.