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 email and display name header injection support - revised from PR #30 #124

Merged
merged 13 commits into from
Jan 25, 2024

Conversation

akrasinsky
Copy link
Contributor

This is a revised version from PR #30

Original text:

Reverse-proxy-auth-plugin supports name & groups header injections but emails and display name are fetched from the LDAP server.

This fork add email and display name header injection support.
Thus we can manage authentication and user profile data with an httpd server.

What's more, using an httpd server as a reverse proxy, provides a simple solution to:

  • connect to several LDAP servers (even with different structures)
  • use technical users managed in the httpd server
  • create an SSO plateform with other integrated applications

The original fork was made on tag 1.4.

@akrasinsky
Copy link
Contributor Author

@sboardwell Hi could you have a look at this?

@sboardwell
Copy link
Contributor

@sboardwell Hi could you have a look at this?

Sorry, missed this somehow. Will check in the next few days. Thanks 👍

@sboardwell sboardwell self-requested a review December 14, 2023 10:17
@sboardwell sboardwell self-assigned this Dec 14, 2023
Copy link
Contributor

@sboardwell sboardwell left a comment

Choose a reason for hiding this comment

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

Looks good. I've added a few nitpicks wrt code styling. Would you be able to add a test as well?

if(forwardedEmail!=null){
toReturn.setEmail(r.getHeader(forwardedEmail));
}
if(forwardedDisplayName!=null){
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(forwardedDisplayName!=null){
if (forwardedDisplayName != null) {

@akrasinsky
Copy link
Contributor Author

Thank you, I'll have a look what I can test.

@akrasinsky
Copy link
Contributor Author

I added some basic tests

@sboardwell
Copy link
Contributor

I have added automated code formatting to the project to avoid having to think about it. This has caused some conflicts - apologies. Could you resolve them please, or would you like me to do it?

@akrasinsky
Copy link
Contributor Author

It would be great if you could resolve the issue.

@sboardwell sboardwell merged commit ad24059 into jenkinsci:master Jan 25, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants