-
-
Notifications
You must be signed in to change notification settings - Fork 67
[FIX JENKINS-20520] Save Jenkins after adding global user permission #23
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
Conversation
b458cd7 to
271cf2c
Compare
3d69656 to
b7708df
Compare
| @Issue("JENKINS-20520") | ||
| public void ensureSavingAfterInitialUser() { | ||
| r.addStep(new Statement() { | ||
| @Override |
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.
BTW once you are on 2.60.x+ you can make this a lot prettier with lambdas.
| public void evaluate() throws Throwable { | ||
| r.j.jenkins.setSecurityRealm(new HudsonPrivateSecurityRealm(true)); | ||
| r.j.jenkins.setAuthorizationStrategy(new GlobalMatrixAuthorizationStrategy()); | ||
| r.j.jenkins.save(); |
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.
BTW in 2.51+ this is unnecessary: jenkinsci/jenkins#2790
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.
I want this to be explicit.
| import hudson.security.ACLContext; | ||
| import hudson.security.GlobalMatrixAuthorizationStrategy; | ||
| import hudson.security.HudsonPrivateSecurityRealm; | ||
| import hudson.security.pages.SignupPage; |
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.
Wow, never heard of this before. Once the private realm gets moved into a plugin (?), I guess this should go into a test-jar.
| HtmlPage success = signup.submit(r.j); | ||
|
|
||
| try (ACLContext _ = ACL.as(User.get("alice"))) { | ||
| Assert.assertTrue(r.j.jenkins.getACL().hasPermission(Jenkins.ADMINISTER)); |
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.
More simply:
assertTrue(r.j.jenkins.hasPermission(Jenkins.ADMINISTER));More simply still, delete the try block and:
assertTrue(r.j.jenkins.getACL().hasPermission(User.get("alice").impersonate(), Jenkins.ADMINISTER));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.
Thanks, used this in #22 but forgot to update it here when I remembered how this works.
| signup.enterFullName("Alice User"); | ||
| HtmlPage success = signup.submit(r.j); | ||
|
|
||
| Assert.assertTrue(r.j.jenkins.getACL().hasPermission(User.get("alice").impersonate(), Jenkins.ADMINISTER)); |
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.
Failing in jenkinsci/bom#359 for reasons I am still struggling to understand: User.get("alice", false, Collections.emptyMap()) returns null. Only when jth.jenkins-war.path is set to the megawar.
JENKINS-20520
@jglick