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

Add client certificate support #596

Merged

Conversation

itzikbekelmicrosoft
Copy link
Contributor

  • Added functionality to support Azure AD application certificate authentication.
  • Updated documentation to include steps for setting up certificate authentication.
  • Ensured that client secret authentication remains the default for existing setups to avoid breaking changes.

Testing done

  • Local Testing: Successfully tested the new certificate authentication feature on a local Jenkins instance.
  • Azure VM Testing: Deployed and tested on a Jenkins instance container running in an Azure VM to ensure compatibility and functionality.

Submitter checklist

  • 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

@itzikbekelmicrosoft itzikbekelmicrosoft marked this pull request as draft July 15, 2024 21:03
@itzikbekelmicrosoft itzikbekelmicrosoft marked this pull request as ready for review July 15, 2024 21:03
@itzikbekelmicrosoft
Copy link
Contributor Author

@timja Please review

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested that client secret still works and made some minor UI tweaks.

I haven't tested the client secret functionality but code looks good

@itzikbekelmicrosoft
Copy link
Contributor Author

itzikbekelmicrosoft commented Jul 16, 2024

I've tested that client secret still works and made some minor UI tweaks.

I haven't tested the client secret functionality but code looks good

You meant you haven't tested the client certificate right? I have tested it with two different certificates and it functions as expected.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really close, I think the tests could be improved a little bit though

@KalleOlaviNiemitalo
Copy link

There are some whitespace issues: removal of final newlines in files, varying numbers of blank lines between methods, extra indentation in one method. Not a big deal though.

@timja timja merged commit c478593 into jenkinsci:master Jul 17, 2024
16 checks passed
@timja
Copy link
Member

timja commented Jul 17, 2024

Thanks for the contribution!

@itzikbekelmicrosoft itzikbekelmicrosoft deleted the add-client-certificate-support branch July 17, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants