Skip to content

Commit

Permalink
Use user display names in mails and enhance anonymous user detection (#…
Browse files Browse the repository at this point in the history 
…198)

* Use standard check for isAnonymous

* Switch to User class for claim actions

* Remove obsolete methods

* Fis spotbugs issue
  • Loading branch information
@Greybird
Greybird authored Jan 11, 2023
1 parent 780da6f commit ad9a836
Show file tree
Hide file tree
Showing 11 changed files with 195 additions and 118 deletions.
109 changes: 50 additions & 59 deletions src/main/java/hudson/plugins/claim/AbstractClaimBuildAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import groovy.lang.Binding;
import hudson.model.*;
import hudson.security.ACL;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript;
Expand All @@ -16,7 +17,9 @@

import jakarta.mail.MessagingException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.userdetails.UsernameNotFoundException;

import javax.annotation.Nonnull;
import javax.servlet.ServletException;
import java.io.IOException;
import java.util.Collections;
Expand All @@ -38,8 +41,6 @@ public abstract class AbstractClaimBuildAction<T extends Saveable>
private String assignedBy;
private Date claimDate;
private boolean transientClaim = !ClaimConfig.get().isStickyByDefault();
@Deprecated
private transient boolean reclaim;
private ClaimBuildFailureAnalyzer bfaClaimer = null;
private String reason;

Expand All @@ -53,17 +54,6 @@ public final CommonMessagesProvider getMessageProvider() {
return CommonMessagesProvider.build(this);
}

/**
* Indicates if the {@link Saveable} is claimed.
*
* @deprecated use {@link #isClaimed()} instead
* @return true if the {@link Saveable} is claimed, else false
*/
@Deprecated
public final boolean isReclaim() {
return isClaimed();
}

public final ClaimBuildFailureAnalyzer getBfaClaimer() {
return bfaClaimer;
}
Expand All @@ -83,18 +73,18 @@ public final String getUrlName() {
public final void doClaim(StaplerRequest req, StaplerResponse resp)
throws Exception {
User currentUser = getCurrentUser();
String currentUserId = currentUser.getId();
String claimedUser = currentUserId; // Default to self-assignment
User claimedUser = currentUser; // Default to self-assignment
String assignee = req.getSubmittedForm().getString("assignee");
if (!StringUtils.isEmpty(assignee) && !claimedUser.equals(assignee)) {

if (!StringUtils.isEmpty(assignee) && !claimedUser.getId().equals(assignee)) {
// Validate the specified assignee.
User resolvedAssignee = User.get(assignee, false, Collections.emptyMap());
User resolvedAssignee = getUserFromId(assignee, false);
if (resolvedAssignee == null) {
LOGGER.log(Level.WARNING, "Invalid username specified for assignment: {0}", assignee);
resp.forwardToPreviousPage(req);
return;
}
claimedUser = assignee;
claimedUser = resolvedAssignee;
}
String reasonProvided = req.getSubmittedForm().getString("reason");

Expand Down Expand Up @@ -122,7 +112,7 @@ public final void doClaim(StaplerRequest req, StaplerResponse resp)
if (StringUtils.isEmpty(reasonProvided)) {
reasonProvided = null;
}
claim(claimedUser, reasonProvided, currentUserId, new Date(), sticky, propagated, true);
claim(claimedUser, reasonProvided, currentUser, new Date(), sticky, propagated, true);
this.getOwner().save();

evalGroovyScript();
Expand All @@ -131,34 +121,20 @@ public final void doClaim(StaplerRequest req, StaplerResponse resp)

private static User getCurrentUser() {
Authentication authentication = Jenkins.getAuthentication2();
return User.getById(authentication.getName(), false);
}

/**
* Claims a {@link Saveable}.
* @param claimedByUser name of the claiming user
* @param providedReason reason for the claim
* @param assignedByUser name of the assigned user
* @param isSticky true if the claim has to be kept until resolution
* @deprecated use {@link #claim(String, String, String, Date, boolean, boolean, boolean)}
*/
@Deprecated
public final void claim(String claimedByUser, String providedReason, String assignedByUser, boolean isSticky) {
claim(claimedByUser, providedReason, assignedByUser, new Date(), isSticky,
ClaimConfig.get().isPropagateToFollowingBuildsByDefault(), false);
return User.get2(authentication);
}

/**
* Claims a {@link Saveable}, and optionally notifies of the claim.
* @param claimedByUser name of the claiming user
* @param claimedByUser claiming user
* @param providedReason reason for the claim
* @param assignedByUser name of the assigner user
* @param assignedByUser assigner user
* @param date date of the claim
* @param isSticky true if the claim has to be kept until resolution
* @param isPropagated true if the claim has to be propagated to following builds
* @param notify true if notifications have to be sent
*/
public final void claim(String claimedByUser, String providedReason, String assignedByUser, Date date,
public final void claim(User claimedByUser, String providedReason, User assignedByUser, Date date,
boolean isSticky, boolean isPropagated, boolean notify) {
applyClaim(claimedByUser, providedReason, assignedByUser, date, isSticky, isPropagated);
if (notify) {
Expand All @@ -174,33 +150,33 @@ public final void claim(String claimedByUser, String providedReason, String assi

/**
* Sends an initial claim email.
* @param claimedByUser name of the claiming user
* @param claimedByUser the claiming user
* @param providedReason reason for the claim
* @param assignedByUser name of the assigner user
* @param assignedByUser the assigner user
* @throws MessagingException if there has been some problem with sending the email
* @throws IOException if there is an IO problem when sending the mail
* @throws InterruptedException if the send operation is interrupted
*/
protected abstract void sendInitialClaimEmail(String claimedByUser, String providedReason, String assignedByUser)
protected abstract void sendInitialClaimEmail(User claimedByUser, String providedReason, User assignedByUser)
throws MessagingException, IOException, InterruptedException;

/**
* Applies the claim data to the {@link AbstractClaimBuildAction}.
* @param claimedByUser name of the claiming user
* @param claimedByUser the claiming user
* @param providedReason reason for the claim
* @param assignedByUser name of the assigner user
* @param assignedByUser the assigner user
* @param date date of the claim
* @param isSticky true if the claim has to be kept until resolution
* @param isPropagated true if the claim has to be propagated to following builds
*/
protected void applyClaim(String claimedByUser, String providedReason, String assignedByUser, Date date,
protected void applyClaim(@Nonnull User claimedByUser, String providedReason, @Nonnull User assignedByUser, Date date,
boolean isSticky, boolean isPropagated) {
this.claimed = true;
this.claimedBy = claimedByUser;
this.claimedBy = claimedByUser.getId();
this.reason = providedReason;
this.transientClaim = !isSticky;
this.claimDate = date;
this.assignedBy = assignedByUser;
this.assignedBy = assignedByUser.getId();
if (isPropagated) {
getNextAction().ifPresent(action -> {
if (!action.isClaimed()) {
Expand Down Expand Up @@ -230,15 +206,6 @@ public final void doUnclaim(StaplerRequest req, StaplerResponse resp)
resp.forwardToPreviousPage(req);
}

/**
* Unclaims a {@link Saveable}.
* @deprecated use {@link #unclaim(boolean)}
*/
@Deprecated
public final void unclaim() {
unclaim(false);
}

/**
* Unclaims a {@link Saveable}, and optionally notifies of the unclaim.
* @param notify true if notifications have to be sent
Expand Down Expand Up @@ -272,7 +239,7 @@ public final String getAssignedBy() {

// used by groovy scripts ?
public final String getClaimedByName() {
User user = User.get(claimedBy, false, Collections.emptyMap());
User user = getUserFromId(claimedBy, false);
if (user != null) {
return user.getDisplayName();
} else {
Expand All @@ -282,7 +249,7 @@ public final String getClaimedByName() {

// used by groovy scripts ?
public final String getAssignedByName() {
User user = User.get(assignedBy, false, Collections.emptyMap());
User user = getUserFromId(assignedBy, false);
if (user != null) {
return user.getDisplayName();
} else {
Expand All @@ -308,9 +275,21 @@ public final boolean isClaimed() {
/**
* Claim a new {@link Saveable} with the same settings as this one.
* @param other the source data
* @return true if claim has been copied, false otherwise
*/
protected void copyTo(AbstractClaimBuildAction<T> other) {
other.applyClaim(getClaimedBy(), getReason(), getAssignedBy(), getClaimDate(), isSticky(), false);
protected boolean copyTo(AbstractClaimBuildAction<T> other) {
User claimedBy = getUserFromId(getClaimedBy(), false);
if (claimedBy == null)
{
return false;
}
User assignedBy = getUserFromId(getAssignedBy(), false);
if (assignedBy == null)
{
assignedBy = User.getUnknown();
}
other.applyClaim(claimedBy, getReason(), assignedBy, getClaimDate(), isSticky(), false);
return true;
}

public final boolean isClaimedByMe() {
Expand All @@ -337,7 +316,7 @@ public final boolean canRelease() {
}

public final boolean isUserAnonymous() {
return Jenkins.getAuthentication2().getName().equals("anonymous");
return ACL.isAnonymous2(Jenkins.getAuthentication2());
}

@Exported
Expand Down Expand Up @@ -453,4 +432,16 @@ protected final void evalGroovyScript() {
}
}
}

protected final User getUserFromId(String userId) {
return getUserFromId(userId, true);
}

protected final User getUserFromId(String userId, boolean throwIfNotFound) {
User resolved = User.get(userId, false, Collections.emptyMap());
if (resolved == null && throwIfNotFound) {
throw new UsernameNotFoundException("Unknown user: " + userId);
}
return resolved;
}
}
3 changes: 2 additions & 1 deletion src/main/java/hudson/plugins/claim/ClaimBuildAction.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package hudson.plugins.claim;

import hudson.model.Run;
import hudson.model.User;
import jenkins.model.RunAction2;

import jakarta.mail.MessagingException;
Expand Down Expand Up @@ -53,7 +54,7 @@ protected Optional<AbstractClaimBuildAction> getNextAction() {
}

@Override
protected void sendInitialClaimEmail(String claimedByUser, String providedReason, String assignedByUser)
protected void sendInitialClaimEmail(User claimedByUser, String providedReason, User assignedByUser)
throws MessagingException, IOException {
ClaimEmailer.sendInitialBuildClaimEmailIfConfigured(
claimedByUser,
Expand Down
29 changes: 16 additions & 13 deletions src/main/java/hudson/plugins/claim/ClaimEmailer.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package hudson.plugins.claim;

import hudson.model.User;
import hudson.plugins.claim.messages.InitialBuildClaimMessage;
import hudson.plugins.claim.messages.InitialTestClaimMessage;
import hudson.plugins.claim.messages.RepeatedBuildClaimMessage;
Expand All @@ -8,6 +9,8 @@
import hudson.tasks.junit.CaseResult;

import jakarta.mail.MessagingException;

import javax.annotation.Nonnull;
import java.io.IOException;
import java.util.List;
import java.util.logging.Logger;
Expand Down Expand Up @@ -40,64 +43,64 @@ private ClaimEmailer() {

/**
* Sends an email to the assignee indicating that the given build has been assigned.
* @param claimedByUser name of the claiming user
* @param assignedByUser name of the assigner user
* @param claimedByUser the claiming user
* @param assignedByUser the assigner user
* @param action the build/action which has been assigned
* @param reason the reason given for the assignment
* @param url the URL the user can view for the assigned build
* @throws MessagingException if there has been some problem with sending the email
* @throws IOException if there is an IO problem when sending the mail
*/
public static void sendInitialBuildClaimEmailIfConfigured(String claimedByUser, String assignedByUser,
public static void sendInitialBuildClaimEmailIfConfigured(@Nonnull User claimedByUser, @Nonnull User assignedByUser,
String action, String reason, String url)
throws MessagingException, IOException {

ClaimConfig config = ClaimConfig.get();
if (config.getSendEmails() && MAILER_LOADED) {
InitialBuildClaimMessage message = new InitialBuildClaimMessage(
action, url, reason, claimedByUser, assignedByUser
action, url, reason, claimedByUser.getDisplayName(), assignedByUser.getDisplayName()
);
message.send();
}
}

/**
* Sends an email to the assignee indicating that the given test has been assigned.
* @param claimedByUser name of the claiming user
* @param assignedByUser name of the assigner user
* @param claimedByUser the claiming user
* @param assignedByUser the assigner user
* @param action the build/action which has been assigned
* @param reason the reason given for the assignment
* @param url the URL the user can view for the assigned build
* @throws MessagingException if there has been some problem with sending the email
* @throws IOException if there is an IO problem when sending the mail
*/
public static void sendInitialTestClaimEmailIfConfigured(String claimedByUser, String assignedByUser,
public static void sendInitialTestClaimEmailIfConfigured(@Nonnull User claimedByUser, @Nonnull User assignedByUser,
String action, String reason, String url)
throws MessagingException, IOException {

ClaimConfig config = ClaimConfig.get();
if (config.getSendEmails() && MAILER_LOADED) {
InitialTestClaimMessage message = new InitialTestClaimMessage(
action, url, reason, claimedByUser, assignedByUser
action, url, reason, claimedByUser.getDisplayName(), assignedByUser.getDisplayName()
);
message.send();
}
}

/**
* Sends an email to the assignee indicating that the given build is still failing.
* @param claimedByUser name of the claiming user
* @param claimedByUser the claiming user
* @param action the build/action which has been assigned
* @param url the URL the user can view for the assigned build
* @throws MessagingException if there has been some problem with sending the email
* @throws IOException if there is an IO problem when sending the mail
*/
public static void sendRepeatedBuildClaimEmailIfConfigured(String claimedByUser, String action, String url)
public static void sendRepeatedBuildClaimEmailIfConfigured(@Nonnull User claimedByUser, String action, String url)
throws MessagingException, IOException {

ClaimConfig config = ClaimConfig.get();
if (config.getSendEmailsForStickyFailures() && MAILER_LOADED) {
RepeatedBuildClaimMessage message = new RepeatedBuildClaimMessage(action, url, claimedByUser);
RepeatedBuildClaimMessage message = new RepeatedBuildClaimMessage(action, url, claimedByUser.getDisplayName());
message.send();
}
}
Expand All @@ -111,13 +114,13 @@ public static void sendRepeatedBuildClaimEmailIfConfigured(String claimedByUser,
* @throws MessagingException if there has been some problem with sending the email
* @throws IOException if there is an IO problem when sending the mail
*/
public static void sendRepeatedTestClaimEmailIfConfigured(String claimedByUser, String action, String url,
public static void sendRepeatedTestClaimEmailIfConfigured(@Nonnull User claimedByUser, String action, String url,
List<CaseResult> failedTests)
throws MessagingException, IOException {

ClaimConfig config = ClaimConfig.get();
if (config.getSendEmailsForStickyFailures() && MAILER_LOADED) {
RepeatedTestClaimMessage message = new RepeatedTestClaimMessage(action, url, claimedByUser, failedTests);
RepeatedTestClaimMessage message = new RepeatedTestClaimMessage(action, url, claimedByUser.getDisplayName(), failedTests);
message.send();
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/hudson/plugins/claim/ClaimPublisher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.User;
import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.BuildStepMonitor;
import hudson.tasks.Notifier;
Expand Down Expand Up @@ -73,14 +74,16 @@ static void addClaimBuildAction(Run<?, ?> build) throws IOException {
if (previousBuild != null) {
ClaimBuildAction c = previousBuild.getAction(ClaimBuildAction.class);
if (c != null && c.isClaimed() && c.isSticky()) {
c.copyTo(action);
sendEmailsForStickyFailure(build, c.getClaimedBy());
if (c.copyTo(action)) {
sendEmailsForStickyFailure(build, c.getUserFromId(c.getClaimedBy()));
}
}
}
}

private static void sendEmailsForStickyFailure(Run<?, ?> build, String claimedByUser) {
private static void sendEmailsForStickyFailure(Run<?, ?> build, User claimedByUser) {
try {

ClaimEmailer.sendRepeatedBuildClaimEmailIfConfigured(claimedByUser, build.toString(), build.getUrl());
} catch (MessagingException | IOException e) {
LOGGER.log(Level.WARNING, "Exception when sending build failure reminder email. Ignoring.", e);
Expand Down
Loading

0 comments on commit ad9a836

Please sign in to comment.