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

[JENKINS-34350] Fix POST to /git/notifyCommit with CSRF protection on #491

Merged
merged 2 commits into from Apr 27, 2017

Conversation

Projects
None yet
2 participants
@liskin
Copy link

commented Apr 21, 2017

No description provided.

@MarkEWaite
Copy link

left a comment

Please add a test which shows that a POST (rather than the GET I was testing) shows the problem with CSRF enabled. I want to have an automated test which confirms this bug stays fixed.

@MarkEWaite

This comment has been minimized.

Copy link

commented Apr 27, 2017

I created an integration test to confirm I could see the failure when a POST to /git/notifyCommit with CSRF enabled, and I've confirmed that this change resolves the failure.

@liskin

This comment has been minimized.

Copy link
Author

commented Apr 27, 2017

Oh, I was just about to submit a unit test I've been writing all day. :-)

@MarkEWaite

This comment has been minimized.

Copy link

commented Apr 27, 2017

Please submit the unit test. Unit tests are much better (for the developers) than integration tests. The integration test I created is outside the plugin repository and requires a full running Jenkins environment to run. A unit test will run for developers on their desktop without requiring a full Jenkins.

@liskin

This comment has been minimized.

Copy link
Author

commented Apr 27, 2017

Pushed. Please do note that this is my first experience with Jenkins codebase, Java unit tests in general and maven, so it might not be up to standards. Also I just noticed that my files use tabs instead of spaces. I should probably amend that.

@MarkEWaite MarkEWaite merged commit fd68967 into jenkinsci:master Apr 27, 2017

1 of 2 checks passed

Jenkins Jenkins is validating pull request ...
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details

@liskin liskin deleted the liskin:JENKINS-34350-notifycommit-csrf branch Apr 28, 2017

@MarkEWaite MarkEWaite added the bug label Jul 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.