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

Use FES /account/feedback endpoint #2827

Open
sosnovsky opened this issue Aug 8, 2024 · 17 comments · May be fixed by #2920
Open

Use FES /account/feedback endpoint #2827

sosnovsky opened this issue Aug 8, 2024 · 17 comments · May be fixed by #2920
Assignees
Milestone

Comments

@sosnovsky
Copy link
Collaborator

For feedback messages we need to migrate from deprecated https://flowcrypt.com/api/help/feedback endpoint to the newer shared-tenant-fes/api/v1/account/feedback endpoint.

Should be done after https://github.com/FlowCrypt/enterprise-server/issues/6123 is implemented and deployed

@DenBond7 DenBond7 self-assigned this Aug 8, 2024
@DenBond7 DenBond7 added this to the soon milestone Aug 8, 2024
@sosnovsky sosnovsky modified the milestones: soon, 1.6.1 Dec 27, 2024
@sosnovsky
Copy link
Collaborator Author

Hi @DenBond7, mentioned functionality was deployed to shared-tenant-fes, please include this task in the next Android release, thanks!

@DenBond7 DenBond7 modified the milestones: 1.6.1, 1.6.2 Dec 30, 2024
@DenBond7
Copy link
Collaborator

I've tried to migrate but I caught errors

-> POST https://flowcrypt.com/shared-tenant-fes/api/v1/account/feedback
2024-12-30 11:32:05.073 okhttp.OkHttpClient  com...email.debug  I  Content-Type: application/json; charset=UTF-8
2024-12-30 11:32:05.073 okhttp.OkHttpClient  com...email.debug  I  Content-Length: 196
2024-12-30 11:32:05.073 okhttp.OkHttpClient  com...email.debug  I  api-version: 3
2024-12-30 11:32:05.074 okhttp.OkHttpClient  com...email.debug  I  {"email":"[email protected]","logs":"","message":"Test issue \n\nhttps://github.com/FlowCrypt/flowcrypt-android/issues/2827\n\nversion: Android 1.6.1_dev_161__2024_12_30__b121724","screenshot":""}
2024-12-30 11:32:05.075 okhttp.OkHttpClient  com...email.debug  I  --> END POST (196-byte body)

2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  <-- 400 Bad Request https://flowcrypt.com/shared-tenant-fes/api/v1/account/feedback (189ms)
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Date: Mon, 30 Dec 2024 09:32:06 GMT
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Content-Type: application/json
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Content-Length: 107
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Connection: keep-alive
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Access-Control-Allow-Origin: *
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  X-Frame-Options: SAMEORIGIN
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  {"code":400,"message":"Domain disabled","details":"For more details, see FlowCrypt External Service logs."}
2024-12-30 11:32:05.264 okhttp.OkHttpClient  com...email.debug  I  <-- END HTTP (107-byte body)

@sosnovsky @martgil Please check my logs. Have I missed something?

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

Hello @DenBond7 - I'll check it in a second and get back to you.

@sosnovsky
Copy link
Collaborator Author

Feedback endpoint is disabled for flowcrypt.com accounts, can you please use some other account, like flowcrypt.compatibility

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

Den, I successfully received the feedback from your non-corporate email:

image

@DenBond7
Copy link
Collaborator

@martgil Please also check this error

2024-12-30 11:50:12.487 okhttp.OkHttpClient  com...email.debug  I  <-- 500 Server Error https://flowcrypt.com/shared-tenant-fes/api/v1/account/feedback (326ms)
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  Date: Mon, 30 Dec 2024 09:50:13 GMT
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  Content-Type: application/json
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  Connection: keep-alive
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  Access-Control-Allow-Origin: *
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  X-Frame-Options: SAMEORIGIN
2024-12-30 11:50:12.488 okhttp.OkHttpClient  com...email.debug  I  Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
2024-12-30 11:50:12.489 okhttp.OkHttpClient  com...email.debug  I  {"code":500,"message":"Illegal base64 character a","details":"java.lang.IllegalArgumentException: Illegal base64 character a\n\tat java.base/java.util.Base64$Decoder.decode0(Base64.java:848)\n\tat java.base/java.util.Base64$Decoder.decode(Base64.java:566)\n\tat java.base/java.util.Base64$Decoder.decode(Base64.java:589)\n\tat com.flowcrypt.shared.extensions.kotlin.StringKt.base64Decode(String.kt:14)\n\tat com.flowcrypt.fes.server.handlers.api.feedback.AccountFeedbackPost.handler(AccountFeedbackPost.kt:71)\n\tat com.flowcrypt.shared.server.handlers.EndpointHandler.handle(EndpointHandler.kt:65)\n\tat io.javalin.router.Endpoint.handle(Endpoint.kt:52)\n\tat io.javalin.router.ParsedEndpoint.handle(ParsedEndpoint.kt:15)\n\tat io.javalin.http.servlet.DefaultTasks.HTTP$lambda$9$lambda$7$lambda$6(DefaultTasks.kt:52)\n\tat io.javalin.http.servlet.JavalinServlet.handleTask(JavalinServlet.kt:99)\n\tat io.javalin.http.servlet.JavalinServlet.handleSync(JavalinServlet.kt:64)\n\tat io.javalin.http.servlet.JavalinServlet.handle(JavalinServlet.kt:50)\n\tat io.javalin.http.servlet.JavalinServlet.service(JavalinServlet.kt:30)\n\tat jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)\n\tat io.javalin.jetty.JavalinJettyServlet.service(JavalinJettyServlet.kt:52)\n\tat jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)\n\tat org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764)\n\tat org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:529)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)\n\tat org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1580)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)\n\tat org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1381)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)\n\tat org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484)\n\tat org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1553)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)\n\tat org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1303)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)\n\tat org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:173)\n\tat org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)\n\tat org.eclipse.jetty.server.Server.handle(Server.java:563)\n\tat org.eclipse.jetty.server.HttpChannel$RequestDispatchable.dispatch(HttpChannel.java:1598)\n\tat org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:753)\n\tat org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:501)\n\tat org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:287)\n\tat org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)\n\tat org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)\n\tat org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)\n\tat org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:421)\n\tat org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:390)\n\tat org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:277)\n\tat org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.run(AdaptiveExecutionStrategy.java:199)\n\tat org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:411)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPo
2024-12-30 11:50:12.489 okhttp.OkHttpClient  com...email.debug  I  ol$Runner.run(QueuedThreadPool.java:1149)\n\tat java.base/java.lang.Thread.run(Thread.java:840)\n"}
2024-12-30 11:50:12.489 okhttp.OkHttpClient  com...email.debug  I  <-- END HTTP (4099-byte body)

it looks like I can't send feedback with a screenshot

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

@DenBond7, were you able to share the HTTP request as well?

@DenBond7
Copy link
Collaborator

DenBond7 commented Dec 30, 2024

@DenBond7, were you able to share the HTTP request as well?

Sure. Also, just in case here is the code(how I prepare base64 string)


logs.txt

curl.txt

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

Got it, I'll just do several tests with the current implementation we have on the Production android app and try to compare them. I'll get back to you as soon as possible.

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

Hi @DenBond7 - I did my testing and investigation and i found out that, it directly base64 decodes the passed value of the screenshot.

However, the screenshot base64 data is segmented by newlines \n - https://github.com/FlowCrypt/flowcrypt-backend/blob/4dd30c43bcc1426e5a3ca4379c5d3b53b037a5eb/cryptup/email.py#L57

In comparison, seems like the port we have in the Enterprise Server [ES] does not have that and directly base64decodes it: https://github.com/FlowCrypt/enterprise-server/blob/3e1626ee3496b2929b11a070f7906892524ff6b6/fes-lib/src/main/kotlin/com/flowcrypt/fes/server/handlers/api/feedback/AccountFeedbackPost.kt#L71

I also manually decodes the images to ensure I have the right information to share:

image

We should've fix the issue on the ES codebase.

cc @sosnovsky

@DenBond7
Copy link
Collaborator

Thank you, Mart!

However, the screenshot base64 data is segmented by newlines \n - https://github.com/FlowCrypt/flowcrypt-backend/blob/4dd30c43bcc1426e5a3ca4379c5d3b53b037a5eb/cryptup/email.py#L57

In comparison, seems like the port we have in the Enterprise Server [ES] does not have that and directly base64decodes it: https://github.com/FlowCrypt/enterprise-server/blob/3e1626ee3496b2929b11a070f7906892524ff6b6/fes-lib/src/main/kotlin/com/flowcrypt/fes/server/handlers/api/feedback/AccountFeedbackPost.kt#L71

It helped me.

When I change my code to use java.util.Base64 instead of android.util.Base64 posting feedback works well. It seems it was an unsupported setup on my side.

image

The following changes work well

https://stackoverflow.com/questions/32935783/java-different-results-when-decoding-base64-string-with-java-util-base64-vs-and

val screenShotBase64 =
        Base64.encodeToString(screenshot?.byteArray ?: byteArrayOf(), Base64.NO_WRAP)

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

Thanks, Den. I checked again your recently sent email with screenshot. As I'm checking them, they now have data but they do not resemble to an image yet.

image image

@DenBond7
Copy link
Collaborator

I checked again your recently sent email with screenshot. As I'm checking them, they now have data but they do not resemble to an image yet.

Got it.

Should I switch to use

val screenShotBase64 =
        Base64.encodeToString(screenshot?.byteArray ?: byteArrayOf(), Base64.NO_WRAP)

or should I leave things as-is and wait for the server changes?

@martgil
Copy link
Collaborator

martgil commented Dec 30, 2024

I haven't checked the result if the app base64 the screenshot with

val screenShotBase64 =
        Base64.encodeToString(screenshot?.byteArray ?: byteArrayOf(), Base64.NO_WRAP)`

I'll be checking that right now, if its ok? If not, then I'll also check if adopting the ES base64 decoding with code from https://github.com/FlowCrypt/enterprise-server/blob/3e1626ee3496b2929b11a070f7906892524ff6b6/fes-lib/src/main/kotlin/com/flowcrypt/fes/server/handlers/api/feedback/AccountFeedbackPost.kt#L71 will let us achieve our goal.

@martgil
Copy link
Collaborator

martgil commented Dec 31, 2024

Hi @DenBond7, I have answer to your question now.

I checked your PR here #2920 and it is generating correct base64 image too.

image

Additionally, I tested to send the same base64 image against old api in /api/help/feedback and it send an ok email with a correct attached screenshot.

This make me think that its the ES that needs to updated. I have tried to diff two screenshots generated given the request is exactly the same and sent on two different APIs. 1-good.png is the result from /api/help/feedback where base64 image for me does not require me to unescape the base64 string. The file 1-good.png is detected as PNG image and can be opened meanwhile the other image cannot be opened.

image

@martgil
Copy link
Collaborator

martgil commented Dec 31, 2024

I have further test the ES server side and when doing file writing, I'm able to save and open the base64 file and it turns out to be decoded very well where the data encoded came from #2920 as I manually tested it:

image

One thing left is that, then uploading it and putting it as an attachment, its file size noticiably changed from 65590 to 61494 considering im sending the same exact base64 screenshot... Does that mean there's some problem with the upload?

@DenBond7
Copy link
Collaborator

DenBond7 commented Jan 2, 2025

One thing left is that, then uploading it and putting it as an attachment, its file size noticiably changed from 65590 to 61494 considering im sending the same exact base64 screenshot... Does that mean there's some problem with the upload?

Hi @martgil,

If I understand you correctly, we have 65590 for the old realization and 61494 for the new.
It looks ok. In the new version(#2920) I use Base64.NO_WRAP. It produces a smaller string.

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 a pull request may close this issue.

3 participants