Skip to content

Commit

Permalink
[JENKINS-45387] Fix validation error displaying in setup wizard's "cr…
Browse files Browse the repository at this point in the history 
…eate first admin" form (#3116)

* [JENKINS-45387] Improve setup wizard account creation in security realm

This commit adds an additional account creation method to the
security realm, that allows to create a new user account (as the system)
and is intended to be used by the setup wizard.

The main difference to the existing method is that the new method does
not force the Stapler to send a response, but instead throws an
exception if invalid data is submitted. This allows to call this from
the setup wizard, and send the response there. (This is necessary
because the setup wizard method has a response return type and
there is no way to access the response already send in the realm)

Further, it splits the private createAccount() method for
clarity as well as to allow code reuse from the new method.

* [JENKINS-45387] Fix SetupWizard sending responses twice on create admin

This commit fixes an issue where the SetupWizard class would send
two responses (indirectly) when invalid form data was provided for
creating the first admin account.

* [Fix JENKINS-45387] Setup wizard not displaying first account errors

This commit fixes the setup wizard not displaying HTML error
responses upon first account creation.

Previously, it just froze (buttons were not re-enabled) and didn't
display responses. (Probably caused by XSS policies on the iframe)

* [JENKINS-45387] Add some @restricted annotations

As per
#3116 (comment)
#3116 (comment)
  • Loading branch information
@literalplus @oleg-nenashev
literalplus authored and oleg-nenashev committed Mar 7, 2018
1 parent 145c606 commit 12031d7
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 87 deletions.
40 changes: 40 additions & 0 deletions core/src/main/java/hudson/security/AccountCreationFailedException.java
View file
@@ -0,0 +1,40 @@
/*
* The MIT License
*
* Copyright (c) 2017 Jenkins contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.security;

import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Thrown if an account creation was attempted but failed due to invalid data being entered into a form.
*
* @author Philipp Nowak
*/
@Restricted(NoExternalUse.class)
public class AccountCreationFailedException extends Exception {
public AccountCreationFailedException(String message) {
super(message);
}
}
99 changes: 78 additions & 21 deletions core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
View file
Expand Up @@ -289,6 +289,26 @@ public User createAccountByAdmin(StaplerRequest req, StaplerResponse rsp, String
return u;
}

/**
* Creates a user account. Intended to be called from the setup wizard.
* Note that this method does not check whether it is actually called from
* the setup wizard. This requires the {@link Jenkins#ADMINISTER} permission.
*
* @param req the request to retrieve input data from
* @return the created user account, never null
* @throws AccountCreationFailedException if account creation failed due to invalid form input
*/
@Restricted(NoExternalUse.class)
public User createAccountFromSetupWizard(StaplerRequest req) throws IOException, AccountCreationFailedException {
checkPermission(Jenkins.ADMINISTER);
SignupInfo si = validateAccountCreationForm(req, false);
if (si.errorMessage != null) {
throw new AccountCreationFailedException(si.errorMessage);
} else {
return createAccount(si);
}
}

/**
* Creates a first admin user account.
*
Expand Down Expand Up @@ -321,73 +341,110 @@ private void tryToMakeAdmin(User u) {
}

/**
* @param req the request to get the form data from (is also used for redirection)
* @param rsp the response to use for forwarding if the creation fails
* @param validateCaptcha whether to attempt to validate a captcha in the request
* @param formView the view to redirect to if creation fails
*
* @return
* null if failed. The browser is already redirected to retry by the time this method returns.
* a valid {@link User} object if the user creation was successful.
*/
private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean selfRegistration, String formView) throws ServletException, IOException {
private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean validateCaptcha, String formView) throws ServletException, IOException {
SignupInfo si = validateAccountCreationForm(req, validateCaptcha);

if (si.errorMessage != null) {
// failed. ask the user to try again.
req.getView(this, formView).forward(req, rsp);
return null;
}

return createAccount(si);
}

/**
* @param req the request to process
* @param validateCaptcha whether to attempt to validate a captcha in the request
*
* @return a {@link SignupInfo#SignupInfo(StaplerRequest) SignupInfo from given request}, with {@link
* SignupInfo#errorMessage} set to a non-null value if any of the supported fields are invalid
*/
private SignupInfo validateAccountCreationForm(StaplerRequest req, boolean validateCaptcha) {
// form field validation
// this pattern needs to be generalized and moved to stapler
SignupInfo si = new SignupInfo(req);

if(selfRegistration && !validateCaptcha(si.captcha))
if (validateCaptcha && !validateCaptcha(si.captcha)) {
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_TextNotMatchWordInImage();
}

if(si.password1 != null && !si.password1.equals(si.password2))
if (si.password1 != null && !si.password1.equals(si.password2)) {
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_PasswordNotMatch();
}

if(!(si.password1 != null && si.password1.length() != 0))
if (!(si.password1 != null && si.password1.length() != 0)) {
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_PasswordRequired();
}

if(si.username==null || si.username.length()==0)
if (si.username == null || si.username.length() == 0) {
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameRequired();
else {
} else {
// do not create the user - we just want to check if the user already exists but is not a "login" user.
User user = User.getById(si.username, false);
User user = User.getById(si.username, false);
if (null != user)
// Allow sign up. SCM people has no such property.
if (user.getProperty(Details.class) != null)
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameAlreadyTaken();
}

if(si.fullname==null || si.fullname.length()==0)
if (si.fullname == null || si.fullname.length() == 0) {
si.fullname = si.username;
}

if(isMailerPluginPresent() && (si.email==null || !si.email.contains("@")))
if (isMailerPluginPresent() && (si.email == null || !si.email.contains("@"))) {
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_InvalidEmailAddress();
}

if (! User.isIdOrFullnameAllowed(si.username)) {
if (!User.isIdOrFullnameAllowed(si.username)) {
si.errorMessage = hudson.model.Messages.User_IllegalUsername(si.username);
}

if (! User.isIdOrFullnameAllowed(si.fullname)) {
if (!User.isIdOrFullnameAllowed(si.fullname)) {
si.errorMessage = hudson.model.Messages.User_IllegalFullname(si.fullname);
}
req.setAttribute("data", si); // for error messages in the view
return si;
}

if(si.errorMessage!=null) {
// failed. ask the user to try again.
req.setAttribute("data",si);
req.getView(this, formView).forward(req,rsp);
return null;
/**
* Creates a new account from a valid signup info. A signup info is valid if its {@link SignupInfo#errorMessage}
* field is null.
*
* @param si the valid signup info to create an account from
* @return a valid {@link User} object created from given signup info
* @throws IllegalArgumentException if an invalid signup info is passed
*/
private User createAccount(SignupInfo si) throws IOException {
if (si.errorMessage != null) {
throw new IllegalArgumentException("invalid signup info passed to createAccount(si): " + si.errorMessage);
}

// register the user
User user = createAccount(si.username,si.password1);
User user = createAccount(si.username, si.password1);
user.setFullName(si.fullname);
if(isMailerPluginPresent()) {
if (isMailerPluginPresent()) {
try {
// legacy hack. mail support has moved out to a separate plugin
Class<?> up = Jenkins.getInstance().pluginManager.uberClassLoader.loadClass("hudson.tasks.Mailer$UserProperty");
Constructor<?> c = up.getDeclaredConstructor(String.class);
user.addProperty((UserProperty)c.newInstance(si.email));
user.addProperty((UserProperty) c.newInstance(si.email));
} catch (ReflectiveOperationException e) {
throw new RuntimeException(e);
}
}
user.save();
return user;
}

@Restricted(NoExternalUse.class)
public boolean isMailerPluginPresent() {
try {
Expand Down
74 changes: 40 additions & 34 deletions core/src/main/java/jenkins/install/SetupWizard.java
View file
Expand Up @@ -39,6 +39,7 @@
import hudson.model.UpdateCenter;
import hudson.model.UpdateSite;
import hudson.model.User;
import hudson.security.AccountCreationFailedException;
import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
import hudson.security.HudsonPrivateSecurityRealm;
import hudson.security.SecurityRealm;
Expand Down Expand Up @@ -240,49 +241,54 @@ public boolean isUsingSecurityToken() {
* Called during the initial setup to create an admin user
*/
@RequirePOST
public HttpResponse doCreateAdminUser(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
@Restricted(NoExternalUse.class)
public HttpResponse doCreateAdminUser(StaplerRequest req, StaplerResponse rsp) throws IOException {
Jenkins j = Jenkins.getInstance();
j.checkPermission(Jenkins.ADMINISTER);

// This will be set up by default. if not, something changed, ok to fail
HudsonPrivateSecurityRealm securityRealm = (HudsonPrivateSecurityRealm)j.getSecurityRealm();
HudsonPrivateSecurityRealm securityRealm = (HudsonPrivateSecurityRealm) j.getSecurityRealm();

User admin = securityRealm.getUser(SetupWizard.initialSetupAdminUserName);
try {
if(admin != null) {
if (admin != null) {
admin.delete(); // assume the new user may well be 'admin'
}

User u = securityRealm.createAccountByAdmin(req, rsp, "/jenkins/install/SetupWizard/setupWizardFirstUser.jelly", null);
if (u != null) {
if(admin != null) {
admin = null;
}

// Success! Delete the temporary password file:
try {
getInitialAdminPasswordFile().delete();
} catch (InterruptedException e) {
throw new IOException(e);
}

InstallUtil.proceedToNextStateFrom(InstallState.CREATE_ADMIN_USER);

// ... and then login
Authentication a = new UsernamePasswordAuthenticationToken(u.getId(),req.getParameter("password1"));
a = securityRealm.getSecurityComponents().manager.authenticate(a);
SecurityContextHolder.getContext().setAuthentication(a);
CrumbIssuer crumbIssuer = Jenkins.getInstance().getCrumbIssuer();
JSONObject data = new JSONObject();
if (crumbIssuer != null) {
data.accumulate("crumbRequestField", crumbIssuer.getCrumbRequestField()).accumulate("crumb", crumbIssuer.getCrumb(req));
}
return HttpResponses.okJSON(data);
} else {
return HttpResponses.okJSON();

User newUser = securityRealm.createAccountFromSetupWizard(req);
if (admin != null) {
admin = null;
}

// Success! Delete the temporary password file:
try {
getInitialAdminPasswordFile().delete();
} catch (InterruptedException e) {
throw new IOException(e);
}

InstallUtil.proceedToNextStateFrom(InstallState.CREATE_ADMIN_USER);

// ... and then login
Authentication auth = new UsernamePasswordAuthenticationToken(newUser.getId(), req.getParameter("password1"));
auth = securityRealm.getSecurityComponents().manager.authenticate(auth);
SecurityContextHolder.getContext().setAuthentication(auth);
CrumbIssuer crumbIssuer = Jenkins.getInstance().getCrumbIssuer();
JSONObject data = new JSONObject();
if (crumbIssuer != null) {
data.accumulate("crumbRequestField", crumbIssuer.getCrumbRequestField()).accumulate("crumb", crumbIssuer.getCrumb(req));
}
return HttpResponses.okJSON(data);
} catch (AccountCreationFailedException e) {
/*
Return Unprocessable Entity from WebDAV. While this is not technically in the HTTP/1.1 standard, browsers
seem to accept this. 400 Bad Request is technically inappropriate because that implies invalid *syntax*,
not incorrect data. The client only cares about it being >200 anyways.
*/
rsp.setStatus(422);
return HttpResponses.forwardToView(securityRealm, "/jenkins/install/SetupWizard/setupWizardFirstUser.jelly");
} finally {
if(admin != null) {
if (admin != null) {
admin.save(); // recreate this initial user if something failed
}
}
Expand Down
54 changes: 23 additions & 31 deletions war/src/main/js/pluginSetupWizardGui.js
View file
Expand Up @@ -872,45 +872,37 @@ var createPluginSetupWizard = function(appendTarget) {
$c.slideDown();
}
};

var handleStaplerSubmit = function(data) {
if(data.status && data.status > 200) {
// Nothing we can really do here
setPanel(errorPanel, { errorMessage: data.statusText });
return;
}

try {
if(JSON.parse(data).status === 'ok') {
showStatePanel();
return;
}
} catch(e) {
// ignore JSON parsing issues, this may be HTML

var handleFirstUserResponseSuccess = function (data) {
if (data.status === 'ok') {
showStatePanel();
} else {
setPanel(errorPanel, {errorMessage: 'Error trying to create first user: ' + data.statusText});
}
// we get 200 OK
var responseText = data.responseText;
};

var handleFirstUserResponseError = function(res) {
// We're expecting a full HTML page to replace the form
// We can only replace the _whole_ iframe due to XSS rules
// https://stackoverflow.com/a/22913801/1117552
var responseText = res.responseText;
var $page = $(responseText);
var $errors = $page.find('.error');
if($errors.length > 0) {
var $main = $page.find('#main-panel').detach();
if($main.length > 0) {
responseText = responseText.replace(/body([^>]*)[>](.|[\r\n])+[<][/]body/,'body$1>'+$main.html()+'</body');
}
var doc = $('iframe[src]').contents()[0];
doc.open();
doc.write(responseText);
doc.close();
}
else {
showStatePanel();
var $main = $page.find('#main-panel').detach();
if($main.length > 0) {
responseText = responseText.replace(/body([^>]*)[>](.|[\r\n])+[<][/]body/,'body$1>'+$main.html()+'</body');
}
var doc = $('iframe#setup-first-user').contents()[0];
doc.open();
doc.write(responseText);
doc.close();
$('button').prop({disabled:false});
};

// call to submit the firstuser
var saveFirstUser = function() {
$('button').prop({disabled:true});
securityConfig.saveFirstUser($('iframe[src]').contents().find('form:not(.no-json)'), handleStaplerSubmit, handleStaplerSubmit);
var $form = $('iframe#setup-first-user').contents().find('form:not(.no-json)');
securityConfig.saveFirstUser($form, handleFirstUserResponseSuccess, handleFirstUserResponseError);
};

var skipFirstUser = function() {
Expand Down
2 changes: 1 addition & 1 deletion war/src/main/js/templates/firstUserPanel.hbs
View file
Expand Up @@ -3,7 +3,7 @@
</div>
<div class="modal-body">
<div class="jumbotron welcome-panel security-panel">
<iframe src="{{baseUrl}}/setupWizard/setupWizardFirstUser"></iframe>
<iframe src="{{baseUrl}}/setupWizard/setupWizardFirstUser" id="setup-first-user"></iframe>
</div>
</div>
<div class="modal-footer">
Expand Down

0 comments on commit 12031d7

Please sign in to comment.