Skip to content

Add form field support for JWT middleware #1697

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

Closed
3 tasks done
rkfg opened this issue Nov 25, 2020 · 0 comments · Fixed by #1704
Closed
3 tasks done

Add form field support for JWT middleware #1697

rkfg opened this issue Nov 25, 2020 · 0 comments · Fixed by #1704

Comments

@rkfg
Copy link
Contributor

rkfg commented Nov 25, 2020

Issue Description

I've added a new TokenLookup value form:<name> to use an HTML form field as a source of JWT tokens. It's trivial and I can open a pull request but I feel like posting an issue first is a good idea.

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Rationale

I found it a bit hard to create printed forms securely in my web apps. They're opened in new tabs and contain simple HTML. I need to pass some parameters to them (according to the user's input) as well as the authentication information. Since I use Authorization: Bearer header for the latter in the webapp itself I need another way to pass the token without using JS (and it's not possible to open a new tab with a custom HTTP header). So it has to be either a GET or POST parameter.

I really don't want to pass a token as a query parameter because it can easily leak this way. I also don't want to use cookies because it opens a CSRF attack possibility. I ended up with a hidden form input that contains the token (in the JS webapp) that opens a new tab with POST method and passes other user-defined values as query parameters so they're visible and easy to reproduce. But Echo can't extract the token from a form field so I added that and it works just fine.

I also wrote tests (expanded the existing test actually) for this new functionality. Tell me if it's worth adding and I'll post the patch.

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 a pull request may close this issue.

1 participant