From b52706a3ca0fef1b07993c73fd8a6fd261ec3fa6 Mon Sep 17 00:00:00 2001 From: Romain Le Jeune Date: Fri, 20 Oct 2023 10:14:20 +0000 Subject: [PATCH] feat(GODT-3015): Add simple algorithm to deal with multiple attachment for bug report. --- go.mod | 2 +- go.sum | 4 +- internal/bridge/bug_report.go | 143 +++++++++++++++------ internal/frontend/grpc/service_methods.go | 23 ++-- tests/bridge_test.go | 46 +++---- tests/features/user/report_problem.feature | 2 +- 6 files changed, 141 insertions(+), 79 deletions(-) diff --git a/go.mod b/go.mod index 972bfa36..9e41d6f5 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.2.0 github.com/ProtonMail/gluon v0.17.1-0.20231009084701-3af0474b0b3c github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.4.1-0.20231018070752-2449db500edd + github.com/ProtonMail/go-proton-api v0.4.1-0.20231020080353-2b3fddd7f72d github.com/ProtonMail/gopenpgp/v2 v2.7.3-proton github.com/PuerkitoBio/goquery v1.8.1 github.com/abiosoft/ishell v2.0.0+incompatible diff --git a/go.sum b/go.sum index 17abd677..06ac5e38 100644 --- a/go.sum +++ b/go.sum @@ -34,8 +34,8 @@ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7 h1:+j+Kd/ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7/go.mod h1:NBAn21zgCJ/52WLDyed18YvYFm5tEoeDauubFqLokM4= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f h1:tCbYj7/299ekTTXpdwKYF8eBlsYsDVoggDAuAjoK66k= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f/go.mod h1:gcr0kNtGBqin9zDW9GOHcVntrwnjrK+qdJ06mWYBybw= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231018070752-2449db500edd h1:/Z3nwzsVeSwTFkrIKG7BLD24P4cxpaJBn8mzbSe2kPg= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231018070752-2449db500edd/go.mod h1:ZmvQMA8hanLiD1tFsvu9+qGBcuxbIRfch/4z/nqBhXA= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231020080353-2b3fddd7f72d h1:w+DiszDUNfGz64zEmmOnetV+YQssrLcOr3NkofW/Nog= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231020080353-2b3fddd7f72d/go.mod h1:ZmvQMA8hanLiD1tFsvu9+qGBcuxbIRfch/4z/nqBhXA= github.com/ProtonMail/go-srp v0.0.7 h1:Sos3Qk+th4tQR64vsxGIxYpN3rdnG9Wf9K4ZloC1JrI= github.com/ProtonMail/go-srp v0.0.7/go.mod h1:giCp+7qRnMIcCvI6V6U3S1lDDXDQYx2ewJ6F/9wdlJk= github.com/ProtonMail/gopenpgp/v2 v2.7.3-proton h1:wuAxBUU9qF2wyDVJprn/2xPDx000eol5gwlKbOUYY88= diff --git a/internal/bridge/bug_report.go b/internal/bridge/bug_report.go index bf444588..0a96cb91 100644 --- a/internal/bridge/bug_report.go +++ b/internal/bridge/bug_report.go @@ -19,6 +19,7 @@ package bridge import ( "context" + "errors" "io" "github.com/ProtonMail/go-proton-api" @@ -33,65 +34,133 @@ const ( DefaultMaxSessionCountForBugReport = 10 ) -func (bridge *Bridge) ReportBug(ctx context.Context, osType, osVersion, title, description, username, email, client string, attachLogs bool) error { - var account = username +type ReportBugReq struct { + OSType string + OSVersion string + Title string + Description string + Username string + Email string + EmailClient string + IncludeLogs bool +} - if info, err := bridge.QueryUserInfo(username); err == nil { - account = info.Username +func (bridge *Bridge) ReportBug(ctx context.Context, report *ReportBugReq) error { + if info, err := bridge.QueryUserInfo(report.Username); err == nil { + report.Username = info.Username } else if userIDs := bridge.GetUserIDs(); len(userIDs) > 0 { if err := bridge.vault.GetUser(userIDs[0], func(user *vault.User) { - account = user.Username() + report.Username = user.Username() }); err != nil { return err } } - var attachment []proton.ReportBugAttachment - - if attachLogs { - logsPath, err := bridge.locator.ProvideLogsPath() + var attachments []proton.ReportBugAttachment + if report.IncludeLogs { + logs, err := bridge.CollectLogs() if err != nil { return err } - - buffer, err := logging.ZipLogsForBugReport(logsPath, DefaultMaxSessionCountForBugReport, DefaultMaxBugReportZipSize) - if err != nil { - return err - } - - body, err := io.ReadAll(buffer) - if err != nil { - return err - } - - attachment = append(attachment, proton.ReportBugAttachment{ - Name: "logs.zip", - Filename: "logs.zip", - MIMEType: "application/zip", - Body: body, - }) + attachments = append(attachments, logs) } - safe.Lock(func() { + var firstAtt proton.ReportBugAttachment + if len(attachments) > 0 && report.IncludeLogs { + firstAtt = attachments[0] + } + + attachmentType := proton.AttachmentTypeSync + if len(attachments) > 1 { + attachmentType = proton.AttachmentTypeAsync + } + + token, err := bridge.createTicket(ctx, report, attachmentType, firstAtt) + if err != nil || token == "" { + return err + } + + safe.RLock(func() { for _, user := range bridge.users { user.ReportBugSent() } }, bridge.usersLock) - _, err := bridge.api.ReportBug(ctx, proton.ReportBugReq{ - OS: osType, - OSVersion: osVersion, + // if we have a token we can append more attachment to the bugReport + for i, att := range attachments { + if i == 0 && report.IncludeLogs { + continue + } + err := bridge.appendComment(ctx, token, att) + if err != nil { + return err + } + } + return err +} - Title: "[Bridge] Bug - " + title, - Description: description, +func (bridge *Bridge) CollectLogs() (proton.ReportBugAttachment, error) { + logsPath, err := bridge.locator.ProvideLogsPath() + if err != nil { + return proton.ReportBugAttachment{}, err + } - Client: client, + buffer, err := logging.ZipLogsForBugReport(logsPath, DefaultMaxSessionCountForBugReport, DefaultMaxBugReportZipSize) + if err != nil { + return proton.ReportBugAttachment{}, err + } + + body, err := io.ReadAll(buffer) + if err != nil { + return proton.ReportBugAttachment{}, err + } + + return proton.ReportBugAttachment{ + Name: "logs.zip", + Filename: "logs.zip", + MIMEType: "application/zip", + Body: body, + }, nil +} + +func (bridge *Bridge) createTicket(ctx context.Context, report *ReportBugReq, + asyncAttach proton.AttachmentType, att proton.ReportBugAttachment) (string, error) { + var attachments []proton.ReportBugAttachment + attachments = append(attachments, att) + res, err := bridge.api.ReportBug(ctx, proton.ReportBugReq{ + OS: report.OSType, + OSVersion: report.OSVersion, + + Title: "[Bridge] Bug - " + report.Title, + Description: report.Description, + + Client: report.EmailClient, ClientType: proton.ClientTypeEmail, ClientVersion: constants.AppVersion(bridge.curVersion.Original()), - Username: account, - Email: email, - }, attachment...) + Username: report.Username, + Email: report.Email, - return err + AsyncAttachments: asyncAttach, + }, attachments...) + + if err != nil || asyncAttach != proton.AttachmentTypeAsync { + return "", err + } + + if asyncAttach == proton.AttachmentTypeAsync && res.Token == nil { + return "", errors.New("no token returns for AsyncAttachments") + } + + return *res.Token, nil +} + +func (bridge *Bridge) appendComment(ctx context.Context, token string, att proton.ReportBugAttachment) error { + var attachments []proton.ReportBugAttachment + attachments = append(attachments, att) + return bridge.api.ReportBugAttachement(ctx, proton.ReportBugAttachmentReq{ + Product: proton.ClientTypeEmail, + Body: "Comment adding attachment: " + att.Filename, + Token: token, + }, attachments...) } diff --git a/internal/frontend/grpc/service_methods.go b/internal/frontend/grpc/service_methods.go index 0c703e7b..48024cef 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -339,18 +339,17 @@ func (s *Service) ReportBug(_ context.Context, report *ReportBugRequest) (*empty defer async.HandlePanic(s.panicHandler) defer func() { _ = s.SendEvent(NewReportBugFinishedEvent()) }() - - if err := s.bridge.ReportBug( - context.Background(), - report.OsType, - report.OsVersion, - report.Title, - report.Description, - report.Address, - report.Address, - report.EmailClient, - report.IncludeLogs, - ); err != nil { + reportReq := bridge.ReportBugReq{ + OSType: report.OsType, + OSVersion: report.OsVersion, + Title: report.Title, + Description: report.Description, + Username: report.Address, + Email: report.Address, + EmailClient: report.EmailClient, + IncludeLogs: report.IncludeLogs, + } + if err := s.bridge.ReportBug(context.Background(), &reportReq); err != nil { s.log.WithError(err).Error("Failed to report bug") _ = s.SendEvent(NewReportBugErrorEvent()) return diff --git a/tests/bridge_test.go b/tests/bridge_test.go index d268ec44..6750e7ae 100644 --- a/tests/bridge_test.go +++ b/tests/bridge_test.go @@ -155,34 +155,29 @@ func (s *scenario) theUserSetSMTPModeToSSL() error { } type testBugReport struct { - OSType string `json:"OS"` - OSVersion string `json:"OSVersion"` - Title string `json:"Title"` - Description string `json:"Description"` - Username string `json:"Username"` - Email string `json:"Email"` - Client string `json:"Client"` - Attachment bool `json:"Attachment"` - - bridge *bridge.Bridge + request bridge.ReportBugReq + bridge *bridge.Bridge } -func newTestBugReport(bridge *bridge.Bridge) *testBugReport { - return &testBugReport{ +func newTestBugReport(br *bridge.Bridge) *testBugReport { + request := bridge.ReportBugReq{ OSType: "osType", OSVersion: "osVersion", Title: "title", Description: "description", Username: "username", Email: "email", - Client: "client", - Attachment: false, - bridge: bridge, + EmailClient: "client", + IncludeLogs: false, + } + return &testBugReport{ + request: request, + bridge: br, } } func (r *testBugReport) report() error { - return r.bridge.ReportBug(context.Background(), r.OSType, r.OSVersion, r.Title, r.Description, r.Username, r.Email, r.Client, r.Attachment) + return r.bridge.ReportBug(context.Background(), &r.request) } func (s *scenario) theUserReportsABug() error { @@ -194,25 +189,25 @@ func (s *scenario) theUserReportsABugWithSingleHeaderChange(key, value string) e switch key { case "osType": - bugReport.OSType = value + bugReport.request.OSType = value case "osVersion": - bugReport.OSVersion = value + bugReport.request.OSVersion = value case "Title": - bugReport.Title = value + bugReport.request.Title = value case "Description": - bugReport.Description = value + bugReport.request.Description = value case "Username": - bugReport.Username = value + bugReport.request.Username = value case "Email": - bugReport.Email = value + bugReport.request.Email = value case "Client": - bugReport.Client = value + bugReport.request.EmailClient = value case "Attachment": att, err := strconv.ParseBool(value) if err != nil { return fmt.Errorf("failed to parse bug report attachment preferences: %w", err) } - bugReport.Attachment = att + bugReport.request.IncludeLogs = att default: return fmt.Errorf("Wrong header (\"%s\") is being checked", key) } @@ -222,10 +217,9 @@ func (s *scenario) theUserReportsABugWithSingleHeaderChange(key, value string) e func (s *scenario) theUserReportsABugWithDetails(value *godog.DocString) error { bugReport := newTestBugReport(s.t.bridge) - if err := json.Unmarshal([]byte(value.Content), &bugReport); err != nil { + if err := json.Unmarshal([]byte(value.Content), &bugReport.request); err != nil { return fmt.Errorf("cannot parse bug report details: %w", err) } - return bugReport.report() } diff --git a/tests/features/user/report_problem.feature b/tests/features/user/report_problem.feature index a762abb0..b8b574cd 100644 --- a/tests/features/user/report_problem.feature +++ b/tests/features/user/report_problem.feature @@ -41,7 +41,7 @@ Feature: The user reports a problem "Description": "Testing Description", "Username": "[user:user]", "Email": "[user:user]@[domain]", - "Client": "Apple Mail" + "EmailClient": "Apple Mail" } """ Then the header in the "POST" multipart request to "/core/v4/reports/bug" has "Title" set to "[Bridge] Bug - Testing Title"