-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add redirect_uri oauth param and configuration #198
base: master
Are you sure you want to change the base?
Conversation
7d3aece
to
6d88ee2
Compare
6d88ee2
to
82101ad
Compare
@basil can you take a look? |
Hi @tdaniely-dn, as of jenkins-infra/repository-permissions-updater#2590 I have removed myself from the list of maintainers for this plugin. I no longer have write access to merge PRs, and I no longer have Artifactory access to perform releases. If the changes from this PR work, you can consider adopting the plugin to merge and release the PR. The "Contributing to Open Source" workshop from DevOps World 2021 is a useful starting point for new maintainers. That document includes links to a five part video series that illustrates many of the steps. If the plugin is crucial to your work, you may want to ask your employer to support your work efforts by allowing you to adopt the plugin. |
@basil Thank you for the references, and thank you for the contributions. I'll look into adopting. |
Oh nice @tdaniely-dn, we would love to have this as part of the standard!!!! |
@tdaniely-dn, this is really cool feature. Hope we can get this soon! |
@Moofasax and @idrislaw the comment from the earlier message applies to you as well. You could adopt the plugin, merge the pull request, and release it. It would help you and help the Jenkins community.
If you'd prefer a more polished tutorial, the "Improve a Plugin" tutorial provides some of the same information in a more polished form. If you're not able to adopt the plugin, you could also install the most recent build of the plugin on your controller and report the results of using it. That would meet your needs and provide feedback to others that you've tested the plugin and confirmed that the feature works in a way that meets your needs. |
@MarkEWaite thanks for this, I will take a look. I don't know much Java, but I'll check out those links. We have been using this PR in production with the latest LTS version (2.387.3-lts) of Jenkins. We run about 15 or 20 separate Jenkins instances, with tons of other plugins on them and haven't had any issues with this PR! I can send screenshots of it installed, or if I can provide anymore feedback on its function, let me know!! |
Co-authored-by: Andreas Nygard <andreas.nygard@gmail.com>
Co-authored-by: Andreas Nygard <andreas.nygard@gmail.com>
…support Add Jenkins agent support for GitHub Committer Authorization Strategy
I'm trying to automate a lot of new jenkins to automatically launch and allow them to authenticate with github oauth. |
I am using the plugin that includes the redirect_uri function in jenkins 2.401.3 version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment around conventions, otherwise LGTM. I will aim to test this soon.
@@ -115,6 +118,8 @@ public class GithubSecurityRealm extends AbstractPasswordBasedSecurityRealm impl | |||
private Secret clientSecret; | |||
private String oauthScopes; | |||
private String[] myScopes; | |||
@NonNull | |||
private String redirectUri = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is well intended, as we are not using @nonnull anywhere else in this project I don't think it makes sense to use it here. It would be 'unfair' to all of the other properties/fields, as this one effectively gets arbitrary treatment.
Could you please remove this annotation, and we can discuss adding it as a separate PR?
https://issues.jenkins.io/browse/JENKINS-43214
Continue work from #111
Add request_uri parameter according to:
https://docs.github.com/en/developers/apps/building-oauth-apps/authorizing-oauth-apps
https://docs.github.com/en/developers/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps
Support:
Just to comment, GitHub App now does support multiple callback domains, so the proxy scenario is less likely.
Still made the value overridable just in case, but the default will be the correct route for the server.
Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue
Manually test mechanism (Tested redirect, manual conf, and JCasc with both valid an invalid values)