go-vikunja / app

MIT License
250 stars 23 forks source link

Support authentication reverse proxy #67

Open denysvitali opened 5 months ago

denysvitali commented 5 months ago

When Vikunja's backend is protected by an authentication reverse proxy such as oauth2-proxy, the app can't log-in.

For this to work, the app should open a browser, take the cookies after the session is authenticated and re-use them until the session expires.

This would allow to use Vikunja behind an Identity-Aware Proxy for increased security of private instances.

denysvitali commented 5 months ago

Instead of implementing the support for in-browser authentication (which then requires to handle redirects & so on) I decided to add a special header (X-Client-Token) to only allow specific clients to access my server.

This of course requires the token to be set in the app as well, which is why I've created feature/add-auth-token

Benimautner commented 5 months ago

In general, I like the idea of supporting this. However, I'm not sure if a manual field is the best idea. Ideally, we would parse the token from the webview login, but I will have to check how we would do that. So for getting this into the app quickly, I like your implementation.

I've looked through your changes, and the only large thing I don't want to do is push the minSdkVersion that high. Is there a particular reason why you did that?

Also hiding the field by having to tap on the logo is a nice idea, but I'm not particularly fond of it bc of feature discoverability. I'd prefer a field in the settings. This would also allow the changing of the token (if it expires) without having to go back to the login screen and having to enter the Vikunja credentials again.

Let me know what you think, and definitely open a pull request if you're satisfied with your changes!

Anyways, thank you so much for actually doing the work yourself and not just requesting the feature :))

denysvitali commented 5 months ago

However, I'm not sure if a manual field is the best idea.

The main issue here is that a lot of those identity-aware proxies, like oauth2-proxy, just send you straight to the Identity Provider, such as Google. These proxies are mainly built for use in browsers, so they save session cookies. But to actually use these cookies from the app, you've got to catch the request and use the in-app browser. The problem is, Google thinks this is dodgy and blocks logins from in-apps by checking stuff like the User-Agent, making it pretty much useless.

My workaround to keep Vikunja's backend not directly exposed to the internet is to use the X-Client-Token as a shared secret. It's like a first layer of defense. Without this token, nobody's getting into the backend. This is a first-layer of defense in case Vikunja's backend has a security flaw.

I've looked through your changes, and the only large thing I don't want to do is push the minSdkVersion that high. Is there a particular reason why you did that?

IIRC this was required by one of the dependencies. When I implemented the two features (#66 and #67), I updated all the dependencies - which required bumping the minSdkVersion. You can try to revert that line, but IIRC it was necessary.

Also hiding the field by having to tap on the logo is a nice idea, but I'm not particularly fond of it bc of feature discoverability.

Yes, I did this to avoid non-expert users to see this additional field, that in 90% of the cases is completely useless.

I'd prefer a field in the settings.

On the first log-in, you can't access the settings (IIRC) - thus you can't perform the first login since you need the token to not get a 403 from the reverse proxy. Maybe we can find a way to expose the settings from the login screen.

This would also allow the changing of the token (if it expires)

In my case, this is a shared secret that never expires - but in other cases this might be something that keeps on changing. I still think the X-Client-Token is more of a hack than a real feature, but at least it allows me to use Vikunja's app when I'm not in front of my computer :)

Let me know what you think, and definitely open a pull request if you're satisfied with your changes!

I'm, and I've been using the two changes for a while now. They might be very-specific to my use case, so I get it if you don't want to merge them.

I can definitely open a PR for both

Anyways, thank you so much for actually doing the work yourself and not just requesting the feature :))

Don't mind me, just giving back my small contribution to this awesome project. Thank you!

kolaente commented 5 months ago

Honest question (without having much knowledge about how this kind of proxy-auth works), why don't you just disable manual registration and use Vikunja with the users you already created? If privacy is your main concern?

In my case, this is a shared secret that never expires

If the token never expires, why not leave it out?

denysvitali commented 5 months ago

It's just a safeguard against possible exploits / vulnerabilities on the Vikunja backend. Of course registration is already disabled - but if the instance isn't reachable from anyone to begin with, it's even better.

If the token never expires, why not leave it out?

Because then the instance (Vikunja's backend) is reachable from anyone on the internet. An exploit there would mean that anonymous users will be able to access the instance data / pivot from there (depending on the setup).

Benimautner commented 4 months ago

If you don't want to expose your programs to the public internet (which is understandable), the recommended approach would be a VPN server. Every operating system supports setting those up transparently, without the apps noticing. That's inherently better since not every app has to support every authentification protocol. You might want to look into setting up Wireguard (full and independent vpn server, harder to install), or at least Tailscale (super easy to install, but your traffic is routed through their servers afaik)

denysvitali commented 4 months ago

I already use Tailscale - but I don't want to use Tailscale here because this is an HTTPS connection that can use directly my IdP