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

Support authentication reverse proxy #67

Open
denysvitali opened this issue Mar 25, 2024 · 7 comments
Open

Support authentication reverse proxy #67

denysvitali opened this issue Mar 25, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@denysvitali
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

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 :))

@Benimautner Benimautner added the enhancement New feature or request label Apr 3, 2024
@denysvitali
Copy link
Contributor Author

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
Copy link
Member

kolaente commented Apr 4, 2024

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
Copy link
Contributor Author

denysvitali commented Apr 4, 2024

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).

denysvitali added a commit to denysvitali/vikunja_app that referenced this issue Apr 5, 2024
denysvitali added a commit to denysvitali/vikunja_app that referenced this issue Apr 5, 2024
@Benimautner
Copy link
Collaborator

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants