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

reply issue by email #13585

Closed
wants to merge 39 commits into from
Closed

reply issue by email #13585

wants to merge 39 commits into from

Conversation

a1012112796
Copy link
Member

@a1012112796 a1012112796 commented Nov 16, 2020

TODOs:

  • test code is not finished

view now:
image

fix #9067

Signed-off-by: a1012112796 <1012112796@qq.com>
@a1012112796 a1012112796 changed the title [WIP] reply issue by email [WIP] [need-help] reply issue by email Nov 16, 2020
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Nov 16, 2020
services/imap/imap.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 16, 2020
@mrsdizzie
Copy link
Member

mrsdizzie commented Nov 17, 2020

This is not secure and won't work as is since it will let you send an email and post a comment as any user you want since it only checks the from address which can be anything the user wants -- there needs to be unique keys per user that GItea already knows or can calculate in order to verify a message.

Github does this that each issue/pull has a unique ID for each user and that is included in the reply-to address in order to verify a comment (which require sub addressing) like Reply-To: go-gitea/gitea <reply+AAMXTQ7FKLLONE4ER6KUPGN5X5TCJEVBNHHXHSI8HW@reply.github.com>.

Gitlab has a second option that if you can't create random/unique reply to addresses you can put the unique value in side the "References" header which most (but not all) mail programs will include in the reply.

@a1012112796
Copy link
Member Author

a1012112796 commented Nov 18, 2020

This is not secure and won't work as is since it will let you send an email and post a comment as any user you want since it only checks the from address which can be anything the user wants -- there needs to be unique keys per user that GItea already knows or can calculate in order to verify a message.

Github does this that each issue/pull has a unique ID for each user and that is included in the reply-to address in order to verify a comment (which require sub addressing) like Reply-To: go-gitea/gitea <reply+AAMXTQ7FKLLONE4ER6KUPGN5X5TCJEVBNHHXHSI8HW@reply.github.com>.

Gitlab has a second option that if you can't create random/unique reply to addresses you can put the unique value in side the "References" header which most (but not all) mail programs will include in the reply.

Ok, Thanks. I see, I not use in-reply-to because many mail service not use the message-id set by user, they will generate their's own, but set other heads (for example X-Microsoft-Original-Message-ID).
But I forgot References, maybe it's usefull, for secure, we should generate an uniq password and append it on the email to send and check it when recived emails. (like unsubscribe link in gh https://github.com/notifications/unsubscribe-auth/xxxxxxxxF7WKGXAPCJNxxxxxxM4TXINDYQ)

some usefull message in RFC 2822
https://tools.ietf.org/html/rfc2822

When creating a reply to a message, the "In-Reply-To:" and
   "References:" fields of the resultant message are constructed as
   follows:

   The "In-Reply-To:" field will contain the contents of the "Message-
   ID:" field of the message to which this one is a reply (the "parent
   message").  If there is more than one parent message, then the "In-
   Reply-To:" field will contain the contents of all of the parents'
   "Message-ID:" fields.  If there is no "Message-ID:" field in any of
   the parent messages, then the new message will have no "In-Reply-To:"
   field.

   The "References:" field will contain the contents of the parent's
   "References:" field (if any) followed by the contents of the parent's
   "Message-ID:" field (if any).  If the parent message does not contain
   a "References:" field but does have an "In-Reply-To:" field
   containing a single message identifier, then the "References:" field
   will contain the contents of the parent's "In-Reply-To:" field
   followed by the contents of the parent's "Message-ID:" field (if
   any).  If the parent has none of the "References:", "In-Reply-To:",
   or "Message-ID:" fields, then the new message will have no
   "References:" field.

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

a few notes

modules/cron/tasks_extended.go Outdated Show resolved Hide resolved
modules/cron/tasks_extended.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 18, 2020

Codecov Report

Merging #13585 (b00df2e) into master (0c0445c) will decrease coverage by 0.11%.
The diff coverage is 20.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13585      +/-   ##
==========================================
- Coverage   41.84%   41.73%   -0.12%     
==========================================
  Files         744      748       +4     
  Lines       79779    80212     +433     
==========================================
+ Hits        33387    33476      +89     
- Misses      40874    41211     +337     
- Partials     5518     5525       +7     
Impacted Files Coverage Δ
modules/base/tool.go 74.64% <ø> (ø)
services/imap/cron.go 0.00% <0.00%> (ø)
services/imap/mail_reciver.go 2.54% <2.54%> (ø)
services/imap/imap.go 12.77% <12.77%> (ø)
modules/setting/mail_recive.go 28.57% <28.57%> (ø)
modules/cron/tasks_extended.go 69.23% <66.66%> (-0.25%) ⬇️
models/issue_comment.go 53.19% <76.92%> (+0.90%) ⬆️
services/mailer/mail.go 62.43% <89.47%> (+1.14%) ⬆️
models/issue.go 56.88% <100.00%> (+0.07%) ⬆️
modules/setting/setting.go 49.13% <100.00%> (+0.09%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c0445c...b00df2e. Read the comment docs.

@a1012112796 a1012112796 added the pr/wip This PR is not ready for review label Dec 7, 2020
* master: (252 commits)
  Issues overview should not show issues from archived repos (go-gitea#13220)
  Display SVG files as images instead of text (go-gitea#14101)
  [skip ci] Updated translations via Crowdin
  Update docs to clarify issues raised in go-gitea#14272 (go-gitea#14318)
  [skip ci] Updated translations via Crowdin
  [Refactor] Passwort Hash/Set (go-gitea#14282)
  Add option to change username to the admin panel (go-gitea#14229)
  fix mailIssueCommentBatch for pull request (go-gitea#14252)
  Remove self from MAINTAINERS (go-gitea#14286)
  Do not reload page after adding comments in Pull Request reviews (go-gitea#13877)
  Fix session bug when introduce chi (go-gitea#14287)
  [skip ci] Updated translations via Crowdin
  Add secure/httpOnly attributes to the lang cookie (go-gitea#9690) (go-gitea#14279)
  Some code improvements (go-gitea#14266)
  [skip ci] Updated translations via Crowdin
  Fix wrong type on hooktask to convert typ from char(16) to varchar(16) (go-gitea#14148)
  Upgrade XORM links in documentation. (go-gitea#14265)
  Check permission for the appropriate unit type (go-gitea#14261)
  Add compliance check for windows to ensure cross platform build (go-gitea#14260)
  [skip ci] Updated translations via Crowdin
  ...
* master:
  Use Request.URL.RequestURI() for fcgi (go-gitea#14312) (go-gitea#14314)
  Update Link
  [skip ci] Updated translations via Crowdin
  Kd/add bountysource (go-gitea#14323)
* master: (27 commits)
  Use path not filepath in routers/editor (go-gitea#14390)
  Display error if twofaSecret cannot be retrieved (go-gitea#14372)
  Check if label template exist first (go-gitea#14384)
  Allow passcode invalid error to appear (go-gitea#14371)
  exclude authored PRs from Review Requested filter (go-gitea#14368)
  Upgrade blevesearch dependency to v2.0.1 (go-gitea#14346)
  Implement ghost comment mitigation (go-gitea#14349)
  Add edit, delete and reaction support to code review comments on issue page (go-gitea#14339)
  Add review requested filter on pull request overview (go-gitea#13701)
  escape branch names in compare url (go-gitea#14364)
  label and milestone webhooks on issue/pull creation (go-gitea#14363)
  Fix middlewares sequences (go-gitea#14354)
  Sort issue search results by revelance (go-gitea#14353)
  KanBan: be able to set default board (go-gitea#14147)
  ...
* master:
  Add pager to the branches page (go-gitea#14202)
  Removed invalid form tag (go-gitea#14391)
  Update back-up restore example for 1.13 changes (go-gitea#14374)
  It seems vet on windows is unnecessary (go-gitea#14302)
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@d2163df). Click here to learn what that means.
The diff coverage is 11.08%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #13585   +/-   ##
=======================================
  Coverage        ?   45.29%           
=======================================
  Files           ?      802           
  Lines           ?    89344           
  Branches        ?        0           
=======================================
  Hits            ?    40465           
  Misses          ?    42372           
  Partials        ?     6507           
Impacted Files Coverage Δ
cmd/hook.go 7.78% <0.00%> (ø)
models/notification.go 64.71% <ø> (ø)
modules/base/tool.go 93.47% <ø> (ø)
services/imap/cron.go 0.00% <0.00%> (ø)
services/imap/imap.go 0.00% <0.00%> (ø)
services/imap/mail_receiver.go 2.65% <2.65%> (ø)
modules/setting/mail_receive.go 28.57% <28.57%> (ø)
models/issue_comment.go 51.98% <46.15%> (ø)
modules/cron/tasks_extended.go 69.92% <66.66%> (ø)
services/mailer/mail.go 62.58% <75.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2163df...5589c19. Read the comment docs.

@a1012112796 a1012112796 removed the pr/wip This PR is not ready for review label Nov 18, 2021
@techknowlogick techknowlogick modified the milestones: 1.16.0, 1.17.0 Nov 23, 2021
@tobiasBora
Copy link

I'm not sure what's the status of this PR, but if subadressing is not available and headers are not preserved, it is always possible to include the token in the title of the email, it is preserved by all clients I guess (at most they will append some texts like "Re")

@lunny lunny modified the milestones: 1.17.0, 1.18.0 May 25, 2022
@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 17, 2022
@a1012112796 a1012112796 modified the milestones: 1.19.0, 1.x.x Nov 27, 2022
@lunny lunny closed this in #22056 Jan 14, 2023
lunny added a commit that referenced this pull request Jan 14, 2023
closes #13585
fixes #9067
fixes #2386
ref #6226
ref #6219
fixes #745

This PR adds support to process incoming emails to perform actions.
Currently I added handling of replies and unsubscribing from
issues/pulls. In contrast to #13585 the IMAP IDLE command is used
instead of polling which results (in my opinion 😉) in cleaner code.

Procedure:
- When sending an issue/pull reply email, a token is generated which is
present in the Reply-To and References header.
- IMAP IDLE waits until a new email arrives
- The token tells which action should be performed

A possible signature and/or reply gets stripped from the content.

I added a new service to the drone pipeline to test the receiving of
incoming mails. If we keep this in, we may test our outgoing emails too
in future.

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@a1012112796 a1012112796 deleted the imap branch January 15, 2023 15:13
@lunny lunny removed this from the 1.x.x milestone Jan 18, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could reply the issue notification email to add new reply to issue?