From 1fe3770126d0cb1879fd23c3cd2d6c53e74a882a Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Sun, 17 Feb 2019 11:23:33 -0600 Subject: [PATCH 01/11] [JENKINS-56005] Add user property-change listener --- .../jenkins/security/SecurityListener.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/src/main/java/jenkins/security/SecurityListener.java b/core/src/main/java/jenkins/security/SecurityListener.java index c473a85fb436..80895e7d5a7f 100644 --- a/core/src/main/java/jenkins/security/SecurityListener.java +++ b/core/src/main/java/jenkins/security/SecurityListener.java @@ -26,6 +26,7 @@ import hudson.ExtensionList; import hudson.ExtensionPoint; +import hudson.model.UserProperty; import hudson.security.AbstractPasswordBasedSecurityRealm; import hudson.security.SecurityRealm; import java.util.ArrayList; @@ -78,6 +79,16 @@ protected void loggedIn(@Nonnull String username){} */ protected void userCreated(@Nonnull String username) {} + /** + * @since TODO + * + * Fired after an existing user object property has been updated. + * + * @param p the user object property + * @param username the username + */ + protected void userPropertyUpdated(@Nonnull UserProperty p, @Nonnull String username){} + /** * Fired when a user has failed to log in. * Would be called after {@link #failedToAuthenticate}. @@ -115,6 +126,14 @@ public static void fireUserCreated(@Nonnull String username) { } } + /** @since TODO */ + public static void fireUserPropertyUpdated(@Nonnull UserProperty p, @Nonnull String username) { + LOGGER.log(Level.FINE, "user {0} property updated", username); + for (SecurityListener l : all()) { + l.userPropertyUpdated(p, username); + } + } + /** @since 1.569 */ public static void fireFailedToAuthenticate(@Nonnull String username) { LOGGER.log(Level.FINE, "failed to authenticate: {0}", username); From f4465d1001980cba899933088ea9bed9aa6f8e9f Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Sun, 17 Feb 2019 11:27:48 -0600 Subject: [PATCH 02/11] [JENKINS-56008] Update security-realm with listener --- .../java/hudson/security/HudsonPrivateSecurityRealm.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java index c31116113dbb..03f721220857 100644 --- a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java +++ b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java @@ -717,14 +717,18 @@ public Details newInstance(StaplerRequest req, JSONObject formData) throws FormE if(!Util.fixNull(pwd).equals(Util.fixNull(pwd2))) throw new FormException("Please confirm the password by typing it twice","user.password2"); + User user = Util.getNearestAncestorOfTypeOrThrow(req, User.class); + UserProperty p = user.getProperty(Details.class); + String data = Protector.unprotect(pwd); if(data!=null) { String prefix = Stapler.getCurrentRequest().getSession().getId() + ':'; if(data.startsWith(prefix)) + SecurityListener.fireUserPropertyUpdated(p, user.getId()); return Details.fromHashedPassword(data.substring(prefix.length())); } - User user = Util.getNearestAncestorOfTypeOrThrow(req, User.class); + SecurityListener.fireUserPropertyUpdated(p, user.getId()); // the UserSeedProperty is not touched by the configure page UserSeedProperty userSeedProperty = user.getProperty(UserSeedProperty.class); if (userSeedProperty != null) { From 124b84cde2ef0092c6f929d0bde1b657b3f64c64 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Sun, 17 Feb 2019 11:46:54 -0600 Subject: [PATCH 03/11] [JENKINS-56008] Add user property-change unit test Added a new unit test which evaluates logging of a user property-change (update of user's password). --- .../HudsonPrivateSecurityRealmTest.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java index 557dbbea4e1d..290b505e19a1 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java @@ -26,6 +26,7 @@ import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; @@ -33,12 +34,15 @@ import com.gargoylesoftware.htmlunit.xml.XmlPage; import hudson.ExtensionList; import hudson.model.User; +import hudson.model.UserProperty; import hudson.remoting.Base64; import static hudson.security.HudsonPrivateSecurityRealm.CLASSIC; import static hudson.security.HudsonPrivateSecurityRealm.PASSWORD_ENCODER; import hudson.security.pages.SignupPage; import java.io.UnsupportedEncodingException; import java.net.URL; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -342,6 +346,41 @@ public void userCreationWithHashedPasswords() throws Exception { assertTrue(spySecurityListener.createdUsers.get(0).equals("charlie_hashed")); } + @Issue("JENKINS-56008") + @Test + public void updateUserPassword() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + j.jenkins.setSecurityRealm(securityRealm); + WebClient wc = j.createWebClient(); + + spySecurityListener.usersWithPasswordUpdate.clear(); + assertTrue(spySecurityListener.usersWithPasswordUpdate.isEmpty()); + + // new user account creation + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("debbie"); + signup.enterPassword("debbie"); + signup.enterFullName(StringUtils.capitalize("debbie user")); + signup.enterEmail("debbie" + "@" + "debbie" + ".com"); + HtmlPage p = signup.submit(j); + + assertEquals(200, p.getWebResponse().getStatusCode()); + + // execute an http request to change a user's password from their config page + User debbie = User.getById("debbie", false); + URL configPage = wc.createCrumbedUrl(debbie.getUrl() + "/" + "configSubmit"); + String formData = "{\"fullName\": \"debbie user\", \"description\": \"\", \"userProperty3\": {\"primaryViewName\": \"\"}, \"userProperty5\": {\"password\": \"admin\", \"$redact\": [\"password\", \"password2\"], \"password2\": \"admin\"}, \"userProperty6\": {\"authorizedKeys\": \"\"}, \"userProperty8\": {\"insensitiveSearch\": true}, \"core:apply\": \"true\"}"; + + WebRequest request = new WebRequest(configPage, HttpMethod.POST); + request.setAdditionalHeader("Content-Type", "application/x-www-form-urlencoded"); + request.setRequestBody("json=" + URLEncoder.encode(formData, StandardCharsets.UTF_8.name())); + Page page = wc.getPage(request); + + // ensure user whose password was changed was in fact logged + assertTrue(spySecurityListener.usersWithPasswordUpdate.get(0).equals("debbie")); + assertEquals(200, page.getWebResponse().getStatusCode()); + } + private void createFirstAccount(String login) throws Exception { assertNull(User.getById(login, false)); @@ -419,6 +458,7 @@ private void selfRegistration(String login) throws Exception { public static class SpySecurityListenerImpl extends SecurityListener { private List loggedInUsernames = new ArrayList<>(); private List createdUsers = new ArrayList(); + private List usersWithPasswordUpdate = new ArrayList<>(); @Override protected void loggedIn(@Nonnull String username) { @@ -427,6 +467,13 @@ protected void loggedIn(@Nonnull String username) { @Override protected void userCreated(@Nonnull String username) { createdUsers.add(username); } + + @Override + protected void userPropertyUpdated(@Nonnull UserProperty p, @Nonnull String username){ + if (p instanceof HudsonPrivateSecurityRealm.Details) { + usersWithPasswordUpdate.add(username); + } + } } @Issue("SECURITY-786") From 13c88646993e1f9504d24dbd2af1c782155137ba Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 18:54:56 -0500 Subject: [PATCH 04/11] [JENKINS-56005] Add userpropertylistener interface --- .../hudson/model/UserPropertyListener.java | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 core/src/main/java/hudson/model/UserPropertyListener.java diff --git a/core/src/main/java/hudson/model/UserPropertyListener.java b/core/src/main/java/hudson/model/UserPropertyListener.java new file mode 100644 index 000000000000..74b6380db0eb --- /dev/null +++ b/core/src/main/java/hudson/model/UserPropertyListener.java @@ -0,0 +1,111 @@ +/* + * The MIT License + * + * + * 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.model; + +import hudson.ExtensionList; +import hudson.ExtensionPoint; +import hudson.model.UserProperty; +import java.text.MessageFormat; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nonnull; + +/** + * Listener interface which all other user property-specific event-listeners make use of. + */ +public interface UserPropertyListener extends ExtensionPoint { + + static final Logger LOGGER = Logger.getLogger(UserPropertyListener.class.getName()); + + /** + * Fired when a new user property has been created. + * + * @param username the user + * @param value property that was newly created. + * + */ + default void onCreated(@Nonnull String username, @Nonnull UserProperty value) { + LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.getClass().toString(), username)); + } + + /** + * Fired when a new user property has been created. + * + * @param username the user + * @param value property that was newly created. + * + */ + default void onCreated(@Nonnull String username, @Nonnull Object value) { + LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.toString(), username)); + } + + /** + * Fired when an existing user property has been changed. + * + * @param username the user + * @param oldValue old property of the user + * @param newValue new property of the user + * + */ + default void onChanged(@Nonnull String username, @Nonnull UserProperty oldValue, @Nonnull UserProperty newValue) { + LOGGER.log(Level.FINE, MessageFormat.format("{0} property changed for user {1}", oldValue.getClass().toString(), username)); + } + + /** + * Fired when an existing user property has been changed. + * + * @param username the user + * @param oldValue old property of the user + * @param newValue new property of the user + * + */ + default void onChanged(@Nonnull String username, @Nonnull Object oldValue, @Nonnull Object newValue) { + LOGGER.log(Level.FINE, MessageFormat.format("{0} property changed for user {1}", oldValue.toString(), username)); + } + + /** + * Fired when an existing user property has been removed or deleted. + * + * @param username the user + * @param value property that was removed. + * + */ + default void onDeleted(@Nonnull String username, @Nonnull UserProperty value) { + LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.getClass().toString(), username)); + } + + /** + * Fired when an existing user property has been removed or deleted. + * + * @param username the user + * @param value property that was removed + * + */ + default void onDeleted(@Nonnull String username, @Nonnull Object value) { + LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.toString(), username)); + } + + static List all() { return ExtensionList.lookup(UserPropertyListener.class); } +} From 87b44dba770a966715a493bfd1ba5d0bc9ea83d6 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:04:29 -0500 Subject: [PATCH 05/11] [JENKINS-56008] Add passwordpropertylistener class --- .../security/PasswordPropertyListener.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 core/src/main/java/hudson/security/PasswordPropertyListener.java diff --git a/core/src/main/java/hudson/security/PasswordPropertyListener.java b/core/src/main/java/hudson/security/PasswordPropertyListener.java new file mode 100644 index 000000000000..f1afb5db27b8 --- /dev/null +++ b/core/src/main/java/hudson/security/PasswordPropertyListener.java @@ -0,0 +1,59 @@ +/* + * The MIT License + * + * + * 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 hudson.Extension; +import hudson.ExtensionList; +import hudson.model.UserPropertyListener; +import java.util.List; +import javax.annotation.Nonnull; + +import static hudson.security.HudsonPrivateSecurityRealm.PASSWORD_ENCODER; + +/** + * Listener notified of user password change events from the jenkins UI. + */ +@Extension +public class PasswordPropertyListener implements UserPropertyListener { + + /** + * @since TODO + * + * Fired when an existing user password property has been changed. + * + * @param username the user + * @param oldValue old password of the user + * @param newValue new password of the user + * + * **/ + static void fireOnChanged(@Nonnull String username, @Nonnull String oldValue, @Nonnull String newValue) { + if ((!oldValue.equals(newValue)) && (!PASSWORD_ENCODER.isPasswordValid(oldValue, newValue, null))) { + for (PasswordPropertyListener l : all()) { + l.onChanged(username, oldValue, newValue); + } + } + } + + static List all() { return ExtensionList.lookup(PasswordPropertyListener.class); } +} From 5de70acd5d71c69f2ce55c1d2624c26b75758f67 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:08:29 -0500 Subject: [PATCH 06/11] [JENKINS-56008] Refactor private-security realm Updated the HudsonPrivateSecurityRealm newInstance() method to use the PasswordPropertyListener class for notifying subscribers on changes to a user's password. --- .../java/hudson/security/HudsonPrivateSecurityRealm.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java index 3a7ebfc96b14..b44aa851afac 100644 --- a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java +++ b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java @@ -719,16 +719,19 @@ public Details newInstance(StaplerRequest req, JSONObject formData) throws FormE User user = Util.getNearestAncestorOfTypeOrThrow(req, User.class); UserProperty p = user.getProperty(Details.class); + String currentUserHashedPassword = ((Details) p).getPassword(); String data = Protector.unprotect(pwd); if(data!=null) { String prefix = Stapler.getCurrentRequest().getSession().getId() + ':'; if(data.startsWith(prefix)) - SecurityListener.fireUserPropertyUpdated(p, user.getId()); + PasswordPropertyListener.fireOnChanged(user.getId(), Util.fixNull(currentUserHashedPassword), Util.fixNull(pwd)); return Details.fromHashedPassword(data.substring(prefix.length())); } - SecurityListener.fireUserPropertyUpdated(p, user.getId()); + if (p != null) { + PasswordPropertyListener.fireOnChanged(user.getId(), Util.fixNull(currentUserHashedPassword), Util.fixNull(pwd)); + } // the UserSeedProperty is not touched by the configure page UserSeedProperty userSeedProperty = user.getProperty(UserSeedProperty.class); if (userSeedProperty != null) { From d0a19f074f4731cda71709f71b83cfde3330a6e9 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:16:16 -0500 Subject: [PATCH 07/11] [JENKINS-56170] Add apitokenpropertylistener class --- .../security/ApiTokenPropertyListener.java | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 core/src/main/java/jenkins/security/ApiTokenPropertyListener.java diff --git a/core/src/main/java/jenkins/security/ApiTokenPropertyListener.java b/core/src/main/java/jenkins/security/ApiTokenPropertyListener.java new file mode 100644 index 000000000000..d4b66c99e900 --- /dev/null +++ b/core/src/main/java/jenkins/security/ApiTokenPropertyListener.java @@ -0,0 +1,74 @@ +/* + * The MIT License + * + * + * 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 jenkins.security; + +import hudson.Extension; +import hudson.ExtensionList; +import hudson.model.UserProperty; +import hudson.model.UserPropertyListener; +import java.util.List; +import javax.annotation.Nonnull; + +/** + * Listener notified of user api-token creation and deletion events from the jenkins UI. + */ +@Extension +public class ApiTokenPropertyListener implements UserPropertyListener { + + /** + * @since TODO + * + * Fired when an api token has been created. + * + * @param username the user + * @param value api token property of the user + * + * **/ + static void fireOnCreated(@Nonnull String username, @Nonnull UserProperty value) { + if (value instanceof ApiTokenProperty) { + for (ApiTokenPropertyListener l : all()) { + l.onCreated(username, value); + } + } + } + + /** + * @since TODO + * + * Fired when an api token has been revoked. + * + * @param username the user + * @param value api token property of the user + * + * **/ + static void fireOnDeleted(@Nonnull String username, @Nonnull UserProperty value) { + if (value instanceof ApiTokenProperty) { + for (ApiTokenPropertyListener l : all()) { + l.onDeleted(username, value); + } + } + } + + static List all() { return ExtensionList.lookup(ApiTokenPropertyListener.class); } +} From d1382a6a11657e2ae922838ef9e43761c7cb4fb1 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:19:06 -0500 Subject: [PATCH 08/11] [JENKINS-56170] Update apitokenproperty methods Updated the ApiTokenProperty doGenerateToken() and doRevoke() methods to use the ApiTokenPropertyListener class for notifying subscribers of api-token creations and revocations from the Jenkins UI, respectively. --- core/src/main/java/jenkins/security/ApiTokenProperty.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/jenkins/security/ApiTokenProperty.java b/core/src/main/java/jenkins/security/ApiTokenProperty.java index 9cb01b62686e..99802b550a61 100644 --- a/core/src/main/java/jenkins/security/ApiTokenProperty.java +++ b/core/src/main/java/jenkins/security/ApiTokenProperty.java @@ -486,6 +486,9 @@ public HttpResponse doGenerateNewToken(@AncestorInPath User u, @QueryParameter S } ApiTokenStore.TokenUuidAndPlainValue tokenUuidAndPlainValue = p.tokenStore.generateNewToken(tokenName); + if ((p != null) && (tokenUuidAndPlainValue != null)) { + ApiTokenPropertyListener.fireOnCreated(u.getId(), p); + } u.save(); return HttpResponses.okJSON(new HashMap() {{ @@ -549,6 +552,7 @@ public HttpResponse doRevoke(@AncestorInPath User u, p.apiToken = null; } p.tokenStats.removeId(revoked.getUuid()); + ApiTokenPropertyListener.fireOnDeleted(u.getId(), p); } u.save(); From 97bfcc3f8ba1f8afe5592c7e85883f312ad6a7a4 Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:29:19 -0500 Subject: [PATCH 09/11] [JENKINS-56008] Update private-security realm unittest Updated HudsonPrivateSecurityRealmTest with a unittest that exercises the added PasswordPropertyListener class for notifying subscribers of user password changes. --- .../HudsonPrivateSecurityRealmTest.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java index 290b505e19a1..0a7c5280612d 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java @@ -80,10 +80,12 @@ public class HudsonPrivateSecurityRealmTest { public JenkinsRule j = new JenkinsRule(); private SpySecurityListenerImpl spySecurityListener; + private SpyPasswordPropertyListenerImpl spyPasswordListener; @Before public void linkExtension() throws Exception { spySecurityListener = ExtensionList.lookup(SecurityListener.class).get(SpySecurityListenerImpl.class); + spyPasswordListener = ExtensionList.lookup(PasswordPropertyListener.class).get(SpyPasswordPropertyListenerImpl.class); } @Before @@ -353,8 +355,8 @@ public void updateUserPassword() throws Exception { j.jenkins.setSecurityRealm(securityRealm); WebClient wc = j.createWebClient(); - spySecurityListener.usersWithPasswordUpdate.clear(); - assertTrue(spySecurityListener.usersWithPasswordUpdate.isEmpty()); + spyPasswordListener.usersWithPasswordUpdate.clear(); + assertTrue(spyPasswordListener.usersWithPasswordUpdate.isEmpty()); // new user account creation SignupPage signup = new SignupPage(wc.goTo("signup")); @@ -369,7 +371,7 @@ public void updateUserPassword() throws Exception { // execute an http request to change a user's password from their config page User debbie = User.getById("debbie", false); URL configPage = wc.createCrumbedUrl(debbie.getUrl() + "/" + "configSubmit"); - String formData = "{\"fullName\": \"debbie user\", \"description\": \"\", \"userProperty3\": {\"primaryViewName\": \"\"}, \"userProperty5\": {\"password\": \"admin\", \"$redact\": [\"password\", \"password2\"], \"password2\": \"admin\"}, \"userProperty6\": {\"authorizedKeys\": \"\"}, \"userProperty8\": {\"insensitiveSearch\": true}, \"core:apply\": \"true\"}"; + String formData = "{\"fullName\": \"debbie user\", \"description\": \"\", \"userProperty3\": {\"primaryViewName\": \"\"}, \"userProperty5\": {\"user.password\": \"admin\", \"$redact\": [\"user.password\", \"user.password2\"], \"user.password2\": \"admin\"}, \"userProperty6\": {\"authorizedKeys\": \"\"}, \"userProperty8\": {\"insensitiveSearch\": true}, \"core:apply\": \"true\"}"; WebRequest request = new WebRequest(configPage, HttpMethod.POST); request.setAdditionalHeader("Content-Type", "application/x-www-form-urlencoded"); @@ -377,8 +379,8 @@ public void updateUserPassword() throws Exception { Page page = wc.getPage(request); // ensure user whose password was changed was in fact logged - assertTrue(spySecurityListener.usersWithPasswordUpdate.get(0).equals("debbie")); assertEquals(200, page.getWebResponse().getStatusCode()); + assertTrue(spyPasswordListener.usersWithPasswordUpdate.get(0).equals("debbie")); } private void createFirstAccount(String login) throws Exception { @@ -458,7 +460,6 @@ private void selfRegistration(String login) throws Exception { public static class SpySecurityListenerImpl extends SecurityListener { private List loggedInUsernames = new ArrayList<>(); private List createdUsers = new ArrayList(); - private List usersWithPasswordUpdate = new ArrayList<>(); @Override protected void loggedIn(@Nonnull String username) { @@ -467,12 +468,15 @@ protected void loggedIn(@Nonnull String username) { @Override protected void userCreated(@Nonnull String username) { createdUsers.add(username); } + } + + @TestExtension + public static class SpyPasswordPropertyListenerImpl extends PasswordPropertyListener { + private List usersWithPasswordUpdate = new ArrayList<>(); @Override - protected void userPropertyUpdated(@Nonnull UserProperty p, @Nonnull String username){ - if (p instanceof HudsonPrivateSecurityRealm.Details) { - usersWithPasswordUpdate.add(username); - } + public void onChanged(@Nonnull String username, @Nonnull Object oldValue, @Nonnull Object newValue) { + usersWithPasswordUpdate.add(username); } } From cc75d2cbe5e0e59e0cbe8470ed1b803f7f41154d Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Mon, 11 Mar 2019 19:36:40 -0500 Subject: [PATCH 10/11] [JENKINS-56170] Update apitokenproperty unittests Added two unittests to the ApiTokenPropertyTest class for exercising the ApiTokenPropertyListener class methods; for notifying subscribers of api-token creation and revocation events. --- .../security/ApiTokenPropertyTest.java | 108 ++++++++++++++++-- 1 file changed, 100 insertions(+), 8 deletions(-) diff --git a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java index fc9e43069d77..c6a6a6cdded9 100644 --- a/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java +++ b/test/src/test/java/jenkins/security/ApiTokenPropertyTest.java @@ -4,12 +4,8 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.xml.HasXPath.hasXPath; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; +import static org.junit.Assert.assertTrue; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.HttpMethod; @@ -18,33 +14,40 @@ import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.xml.XmlPage; +import hudson.ExtensionList; import hudson.Util; import hudson.model.Cause; import hudson.model.FreeStyleProject; import hudson.model.User; +import hudson.model.UserProperty; import hudson.security.ACL; import hudson.security.ACLContext; import java.net.HttpURLConnection; import java.net.URL; +import hudson.security.HudsonPrivateSecurityRealm; +import hudson.security.pages.SignupPage; import jenkins.model.Jenkins; import jenkins.security.apitoken.ApiTokenPropertyConfiguration; import jenkins.security.apitoken.ApiTokenStore; import jenkins.security.apitoken.ApiTokenTestHelper; import net.sf.json.JSONObject; +import org.apache.commons.lang.StringUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; import java.util.function.Predicate; import java.util.stream.Collectors; import javax.annotation.Nonnull; import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.TestExtension; import org.jvnet.hudson.test.recipes.LocalData; /** @@ -52,6 +55,8 @@ */ public class ApiTokenPropertyTest { + private SpyApiTokenPropertyListenerImpl spyApiTokenListener; + @Rule public JenkinsRule j = new JenkinsRule(); @@ -59,6 +64,11 @@ public class ApiTokenPropertyTest { public void setupLegacyConfig(){ ApiTokenTestHelper.enableLegacyBehavior(); } + + @Before + public void linkExtension() throws Exception { + spyApiTokenListener = ExtensionList.lookup(ApiTokenPropertyListener.class).get(SpyApiTokenPropertyListenerImpl.class); + } /** * Tests the UI interaction and authentication. @@ -158,6 +168,69 @@ public void adminsShouldBeUnableToChangeTokensByDefault() throws Exception { Messages.ApiTokenProperty_ChangeToken_SuccessHidden(), "
" + res.getBody().asText() + "
"); } + @Issue("JENKINS-56170") + @Test + public void createUserTokenFromUi() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + j.jenkins.setSecurityRealm(securityRealm); + WebClient wc = j.createWebClient(); + + spyApiTokenListener.usersWithCreatedTokens.clear(); + assertTrue(spyApiTokenListener.usersWithCreatedTokens.isEmpty()); + + // new user account creation + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("charlie"); + signup.enterPassword("charlie"); + signup.enterFullName(StringUtils.capitalize("charlie user")); + signup.enterEmail("charlie" + "@" + "example.com"); + HtmlPage page = signup.submit(j); + + // execute an http request to create a new a user api token from their config page + User charlie = User.getById("charlie", false); + URL configPage = wc.createCrumbedUrl(charlie.getUrl() + "/" + "/descriptorByName/" + ApiTokenProperty.class.getName() + "/generateNewToken/?newTokenName=" + "charlie-token"); + Page p = wc.getPage(new WebRequest(configPage, HttpMethod.POST)); + + // ensure user whose new token was deleted was in fact logged + assertEquals(200, p.getWebResponse().getStatusCode()); + assertEquals("charlie", spyApiTokenListener.usersWithCreatedTokens.get(0)); + } + + @Issue("JENKINS-56170") + @Test + public void revokeUserTokenFromUi() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + j.jenkins.setSecurityRealm(securityRealm); + WebClient wc = j.createWebClient(); + + spyApiTokenListener.usersWithDeletedTokens.clear(); + assertTrue(spyApiTokenListener.usersWithDeletedTokens.isEmpty()); + + // new user account creation + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("alice"); + signup.enterPassword("alice"); + signup.enterFullName(StringUtils.capitalize("alice user")); + signup.enterEmail("alice" + "@" + "example.com"); + HtmlPage page = signup.submit(j); + + // execute an http request to create a new a user api token from their config page + User alice = User.getById("alice", false); + URL configPage = wc.createCrumbedUrl(alice.getUrl() + "/" + "/descriptorByName/" + ApiTokenProperty.class.getName() + "/generateNewToken/?newTokenName=" + "alice-token"); + Page p = wc.getPage(new WebRequest(configPage, HttpMethod.POST)); + JSONObject responseJson = JSONObject.fromObject(p.getWebResponse().getContentAsString()); + GenerateNewTokenResponse userToken = (GenerateNewTokenResponse) responseJson.getJSONObject("data").toBean(GenerateNewTokenResponse.class); + assertNotNull(userToken.tokenUuid); + + // execute a second http request to delete the just created user api token from their config page + configPage = wc.createCrumbedUrl(alice.getUrl() + "/" + "/descriptorByName/" + ApiTokenProperty.class.getName() + "/revoke/?tokenUuid=" + userToken.tokenUuid); + p = wc.getPage(new WebRequest(configPage, HttpMethod.POST)); + + // ensure user whose new token was deleted was in fact logged + assertEquals(200, p.getWebResponse().getStatusCode()); + assertEquals("alice", spyApiTokenListener.usersWithDeletedTokens.get(0)); + } + @Test public void postWithUsernameAndTokenInBasicAuthHeader() throws Exception { FreeStyleProject p = j.createFreeStyleProject("bar"); @@ -460,7 +533,26 @@ private GenerateNewTokenResponse generateNewToken(WebClient wc, String login, St Object result = responseJson.getJSONObject("data").toBean(GenerateNewTokenResponse.class); return (GenerateNewTokenResponse) result; } - - + + @TestExtension + public static class SpyApiTokenPropertyListenerImpl extends ApiTokenPropertyListener { + private List usersWithCreatedTokens = new ArrayList<>(); + private List usersWithDeletedTokens = new ArrayList<>(); + + @Override + public void onCreated(@Nonnull String username, @Nonnull UserProperty value) { + if (value instanceof ApiTokenProperty) { + usersWithCreatedTokens.add(username); + } + } + + @Override + public void onDeleted(@Nonnull String username, @Nonnull UserProperty value) { + if (value instanceof ApiTokenProperty) { + usersWithDeletedTokens.add(username); + } + } + } + // test no token are generated for new user with the global configuration set to false } From 4e1c33f6adf880e5e38add7ee82746e29de4a60f Mon Sep 17 00:00:00 2001 From: David Olorundare Date: Wed, 20 Mar 2019 11:42:08 -0500 Subject: [PATCH 11/11] Refactor userpropertylistener methods --- core/src/main/java/hudson/model/UserPropertyListener.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/model/UserPropertyListener.java b/core/src/main/java/hudson/model/UserPropertyListener.java index 74b6380db0eb..42f5d7cfb22f 100644 --- a/core/src/main/java/hudson/model/UserPropertyListener.java +++ b/core/src/main/java/hudson/model/UserPropertyListener.java @@ -57,7 +57,7 @@ default void onCreated(@Nonnull String username, @Nonnull UserProperty value) { * @param value property that was newly created. * */ - default void onCreated(@Nonnull String username, @Nonnull Object value) { + default void onCreated(@Nonnull String username, @Nonnull T value) { LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.toString(), username)); } @@ -81,7 +81,7 @@ default void onChanged(@Nonnull String username, @Nonnull UserProperty oldValue, * @param newValue new property of the user * */ - default void onChanged(@Nonnull String username, @Nonnull Object oldValue, @Nonnull Object newValue) { + default void onChanged(@Nonnull String username, @Nonnull T oldValue, @Nonnull T newValue) { LOGGER.log(Level.FINE, MessageFormat.format("{0} property changed for user {1}", oldValue.toString(), username)); } @@ -103,7 +103,7 @@ default void onDeleted(@Nonnull String username, @Nonnull UserProperty value) { * @param value property that was removed * */ - default void onDeleted(@Nonnull String username, @Nonnull Object value) { + default void onDeleted(@Nonnull String username, @Nonnull T value) { LOGGER.log(Level.FINE, MessageFormat.format("new {0} property created for user {1}", value.toString(), username)); }