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

Remove query JWTs (breaking change) #793

Conversation

omninonsense
Copy link
Contributor

@omninonsense omninonsense commented Sep 29, 2024

Is there a good reason to need to support query JWTs at the framework level?

The reason why I ask is because I believe enabling query JWTs punches a small security hole into the framework, and by extension Loco's users.

Intermediate proxies log requests, the requirements for shipping/aggregating those is usually less secure than applications and surrounding infrastructure themselves. This can be further exacerbated by engineering and ops teams having access to production logs via their machines. Finally, end users can also accidentally end up sharing their tokens. All of those can lead to session stealing/hijacking/replays.

I realise a lot of these are self inflicted errors, and some of them can be worked around or diminished, but I don't expect everyone to be hyper-vigilant about security!

Lastly, I don't think it has a great cost-benefit ratio: the cost is very steep (potential security hole, which are nightmares on multiple fronts: monetary, reputation, data loss), while the benefit is negligible: most anything sending a request can attach a header or a cookie already; usually attaching headers/cookies is also ergonomic than fiddling with query params.

Overall, I'd rather Loco errs on the side of providing only safe options out of the box, especially surrounding stuff like AuthN/AuthZ.

If users really want or need to implement something like this, they can shoot themselves in the foot on their own terms, but at least we've not given them the gun!

Hope I managed to get my thoughts across clearly. Also, I realise this might be a Chesterton's fence type of situation, though! So, I'm happy to discuss and even be convinced otherwise!

@vanhalt
Copy link

vanhalt commented Oct 9, 2024

Great reasoning behind your opinion.

The reasons that I see to keep the query JWT are:

  • It still an option without framework user interaction, thus, reduces usage friction.
  • In my experience query parameters are easy to filter in monitoring tools.
  • In your case there is the assumption that all the proxies are external to the infrastructure. In a micro services world an internal service could identify services by this token on the query params.
  • Intermediate proxies can read/re-write headers
  • There are too many scenarios that are not considered.

Having said that. I think that the approach could be to leave it as an option but discourage it's usage mentioning the potential security risks.

@kaplanelad kaplanelad closed this Oct 13, 2024
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 this pull request may close these issues.

3 participants