mattermost / mattermost-mobile

Next generation iOS and Android apps for Mattermost in React Native
https://about.mattermost.com/
Apache License 2.0
2.24k stars 1.36k forks source link

Latest beta mobile app doesn't send the Bearer token for external accounts #7922

Open sdomi opened 6 months ago

sdomi commented 6 months ago

Summary

If the user logs in through SSO (GitLab), certain requests are not authenticated properly against the API. image. This is only true for requests for files or images - sending and receiving messages works.

This only happens on Android + External Login. If user logs in on Android through the internal login, images load just fine.

Environment Information

Steps to reproduce

  1. Log in onto mattermost through the GitLab SSO integration on Android (bug doesn't occur on iOS, so far)
  2. enter any channel

Expected behavior

The timeline shows up fully.

Observed behavior (that appears unintentional)

Messages can be seen, but no images load. If requests are intercepted, they lack the Authorization header.

Possible fixes

n/a

amyblais commented 6 months ago

@sdomi We were unable to reproduce this internally. Is there any other information that you can help share (e.g. app/server logs or other info)?

sdomi commented 6 months ago

@amyblais thanks for the quick response! for now I've only gathered some data from adb logcat (as seen below), but I will attempt to dump you a (redacted) HAR of the requests later tonight.

04-26 15:23:40.849 29378 29378 W Glide   : Load failed for https://mm.sakamoto.pl/api/v4/files/1kg38i1fi7n1df3qzj1m6osjje/preview with size [1213x1093]
04-26 15:23:40.849 29378 29378 W Glide   : class com.bumptech.glide.load.engine.GlideException: Failed to load resource
04-26 15:23:40.849 29378 29378 W Glide   : There was 1 root cause:
04-26 15:23:40.849 29378 29378 W Glide   : com.bumptech.glide.load.HttpException(Unauthorized, status code: 401)
04-26 15:23:40.849 29378 29378 W Glide   :  call GlideException#logRootCauses(String) for more detail
04-26 15:23:40.849 29378 29378 W Glide   :   Cause (1 of 1): class com.bumptech.glide.load.engine.GlideException: Fetching data failed, class java.io.InputStream, REMOTE
04-26 15:23:40.849 29378 29378 W Glide   : There was 1 root cause:
04-26 15:23:40.849 29378 29378 W Glide   : com.bumptech.glide.load.HttpException(Unauthorized, status code: 401)
04-26 15:23:40.849 29378 29378 W Glide   :  call GlideException#logRootCauses(String) for more detail
04-26 15:23:40.849 29378 29378 W Glide   :     Cause (1 of 1): class com.bumptech.glide.load.HttpException: Unauthorized, status code: 401
04-26 15:23:40.849 29378 29378 I Glide   : Root cause (1 of 1)
04-26 15:23:40.849 29378 29378 I Glide   : com.bumptech.glide.load.HttpException: Unauthorized, status code: 401
04-26 15:23:40.849 29378 29378 I Glide   :     at com.bumptech.glide.integration.okhttp3.OkHttpStreamFetcher.onResponse(OkHttpStreamFetcher.java:71)
04-26 15:23:40.849 29378 29378 I Glide   :     at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
04-26 15:23:40.849 29378 29378 I Glide   :     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
04-26 15:23:40.849 29378 29378 I Glide   :     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
04-26 15:23:40.849 29378 29378 I Glide   :     at java.lang.Thread.run(Thread.java:1012)
amyblais commented 6 months ago

@larkox QA was unable to reproduce this, but I'm adding you here in case you're familiar with these errors.

larkox commented 6 months ago

@sdomi There might be some clues in the server logs. Can you enable the debug server logs and see what happens with those requests?

It may be also useful checking the app logs (Profile -> Settings -> Report a problem). There may be other errors, but let me know if you see any there about the loading of images.

amyblais commented 6 months ago

@sdomi Also, what device are you on? We've tested this on Samsung S22 (Android 14) and Pixel 5a.

sdomi commented 6 months ago

I'm on Pixel 7 Pro, latest GrapheneOS. this shouldn't affect the app tho.

I will provide more debug info later today - sorry for the belated response, lots of things happening the past few days.

sdomi commented 6 months ago

okay, managed to look through it:

The codepath I found is roughtly:

  1. emoji components requests a HTTP client
  2. NetworkManager returns an existing client; said client needs to be initialized earlier, which happens here
  3. said client gets its cookies from here
  4. cookiejar only contains those cookies if the login flow went fully through the app

My working theory is that a redirect to the browser wasn't intended, and instead it all should take place inside of the app? Either that, or something later was meant to save those cookies into the cookiejar and didn't.

I've got HARs dumped, but after comparing them request by request, they're exactly the same outside of the middle part of the auth flow. Redacting them would be kind of a pain to do, so I decided not to bother (sorry!).

"good" flow through the app: server returns Set-Cookie headers which get appended to the cookie jar; (request first, then response): image image


broken flow with GitLab: this request is still in the browser; the next one from the app goes to /api/v4/users/me, and it already has Authorization: BEARER ... set. Thus, no cookies. image image

larkox commented 6 months ago

@sdomi Were you able to check also the server logs? Our server code should be able to handle the token coming from different places (not only from the cookie) so that may not be the problem. I want to make sure what is the error the server is really reporting.

Maybe the body of the unauthorized response has also the real reason, but the one in the server logs should be more clear.

sdomi commented 6 months ago

@larkox the server response is as follows:

{
    "detailed_error": "",
    "id": "api.context.session_expired.app_error",
    "message": "Invalid or expired session, please login again.",
    "request_id": "1ani8qrkgpbfddbmqkjupqfx8e",
    "status_code": 401
}

this is, again, caused by neither the BEARER token nor the cookie being attached to the request.

(slightly redacted) server logs attached below logs.txt

larkox commented 6 months ago

@sdomi looking a bit more into the code, I see that some logs get sanitized when the instance is not in developer mode, in particular the "detail_error" field you see there. Not sure if it is an option to enable developer mode in your instance to get the logs.

I could understand it is not possible, so let me know either way.

sdomi commented 6 months ago

@larkox i can set up a separate developer instance for repro purposes later. Don't want to mess around with this one too much as it's mission-critical for me and my friends :)

will get back to you on that sometime soon

larkox commented 6 months ago

@sdomi Thank you so much for helping into troubleshooting this bug!

NonNobody commented 4 months ago

I'm on Pixel 7 Pro, latest GrapheneOS. this shouldn't affect the app tho.

I will provide more debug info later today - sorry for the belated response, lots of things happening the past few days.

I encountered the same issue. The problem is that if getDeviceToken fails, there is no request to /api/v4/users/sessions/device, which triggers the Set-Cookie operation for the device. GrapheneOS and other devices without google service will fail to execute getDeviceToken.

This bro figured out the problem: https://github.com/mattermost/mattermost-mobile/issues/4486#issuecomment-1729067894