-
Notifications
You must be signed in to change notification settings - Fork 495
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
add new feedback api #11162
Open
stevenwinship
wants to merge
11
commits into
develop
Choose a base branch
from
11129-send-feedback-to-contacts
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+525
−7
Open
add new feedback api #11162
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
e92fab6
add new feedback api
stevenwinship 1d32d9e
unit test reset rate limit cache
stevenwinship 98ab27b
unit test reset rate limit cache
stevenwinship 5347a8d
unit test reset rate limit cache
stevenwinship 8cbd892
adding error strings from bundle
stevenwinship 4748ffc
doc update
stevenwinship 69983a4
more validation and lookup by id, alias, or persistentId
stevenwinship c4b721a
add json 'identifier' to the documentation
stevenwinship f6a1ba9
review comments addressed
stevenwinship 9f8a9e1
fix for support feedback check
stevenwinship 67c2fe6
Merge branch 'develop' into 11129-send-feedback-to-contacts
stevenwinship File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
This feature adds a new API to send feedback to the Collection, Dataset, or DataFile's contacts. | ||
Similar to the "admin/feedback" API the "sendfeedback" API sends an email to all the contacts listed for the Dataset. The main differences for this feature are: | ||
1. This API is not limited to Admins | ||
2. This API does not return the email addresses in the "toEmail" and "ccEmail" elements for privacy reasons | ||
3. This API can be rate limited to avoid spamming | ||
4. The body size limit can be configured | ||
5. The body will be stripped of any html code to prevent malicious scripts or links | ||
6. The fromEmail will be validated for correct format | ||
|
||
To set the Rate Limiting for guest users (See Rate Limiting Configuration for more details. This example allows 1 send per hour for any guest) | ||
``curl http://localhost:8080/api/admin/settings/:RateLimitingCapacityByTierAndAction -X PUT -d '[{\"tier\": 0, \"limitPerHour\": 1, \"actions\": [\"CheckRateLimitForDatasetFeedbackCommand\"]}]'`` | ||
|
||
To set the message size limit (example limit of 1080 chars): | ||
``curl -X PUT -d 1080 http://localhost:8080/api/admin/settings/:ContactFeedbackMessageSizeLimit`` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
src/main/java/edu/harvard/iq/dataverse/api/SendFeedbackAPI.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
package edu.harvard.iq.dataverse.api; | ||
|
||
import edu.harvard.iq.dataverse.*; | ||
import edu.harvard.iq.dataverse.api.auth.AuthRequired; | ||
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; | ||
import edu.harvard.iq.dataverse.authorization.users.User; | ||
import edu.harvard.iq.dataverse.branding.BrandingUtil; | ||
import edu.harvard.iq.dataverse.engine.command.impl.CheckRateLimitForDatasetFeedbackCommand; | ||
import edu.harvard.iq.dataverse.feedback.Feedback; | ||
import edu.harvard.iq.dataverse.feedback.FeedbackUtil; | ||
import edu.harvard.iq.dataverse.util.BundleUtil; | ||
import edu.harvard.iq.dataverse.util.cache.CacheFactoryBean; | ||
import edu.harvard.iq.dataverse.util.json.JsonUtil; | ||
import edu.harvard.iq.dataverse.validation.EMailValidator; | ||
import jakarta.ejb.EJB; | ||
import jakarta.json.*; | ||
import jakarta.mail.internet.InternetAddress; | ||
import jakarta.ws.rs.Consumes; | ||
import jakarta.ws.rs.POST; | ||
import jakarta.ws.rs.Path; | ||
import jakarta.ws.rs.container.ContainerRequestContext; | ||
import jakarta.ws.rs.core.Context; | ||
import jakarta.ws.rs.core.Response; | ||
|
||
import java.text.MessageFormat; | ||
import java.util.logging.Logger; | ||
|
||
@Path("sendfeedback") | ||
public class SendFeedbackAPI extends AbstractApiBean { | ||
private static final Logger logger = Logger.getLogger(SendFeedbackAPI.class.getCanonicalName()); | ||
@EJB | ||
MailServiceBean mailService; | ||
@EJB | ||
CacheFactoryBean cacheFactory; | ||
/** | ||
* This method mimics the contact form and sends an email to the contacts of the | ||
* specified Collection/Dataset/DataFile, optionally ccing the support email | ||
* address, or to the support email address when there is no target object. | ||
**/ | ||
@POST | ||
@AuthRequired | ||
public Response submitFeedback(@Context ContainerRequestContext crc, String jsonString) { | ||
try { | ||
JsonObject jsonObject = JsonUtil.getJsonObject(jsonString); | ||
if (!jsonObject.containsKey("subject") || !jsonObject.containsKey("body")) { | ||
return badRequest(BundleUtil.getStringFromBundle("sendfeedback.body.error.missingRequiredFields")); | ||
} | ||
|
||
JsonNumber jsonNumber = jsonObject.containsKey("targetId") ? jsonObject.getJsonNumber("targetId") : null; | ||
// idtf will hold the "targetId" or the "identifier". If neither is set then this is a general feedback to support | ||
String idtf = jsonNumber != null ? jsonNumber.toString() : jsonObject.containsKey("identifier") ? jsonObject.getString("identifier") : null; | ||
DvObject feedbackTarget = null; | ||
|
||
if (jsonNumber != null) { | ||
feedbackTarget = dvObjSvc.findDvObject(jsonNumber.longValue()); | ||
} else if (idtf != null) { | ||
if (feedbackTarget == null) { | ||
feedbackTarget = dataverseSvc.findByAlias(idtf); | ||
} | ||
if (feedbackTarget == null) { | ||
feedbackTarget = dvObjSvc.findByGlobalId(idtf, DvObject.DType.Dataset); | ||
} | ||
if (feedbackTarget == null) { | ||
feedbackTarget = dvObjSvc.findByGlobalId(idtf, DvObject.DType.DataFile); | ||
} | ||
} | ||
|
||
// feedbackTarget and idtf are both null this is a support feedback and is ok | ||
if (feedbackTarget == null && idtf != null) { | ||
return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("sendfeedback.request.error.targetNotFound")); | ||
} | ||
// Check for rate limit exceeded. | ||
if (!cacheFactory.checkRate(getRequestUser(crc), new CheckRateLimitForDatasetFeedbackCommand(null, feedbackTarget))) { | ||
return error(Response.Status.TOO_MANY_REQUESTS, BundleUtil.getStringFromBundle("sendfeedback.request.rateLimited")); | ||
} | ||
|
||
DataverseSession dataverseSession = null; | ||
String userMessage = sanitizeBody(jsonObject.getString("body")); | ||
InternetAddress systemAddress = mailService.getSupportAddress().orElse(null); | ||
String userEmail = getEmail(jsonObject, crc); | ||
String messageSubject = jsonObject.getString("subject"); | ||
String baseUrl = systemConfig.getDataverseSiteUrl(); | ||
String installationBrandName = BrandingUtil.getInstallationBrandName(); | ||
String supportTeamName = BrandingUtil.getSupportTeamName(systemAddress); | ||
JsonArrayBuilder jab = Json.createArrayBuilder(); | ||
Feedback feedback = FeedbackUtil.gatherFeedback(feedbackTarget, dataverseSession, messageSubject, userMessage, systemAddress, userEmail, baseUrl, installationBrandName, supportTeamName, SendFeedbackDialog.ccSupport(feedbackTarget)); | ||
jab.add(feedback.toLimitedJsonObjectBuilder()); | ||
mailService.sendMail(feedback.getFromEmail(), feedback.getToEmail(), feedback.getCcEmail(), feedback.getSubject(), feedback.getBody()); | ||
return ok(jab); | ||
} catch (WrappedResponse resp) { | ||
return resp.getResponse(); | ||
} catch (JsonException je) { | ||
return error(Response.Status.BAD_REQUEST, "Invalid JSON; error message: " + je.getMessage()); | ||
} | ||
} | ||
|
||
private String getEmail(JsonObject jsonObject, ContainerRequestContext crc) throws WrappedResponse { | ||
String fromEmail = jsonObject.containsKey("fromEmail") ? jsonObject.getString("fromEmail") : ""; | ||
if (fromEmail.isBlank() && crc != null) { | ||
User user = getRequestUser(crc); | ||
if (user instanceof AuthenticatedUser) { | ||
fromEmail = ((AuthenticatedUser) user).getEmail(); | ||
} | ||
} | ||
if (fromEmail == null || fromEmail.isBlank()) { | ||
throw new WrappedResponse(badRequest(BundleUtil.getStringFromBundle("sendfeedback.fromEmail.error.missing"))); | ||
} | ||
if (!EMailValidator.isEmailValid(fromEmail)) { | ||
throw new WrappedResponse(badRequest(MessageFormat.format(BundleUtil.getStringFromBundle("sendfeedback.fromEmail.error.invalid"), fromEmail))); | ||
} | ||
return fromEmail; | ||
} | ||
private String sanitizeBody (String body) throws WrappedResponse { | ||
// remove malicious html | ||
String sanitizedBody = body == null ? "" : body.replaceAll("\\<.*?>", ""); | ||
|
||
long limit = systemConfig.getContactFeedbackMessageSizeLimit(); | ||
if (limit > 0 && sanitizedBody.length() > limit) { | ||
throw new WrappedResponse(badRequest(MessageFormat.format(BundleUtil.getStringFromBundle("sendfeedback.body.error.exceedsLength"), sanitizedBody.length(), limit))); | ||
} else if (sanitizedBody.length() == 0) { | ||
throw new WrappedResponse(badRequest(BundleUtil.getStringFromBundle("sendfeedback.body.error.isEmpty"))); | ||
} | ||
|
||
return sanitizedBody; | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
...edu/harvard/iq/dataverse/engine/command/impl/CheckRateLimitForDatasetFeedbackCommand.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package edu.harvard.iq.dataverse.engine.command.impl; | ||
|
||
import edu.harvard.iq.dataverse.DvObject; | ||
import edu.harvard.iq.dataverse.engine.command.AbstractVoidCommand; | ||
import edu.harvard.iq.dataverse.engine.command.CommandContext; | ||
import edu.harvard.iq.dataverse.engine.command.DataverseRequest; | ||
import edu.harvard.iq.dataverse.engine.command.exception.CommandException; | ||
|
||
public class CheckRateLimitForDatasetFeedbackCommand extends AbstractVoidCommand { | ||
|
||
public CheckRateLimitForDatasetFeedbackCommand(DataverseRequest aRequest, DvObject dvObject) { | ||
super(aRequest, dvObject); | ||
} | ||
|
||
@Override | ||
protected void executeImpl(CommandContext ctxt) throws CommandException { } | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think we need to say && (idtf != null || jsonNumber != null) because now if the jsonNumber doesn't return a dvObject it will go to general support, which we probably don't want.
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.
Good catch. the idtf used to have a string version of the jsonNumber but now it doesn't
// idtf will hold the "targetId" or the "identifier". If neither is set then this is a general feedback to support
String idtf = jsonNumber != null ? jsonNumber.toString() : jsonObject.containsKey("identifier") ? jsonObject.getString("identifier") : null;