-
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
base: develop
Are you sure you want to change the base?
add new feedback api #11162
Changes from 6 commits
e92fab6
1d32d9e
98ab27b
5347a8d
8cbd892
4748ffc
69983a4
c4b721a
f6a1ba9
9f8a9e1
67c2fe6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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`` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
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.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. | ||
* | ||
* !!!!! This should not be moved outside the /admin path unless/until some form | ||
* of captcha or other spam-prevention mechanism is added. As is, it allows an | ||
* unauthenticated user (with access to the /admin api path) to send email from | ||
* anyone to any contacts in Dataverse. It also does not do much to validate | ||
* user input (e.g. to strip potentially malicious html, etc.)!!!! | ||
**/ | ||
@POST | ||
@AuthRequired | ||
@Consumes("application/json") | ||
public Response submitFeedback(@Context ContainerRequestContext crc, JsonObject jsonObject) { | ||
try { | ||
JsonNumber jsonNumber = jsonObject.getJsonNumber("targetId"); | ||
DvObject feedbackTarget = null; | ||
if (jsonNumber != null) { | ||
feedbackTarget = dvObjSvc.findDvObject(jsonNumber.longValue()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably out of scope, but how hard would it be to support finding Dataverses by alias and datasets/files by persistent id? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not hard but it would require a documentation change that makes the api under /admin different from this new one. Would it be more confusing to have the json files differ between the 2 APIs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll make the change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
if(feedbackTarget==null) { | ||
return error(Response.Status.BAD_REQUEST, "Feedback target object not found"); | ||
} | ||
} | ||
// Check for rate limit exceeded. | ||
if (!cacheFactory.checkRate(getRequestUser(crc), new CheckRateLimitForDatasetFeedbackCommand(null, feedbackTarget))) { | ||
return error(Response.Status.TOO_MANY_REQUESTS, "Too many requests to send feedback"); | ||
} | ||
|
||
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(); | ||
} | ||
} | ||
|
||
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; | ||
} | ||
} |
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 { } | ||
} | ||
|
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 it might be helpful to get the json as a string and then parse it within the try so that you can give the user better feedback. I tried it with invalid json and I get a 500 error and an empty response.
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 idea
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 made the change to pass in a string and validate it