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

[FIXED JENKINS-31915] Proxy settings in plugins page are ignored #1955

Merged
merged 3 commits into from Jun 2, 2016

Conversation

7 participants
@escoem
Contributor

escoem commented Dec 16, 2015

Added NTLM support to doValidateProxy and noProxyHost is never ignored.

@reviewbybees

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees Dec 16, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

if (userName.indexOf('\\') >= 0){
final String domain = userName.substring(0, userName.indexOf('\\'));
final String user = userName.substring(userName.indexOf('\\') + 1);
return new NTCredentials(user, Secret.fromString(password).getPlainText(), domain, "");

This comment has been minimized.

@stephenc

stephenc Dec 16, 2015

Member

what happens if you are connecting to a non-NTLM proxy where \ is a valid character in the username?

@stephenc

stephenc Dec 16, 2015

Member

what happens if you are connecting to a non-NTLM proxy where \ is a valid character in the username?

@stephenc

This comment has been minimized.

Show comment
Hide comment
@stephenc

stephenc Dec 16, 2015

Member

🐝 but I do wonder about the case where the proxy is using crazy characters in the username

Member

stephenc commented Dec 16, 2015

🐝 but I do wonder about the case where the proxy is using crazy characters in the username

@amuniz

This comment has been minimized.

Show comment
Hide comment
@amuniz

amuniz Dec 16, 2015

Contributor

I agree that the \ in usernames non-ntlm could be a problem. This could be fixed (without breaking compatibility with previous configurations) by adding a configuration option (a checkbox) to indicate if you are using NTLM auth or not (disabled by default), so userName.indexOf('\\') >= 0 would be only checked if NTLM is active.

This is a 🐝 from me, but I would encourage you to consider that option.

Contributor

amuniz commented Dec 16, 2015

I agree that the \ in usernames non-ntlm could be a problem. This could be fixed (without breaking compatibility with previous configurations) by adding a configuration option (a checkbox) to indicate if you are using NTLM auth or not (disabled by default), so userName.indexOf('\\') >= 0 would be only checked if NTLM is active.

This is a 🐝 from me, but I would encourage you to consider that option.

@escoem

This comment has been minimized.

Show comment
Hide comment
@escoem

escoem Dec 17, 2015

Contributor

Please @stephenc , @amuniz , I have updated the help file removing obsolete references to setting configuration as system property.

Contributor

escoem commented Dec 17, 2015

Please @stephenc , @amuniz , I have updated the help file removing obsolete references to setting configuration as system property.

@amuniz

This comment has been minimized.

Show comment
Hide comment
@amuniz

amuniz Dec 18, 2015

Contributor

re-🐝ing

Contributor

amuniz commented Dec 18, 2015

re-🐝ing

@andresrc

This comment has been minimized.

Show comment
Hide comment
@andresrc

andresrc Dec 18, 2015

Contributor

🐝

Regarding the \ in user names, I assume that, as we say in spanish, "when we reach that river, we will walk through that bridge" 😉

Contributor

andresrc commented Dec 18, 2015

🐝

Regarding the \ in user names, I assume that, as we say in spanish, "when we reach that river, we will walk through that bridge" 😉

@escoem

This comment has been minimized.

Show comment
Hide comment
@escoem

escoem Dec 18, 2015

Contributor

Thanks!

@reviewbybees done

Contributor

escoem commented Dec 18, 2015

Thanks!

@reviewbybees done

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev May 8, 2016

Member

@escoem Merge conflict :(

Member

oleg-nenashev commented May 8, 2016

@escoem Merge conflict :(

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
Member

oleg-nenashev commented May 14, 2016

@escoem bump

@escoem

This comment has been minimized.

Show comment
Hide comment
@escoem

escoem May 14, 2016

Contributor

hi @oleg-nenashev I have to review it and I will do asap. :-)

Contributor

escoem commented May 14, 2016

hi @oleg-nenashev I have to review it and I will do asap. :-)

@oleg-nenashev oleg-nenashev changed the title from [JENKINS-31915] Proxy settings in plugins page are ignored to [FIXED JENKINS-31915] Proxy settings in plugins page are ignored Jun 2, 2016

@oleg-nenashev oleg-nenashev merged commit 81e00cc into jenkinsci:master Jun 2, 2016

1 check passed

Jenkins This pull request looks good
Details

oleg-nenashev added a commit that referenced this pull request Jun 4, 2016

olivergondza added a commit that referenced this pull request Jun 19, 2016

[FIXED JENKINS-31915] Proxy settings in plugins page are ignored (#1955)
[FIXED JENKINS-31915] Proxy settings in plugins page are ignored
(cherry picked from commit 81e00cc)

samatdav added a commit to samatdav/jenkins that referenced this pull request Jul 25, 2016

[FIXED JENKINS-31915] Proxy settings in plugins page are ignored (#1955)
[FIXED JENKINS-31915] Proxy settings in plugins page are ignored

samatdav added a commit to samatdav/jenkins that referenced this pull request Jul 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment