-
Notifications
You must be signed in to change notification settings - Fork 181
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
JWT validate only #51
Conversation
Hey, just curious, what's the use case? |
We have a React-based SPA and wanted to utilize MQTT via WebSockets to subscribe and receive messages from Mosquitto. For authentication, we wanted to simply use the users JWT. We initially tried to use the remote method, however we were unable to get the cookies over with the request (HTTP Only). From there we looked at using the local method. We quickly found that required a local database which contained user information. In our case we do not maintain user information as we utilize IAM. Thus, we needed the ability to utilize local mode without a database backing while ensuring token validity. I hope that helps. |
I've been giving this a thought for the last couple of days and I always end up wondering why you'd want to do this as it kind of beats the purpose of having both authentication and authorization mechanisms. Don't get me wrong, I'm not dismissing the use case, but there's probably better/cleaner ways to achieve that than throwing a check to see if the token was effectively signed by the secret (I mean, there are plenty of ways of doing this) and disregarding the rest of the information. I understand that's what you're exposing to your clients and it fits, but it seems kind of hackish. It kind of reminds me why I started writing this plugin on the first place not being able to do something similar with jpem's one. To be honest, I think this calls for a new backend that supports such check: if you were given a signed token and when tested it checks out against the owner's secret, then you're cool. The thing is it doesn't necessarily have to be a JWT secret, it could be any signing mechanism that's able to ensure that. And this could include a bunch of things, signed certificates being the first one that pops to my mind. Working and discussing which mechanisms to support would be really fun! On another note, I used to be super messy with this project when I started it because it was just serving my former employer simple needs and "it works" was just fine (and they just happened to trust me on this and gave me some time to introduce things they didn't need), but since then I've switched jobs and learned a couple of lessons: separation of concerns/issues is something I really value nowadays. So though I love your proposed changes to the Dockerfile (and I know they really need it!), I'd prefer to have them as a separate issues/PRs. I didn't mean to make this comment so boring/long, so to wrap it up: I really appreciate the PR and the will to contribute on something I had been thinking about but couldn't really put my finger on, so this is really helpful. For the same reasons, and if you have the time, I'd love to discuss it, be it here in comments, on a Slack channel, a video call, or whatever suits you. Just let me know. |
Hey @iegomez, I understand where you are coming from as far as the Authorization piece is concerned. However, I went forward based on the current methodology that is in use -- if no While my pull request did not specifically check for Authorization, I do believe the Authentication piece is still valid. If the token is signed with the same JWT secret then it is valid/authenticated. To cover the Authorization piece, I added a note in my initial comment describing this a bit:
What I was getting at was that I believe that the appropriate way to incorporate the Authorization piece is to utilize the "scp" (Scope) claims within the JWT itself. For example, we could configure our ACL file:
Then we give our user the "scp" of: {
...
"scp": ["mqtt_readers"]
} When the request comes in we can check the ACL file against the "scp" provided and either allow or disallow. Anyhow, something I was thinking about -- what are your thoughts? |
@intolerance I like your idea. If you wanna go ahead with that and remove the Docker parts you moved to the other PR, then I'll happily merge it. |
Hey, @intolerance, I logged out from the shared Slack some time ago because the app was working awfully with so many accounts and I had to focus on the work ones, so sorry for the prolonged silence. Are you still interested in delivering this feature? |
Hey @iegomez, I certainly would like to work on this feature on the future. I'm just not able to right at this moment. |
@intolerance That's cool, no rush. Thanks again! |
Hi @iegomez and @intolerance, |
Hey, @pmous, thanks for your interest in contributing! To do so just fork the repo, do the work in your repo and then open a Pull Request at mine so I may see the changes. |
Thanks for the instruction. I just did. @iegomez @intolerance check it out! |
@intolerance Hey, Allen! I've been terribly busy with work and thus am trying to cleanup the repo a bit so I can focus on any critical issue that may come up, as well as checking the upcoming 2.0 release of Mosquitto and needed changes to accomodate for it. Besides that, I'd say right now the plugin is basically in maintenance mode until I get the time to catch up and go forward, or maybe go along with an idea I've had for some time now of porting it to Rust (no fad involved, I just wanna get back to my long paused Rust learning I started and left due a long time ago, while probably improving the plugin's performance). On a related note, Paul (@pmous) came up with a follow up on your work (#90), but he was/is pretty busy too, and last I checked I was not entirely sold (and he agreed) on adding the special syntax he proposed. So if you want to chime in on that, please do whenever you get the time. If not, no worries: just let me know so I may close this PR and we can revisit the idea later. In any case, thanks again for bringing this up, it really started a nice discussion and brought up some issues I hadn't considered. Finally, let me know if any of you are interested in doing "periodic" contributions so I may grant you access to the repo. Right now settings restrict merging to my approvals just in case, but I believe you would both be valuable contributors and could become Cheers! |
@intolerance I have a similar use case (but with an Angular app), and as I mentioned in #90 I'd be happy to collaborate on a local jwt solution. |
I'm closing this issue in favor of #90, let's keep the discussion there (if anyone is willing after all the time that's passed, that is). |
@iegomez , I found myself simply wanting to validate a JWT token locally and was unable to accomplish that using the existing options.
This PR adds support for simple JWT validation against the supplied
jwt_secret
. At this point it does not perform any other ACL or Super/User checks. (they all returntrue
)Although, I think it would be pretty neat to use the existing
jwt_*query
options to check the "claims" against the files to determine if the token bearer has access.Additionally, I've included a
Dockerfile
that will allow you to build the plugin from your local source instead of pulling down the released source. I had to move this out to the root directory becausedocker build
will not let you traverse outside of it's pwd (e.g.../
). I included a note in the Docker part of the readme.I hope you will consider merging this PR -- if I need to update or clean up anything, please let me know.