feat(mobile): Added support for BasicAuth via userinfo in server url #6617
Conversation
jpg0
changed the title
jpg0
changed the title
|
what is the use case of this? |
|
This is to allow deployment of immich behind a proxy implementing/enforcing
BasicAuth. My personal use-case is that I host Immich at home and want to
use the mobile app when I'm not at home, but want to follow the advice of
not opening a developing product up to the internet (and also not having to
regularly toggle a VPN on my phone). This means assuming that I trust
BasicAuth at the proxy level, no security hole in Immich can be exploited
if I open it up.
There is also a discussion thread on exactly this use-case; I'll link it
here...
…On Wed, 24 Jan 2024, 1:27 pm Alex, ***@***.***> wrote:
what is the use case of this?
—
Reply to this email directly, view it on GitHub
<#6617 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFNH5UKEL32BATQDEWXSPLYQCLUNAVCNFSM6AAAAABCIDC6RWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBXGQYDEOBXGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Existing discussion: #3204 |
|
So it is currently working with the web app but not the mobile app, correct? |
|
Yes. With the web app the browser handles this automatically. |
jpg0
force-pushed
the
feat/basic-auth-in-url
branch
from
94ce4f7
to
6998ddb
Compare
|
I would really appreciate if Basic Auth was working. I have various services running on my home server, exposed via nginx with mTLS. This way, I have an extra layer of protection in case of application bugs (e.g. forgotten authorization check for an endpoint), because attackers would additionally need my certificate (or a 0-day in nginx). Otherwise none of the resources are reachable. There's a discussion saying that mTLS will probably never be supported in the app because the underlying framework doesn't support it. Fair enough. My next attempt was Basic Auth, but while the initial connection attempt (like fetching server config) used the username/password, further requests added the Bearer token and that overwrote Basic Auth. I currently use a separate domain with a secret path prefix (like https://app.photos.example.org/xyz). I disabled mTLS for that domain and forward only /xyz to Immich. Since the path is only transmitted within TLS, that should be save (as long as it's a bit more complex than xyz). But obviously, it's a hack and also doesn't work in the browser (which requires Immich to be served from top level). So I'll gladly test this PR with nginx. Is there any automatically built APK? |
|
I don't believe that there is an official APK built automatically (well, certainly not until the workflows are run), but I have the one that I have built and am now using. It is not using the official signing key however (as I don't have that), so you'd need to uninstall the official Immich before installing this one. It's too big to attach here, but you can get it from my Google drive: https://drive.google.com/file/d/14t3WGoRfSqpSz7-M8JsC2sm8M2ucPf1r/view?usp=sharing |
This sounds to me like basic auth already 'just works', except we end up overwriting the auth header with the Immich token later on. If that is the case, it should be sufficient to just always put the Immich token in |
TBH I'm a little confused at this because it certainly didn't for me. There is some fallback mechanism in the app which allows you to proceed to requesting username/password even if it cannot access the server info, but there is no BasicAuth headers actually being sent to the server. As you can see in the commit for this PR, the userInfo data is explicitly pulled from the URI and attached as BasicAuth. (Actually this is pretty much all that the change actually does.) |
jpg0
force-pushed
the
feat/basic-auth-in-url
branch
from
6998ddb
to
2718ee1
Compare
Then I misunderstood. I expected that whatever http client we're using already supported basic auth under the hood but we were just overwriting the headers. |
|
From my nginx logs: In that example I used something like |
|
All the URLs in your log don't require authentication. Also if you set a URL with a user:pass at the front then typically the app just throws them away, so what you are seeing makes sense to me for a typical Immich install. However I thought that you would be routing everything through a reverse-proxy and requiring basic auth on all requests - if so then I don't understand what is going on there as I don't believe that the basic auth headers would be sent. |
| if(uri.userInfo.contains(":")) { //need BasicAuth, use custom header for Bearer Auth | ||
| headers['Authorization'] = "Basic ${base64.encode(utf8.encode(uri.userInfo))}"; | ||
|
|
||
| if(accessToken != null) { | ||
| headers['x-immich-user-token'] = accessToken; | ||
| } | ||
|
|
||
| } else if(accessToken != null) { | ||
| headers['Authorization'] = 'Bearer $accessToken'; | ||
| } |
There was a problem hiding this comment.
Why not simply always add the access token to the custom header instead?
|
Looks like i missed a few places! I'm certainly happy with switching the token header entirely - I only left it as-is for non Basic to try to change as little as possible, but I agree a single approach is better. Only one thing I have a question on which is the client sending basic auth headers - i'll comment on your PR though. |
|
superseded by #6840 |
Added support for BasicAuth protected immich servers to mobile apps.
Allows specifying credentials in
https://user:pass@hostformat in server URL. They will be sent in theAuthorizationheader for all requests - in this case moving the bearer token auth to the (already supported)x-immich-user-token(it's not moved when BasicAuth is not used). I have added a note in the docs, and tested using Traefik.