Skip to content
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

Impl. send-email endpoint #473

Closed
wants to merge 3 commits into from
Closed

Impl. send-email endpoint #473

wants to merge 3 commits into from

Conversation

pvannierop
Copy link
Contributor

@pvannierop pvannierop commented Jun 20, 2024

We currently are building functionality to allow sending emails from aRMT app on behalf of the study subject. The use case is that study subject are allowed to contact their assigned physician from within the app so that any problems or questions can be raised in a convenient manner.

This PR will implement a new POST endpoint /email/projects/<projectId>/users/<subjectId> endpoint. This endpoint receives a post body with structure:

{
  "to": "me@there.org",
  "cc": "you@there.org;andyou@there.org",
  "bcc": "everyone@there.org",
  "subject": "Hi!",
  "message": "How <i>you</i> doin...."
}

In response the Appserver will send the email.

The way how emails are sent is polymorphic. At present, the only method is by using the Firebase Trigger Email from Firestore extension. This method involves placing a JSON document in the Firebase database collection. The extension is configured to send emails from this database collection.

In the future, we can implement a way to directly send emails from Appserver. This would require to register SMTP server credentials in the service.

Remarks

  • At the moment I did not implement some for of rate limiting for sending email. Lets discuss whether we think this is needed.
  • In the current implementation, the aRMT app is responsible for defining all aspects of the email including the to, cc and bcc fields. In theory this would allow a subject to spam the whole world with emails. Lets discuss whether we need to change this.

@pvannierop pvannierop requested a review from mpgxvii June 20, 2024 07:48
@pvannierop pvannierop self-assigned this Jun 20, 2024
@pvannierop pvannierop changed the base branch from master to dev June 20, 2024 07:49
Currently only implemented for firebase mail extension type.
Copy link
Member

@yatharthranjan yatharthranjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @pvannierop, thanks for this.

Some concerns after discussing here at KCL -

  1. Security concerns - As tokens in the frontend are considered open, any user with a token can send any email with any detail to anyone using this endpoint.
  2. The firestore security - How are you securing the Firestore endpoints, would anyone in that firebase project be able to access the firestore email collection ?
  3. The permissions on the controller endpoint seem incorrect. The subjectId is not being used for any business logic. I think the permissions should reflect this and be on the project?
  4. Is there a particular preference for using the Firestore triggers for emails? Can we use a similar smtp setup as in the management portal. You can also have an docker container acting as smtp relay alongside the appserver so you don't have to implement anything in the code.

For 1, I would suggest a more locked-down approach. You can have a mapping of Project to Physician/Clinician email-ids in the backend, then when a user presses contact physician in the app, they only provide the message+subject. The appserver receives this message and based on project of the user (after auth), gets the physician email and sends the email.

In other apps with similar use cases, I have noticed the app redirects me to the email client on my phone and populates the subject to address and the message. The user can then choose to send it from their existing email client using their own email address (I assume since these are clinicians, they would have access to subject PII anyways, so this would not be a privacy concern).

Let us discuss this further in the developer call today.

@pvannierop
Copy link
Contributor Author

Closing this. Superseeded by #474

@pvannierop pvannierop closed this Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants