jwt-backend local but without db. Use rsa public-key for secret.

[pmous]

I want to use the jwt-backend without remote calls, but also without a local database. This is possible, because the jwt is signed and can be checked with a public key. So I made a couple of changes:

• the option jwt_db can now hold the value "none". This disables the db-checks.
• the option jwt_aclquery together with a new jwt_acl_scope_field option determine if a user has access.

jwt_aclquery contains a comma separated string with expressions. Each expression consists of a string to match with, then a column (:), then a regex to match against. Both string and regex can have placeholders.
The placeholders in the form %fieldname% are substituted first with as values the fields from the claims (short, lowercase variants, as they are in json). Additionally, %clientId%, %topic%, %access% and %scope% can be used. %access% is the requested access as string, so either read, write or subscribe. If access is readwrite, the test is done for write and read apart and AND'd. %scope% is taken from the claims, the field is set in config jwt_acl_scope_field. It is then split on spaces and for each %scope% is filled and the expression is evaluated. On the first match access is granted (so OR'd). If no scope is given, ["default"] is used.
Example:
auth_opt_jwt_aclquery %scope% %access% %topic%:read-scope read topic/%sub%,%scope% %access% %topic%:test-scope (read|write|subscribe) other/%clientId%/%sub%
auth_opt_jwt_acl_scope_field scope

The Secret-property should be stored as interface{} to interface better with ParseRSAPublicKeyFromPEM. Problem is that Secret is exported (why?), so this might cause problems in other uses. Other solution might be to convert the parsed RSA to a byte-sequence. Not sure how...
For now a RSA is assumed. Would be nice to have some sort of auto-type-select for this.

[iegomez]

He @pmous!

Could you update the title and give a description of what's the PR intention, along with any relevant details for it?
Also, please run the code through gofmt.

Thanks!

[pmous]

Hi @iegomez

I am not sure I understand what's wrong with the title. The PR adds a non-db option to the local mode of the jwt-backend. That is the first part of the title. The second part is there because I did a small addition to allow easy inclusion of a rsa-public file. But I will shorten the title, because it is now cut off. Perhaps I am missing something, so if you have another suggestion, please let me know.
I will fix the description shortly.
I am working right now on a revision, because I came upon a big problem, I will comment on that in another comment.
I will run the next commit through gofmt.

[pmous]

I came upon the following problem. The acl check is done for every publish and also for every listener apart. The plugin is sending the user (which is the jwt) to that call. In my code that jwt is validated and is expired at some point. But since the listener is passive, the jwt is not renewed. The client is completely unaware that starts missing messages, because mosquitto simply ignores that listener if the acl fails. And even for an active event, like publising, what should the client do? Disconnect?? There is no way the client can tell why the acl is disallowing the event: is it because I really have no rights, or because the token is expired? So... this brought me back to think about what OAuth is actually doing. It gives you a token that grants you access for a period of time to a certain resource. But the way mosquitto is communicating with the plugin makes it impossible to honour this: The only way to handle an expire is to hang up the connection and let the client reconnect (right?). This is not possible for the auth-plugin. So I think we have to bend the OAuth rules a bit. Let's interpret it like: the token grants you to connect to mqtt for a period of time. After that, you may stay connected for as long as you want. This is actually better for a client to handle. So this is what I suggest: On connect, the GetUser call is done. This checks the jwt with a public key and validates the expire. Then every acl check is only checking the jwt with the public key, but ignores the expire. I think this is reasonable safe, since the signing is protecting the acl checks pretty strongly. It is actually safer, since I can now set the expire very short, so the token is only valid for one connect-action, say 10 secs. This is enough time for the OAuth cycle and mqtt-connect after it.
Mind you: this is actually a problem that occurs for the jwt backend in general: the database variant does a getClaims that fails on expire. I did not fix that because I don't need it. If you use remote, the remote side should also NOT check the expire for acl calls. This should be documented so people can decide for themselves.
Perhaps I am missing something, so I am looking forward to your reply.
Best mous

[pmous]

Hi @iegomez
Ok, I committed the revision, addressing the problem described.
Changed the title.
You asked for a description. Could not find where to put it, so I changed the first comment instead. Is that what you meant?
Best, Paul

[iegomez]

Hi, Paul.

I've been a bit busy, but I'll review everything as soon as I get some time.
Thanks!

[iegomez]

@pmous Hey, Paul, I have some comments on your ideas.

I don't think I fully understand how jwt_aclquery works, could you present a concrete example? In any case, I can see it getting veeery cumbersome to write these rules as an option for the plugin, they would be better suited in e.g. an additional file, just as ACL rules for static files.

Now, regarding the expiration problem you hit, I have a few thoughts:

• It's actually something I missed, so thanks for bringing it up: the backend should check if the token expired unless we explicitly tell it not to do it (think of it like not verifying peer on TLS connections). So I see how introducing an option to bypass expiration would be helpful in the way skipping verification on local environments to circumvent self signed certificates and so on is helpful, but I wouldn't make that the default.
• It's not the plugin's job to handle expiration, after all it's not the one providing the tokens, it's only validating them. It couldn't either, since all it does is check what Mosquitto passes to it. So a JWT backend should be backed by a third service that grants tokens to users by whatever means it wants, shares the same secret as the backend so the plugin may correctly validate them, and offers a way for the client to refresh said tokens.
• The connection between the client and the plugin, and the one between the client and it's JWT token provider, are totally independent and thus I don't see any need for disconnecting the former. If I'm reading it right, you agree on that, but please correct me if not.
• The plugin won't and should not ever tell the client why it's being rejected: invalid token, expired token, denied access to the topic, etc. It's all the same from the plugin's POV, and it's the client's job to refresh a token if needed so. This is very much like not authenticating a user because either its username or password are wrong (and I'm talking in general here, not something specific to the plugin): you should get a rejection but not a reason so you don't leak information.

That's it for now, we can go through your other concerns like the secret's type to adjust for different keys, but please share your thoughts first and we'll go from there. Also, sorry for taking so much time to review and reply, just today I got to check your comments and glance over the code. I know it's far from ideal to have you wait so much, but this is not a light change I can respond to in a few minutes as some issues are, so it's hard to get the time to do it.

Cheers and thanks again!

[pmous]

Hi @iegomez
Thanks for your reply. You should stop apologizing for taking some time to reply ;-). It is amazing that you are maintaining this plugin in the first place. If your work hadn't been done, we would have had a problem!
Now it is my turn to ask for some time to reply. We are a start-up and I am responsible for the whole backend-flow, so right now I am busy writing an admin in php. Perhaps I can find half an hour to write a reply within the next week, otherwise I will be on a short vacation, back on 1 sept.
I am excited to talk more about this project with you.
Best, Paul

[iegomez]

Sounds like a plan. Talk to you soon!

[pmous]

I don't think I fully understand how jwt_aclquery works, could you present a concrete example? In any case, I can see it getting veeery cumbersome to write these rules as an option for the plugin, they would be better suited in e.g. an additional file, just as ACL rules for static files.

About that. For my application the whole thing works with only one rule. So it is not cumbersome at all. I use the regex to support as many schema's as possible. I assume that in most scenarios there is a clear relation between a topic and a user, so that a single rule can cover all users. But maybe I am wrong in that assumption.
I do agree that an additional file would be better, taking one line per rule. This was easier to implement (I am a bit new in golang, my cup of tee is PHP, javascript and such), but I assume this is also not very difficult to do. I would have had to add another option-value, so it would change your code base more than my current idea, though.

An example:
my application uses (simplified):

auth_opt_jwt_aclquery %scope% %access% %topic%:filter-extension|mqtt-extension subscribe|read myndr/preferences/%sub%
auth_opt_jwt_acl_scope_field scope

Which means: auth_opt_jwt_acl_scope_field is the field in the jwt that holds the requested scope. First alle %...% are replaced with requested scope, access and topic, and any field from the jwt, like sub in this example. In my app, sub is actually the user-id. Than the regex on the right side of the : is matched against the string on the left. So in my example, if the scope is either filter-extension or mqtt-extension, access is either subscribe or read, the topic myndr/preferences/<userid> would be accessible. You see how flexible this is? Since I run my own OAuth server, I have total control over what fields are in the jwt, so for me this works like a charm.

[pmous]

Now, regarding the expiration problem you hit, I have a few thoughts:

• It's actually something I missed, so thanks for bringing it up: the backend should check if the token expired unless we explicitly tell it not to do it (think of it like not verifying peer on TLS connections). So I see how introducing an option to bypass expiration would be helpful in the way skipping verification on local environments to circumvent self signed certificates and so on is helpful, but I wouldn't make that the default.

Yes, I can understand that you would say so. But, like I said, the whole idea of checking the expire once the socket is connected, is flawed in the first place. If the client side is listening, the server will simply stop sending updates to the client. There is no way the client can tell that it should renew the token, except if the client itself would keep track of the time, of course. But that is not supported in the lib I use and I really think it is the wrong place to do so anyway. So the question is: do we rely on the safety of a socket, once connected? I think we should. There are lots of applications, like chat for example, that do this: login to open a socket, then continue to use this socket for as long as you like. A socket is of course something else then a stateless website: For a website we need to make sure of the safety every single request.
This problem, that the server stops updates to the listening client, is also a problem in the other backend modes of the plugin: for database or remote checking, the same would happen on an expire. But I didn't fix this, because I don't need it (and thus would not test it). But it should be fixed as well in my opinion. So what is the point making something optional if the first option is flawed? You should explain this clearly, though.

• It's not the plugin's job to handle expiration, after all it's not the one providing the tokens, it's only validating them. It couldn't either, since all it does is check what Mosquitto passes to it. So a JWT backend should be backed by a third service that grants tokens to users by whatever means it wants, shares the same secret as the backend so the plugin may correctly validate them, and offers a way for the client to refresh said tokens.

Exactly my point: Since the plugin structure of Mosquitto is as it is, there is no way around it. I just said that IF Mosquitto would support a callback to force disconnect, that could also solve this problem. In a pretty brute manner, and I agree, very hacky, I just felt I should make clear how limited we are here...

• The connection between the client and the plugin, and the one between the client and it's JWT token provider, are totally independent and thus I don't see any need for disconnecting the former. If I'm reading it right, you agree on that, but please correct me if not.

Yes of course. Like I said in the reply just before, it was just to point out that there is not really a valid other solution in my opinion.

• The plugin won't and should not ever tell the client why it's being rejected: invalid token, expired token, denied access to the topic, etc. It's all the same from the plugin's POV, and it's the client's job to refresh a token if needed so. This is very much like not authenticating a user because either its username or password are wrong (and I'm talking in general here, not something specific to the plugin): you should get a rejection but not a reason so you don't leak information.

Yes, I agree. But if the flow should change, if the rejection should cause another action on the client, I think that we could make an exception in favour of a cleaner flow. But this would not solve the passive listener problem anyway, just the publish. So this discussion is a bit academic.
So the more I think about it, the more I am convinced that the only workable solution is to not check expire for acl checks. I hope you can agree on this now.

[iegomez]

Which means: auth_opt_jwt_acl_scope_field is the field in the jwt that holds the requested scope. First alle %...% are replaced with requested scope, access and topic, and any field from the jwt, like sub in this example. In my app, sub is actually the user-id. Than the regex on the right side of the : is matched against the string on the left. So in my example, if the scope is either filter-extension or mqtt-extension, access is either subscribe or read, the topic myndr/preferences/<userid> would be accessible. You see how flexible this is? Since I run my own OAuth server, I have total control over what fields are in the jwt, so for me this works like a charm.

Thanks for the explanation, now it's a lot clearer! It seems quite a clever and nice way of defining rules, though oddly specific. That is, I'm not sure anyone else would use this way of granting auth because it looks tied to your use case, which in turn hints it could possibly be better suited for the custom plugin backend: you can always change it later according to your domain requirements while keeping up to date with the plugin without needing your own fork. I could be wrong and others may be inclined to use it, but in that case once it's out then there's no adjusting to suit any later specific need you may have that would break compatibility, you're married to the original implementation and only non-breaking enhancements may be added.

About that. For my application the whole thing works with only one rule. So it is not cumbersome at all. I use the regex to support as many schema's as possible. I assume that in most scenarios there is a clear relation between a topic and a user, so that a single rule can cover all users. But maybe I am wrong in that assumption.
I do agree that an additional file would be better, taking one line per rule. This was easier to implement (I am a bit new in golang, my cup of tee is PHP, javascript and such), but I assume this is also not very difficult to do. I would have had to add another option-value, so it would change your code base more than my current idea, though.

Yeah, with one rule it's clean, but I don't think the assumption of a direct relation between topic and user is a given one. Just look at Mosquitto's static files auth where topics may make use of clientid and/or user patterns, but are absolutely not required too. In fact, in typical IoT applications and such, it's very common to have topics such as rooms/1/lights/1, rooms/1/sound/2, etc., only associated to specific users that may access those topics through static rules that don't mention them explicitly. I know, crappy quick example, but I do believe it could get very cumbersome to write rules with no explicit topics to user association.

Thanks also for the discussion on expiration. I'm still inclined to say that expirations should be honored unless explicitly said not to, I don't believe it to be safe to just allow a listener whose token is no longer valid to keep getting messages. Not the best one for sure, but short expirations are used as a way of quickly revoking access to users that have been compromised: you may not be able to instantly kick the user out by invalidating the token, but at least you can deny them a new one and thus mitigate the situation by ensuring they are forbidden access once the current token expires.

So though I understand your reasons, I do think you'd be covered by some jwt_skip_expiration true option to keep your connections open even when tokens expire, while the default behavior would honor said expiration.

Those are my thoughts, now you need to sell me this is a general use case and not just specific to your domain and we're on 😅. I kid but half joking only: adding this means maintenance, documentation, tests, etc., so adding domain specific functionality is something I try to avoid.

Cheers!

[pmous]

Which means: auth_opt_jwt_acl_scope_field is the field in the jwt that holds the requested scope. First alle %...% are replaced with requested scope, access and topic, and any field from the jwt, like sub in this example. In my app, sub is actually the user-id. Than the regex on the right side of the : is matched against the string on the left. So in my example, if the scope is either filter-extension or mqtt-extension, access is either subscribe or read, the topic myndr/preferences/<userid> would be accessible. You see how flexible this is? Since I run my own OAuth server, I have total control over what fields are in the jwt, so for me this works like a charm.

Thanks for the explanation, now it's a lot clearer! It seems quite a clever and nice way of defining rules, though oddly specific. That is, I'm not sure anyone else would use this way of granting auth because it looks tied to your use case, which in turn hints it could possibly be better suited for the custom plugin backend: you can always change it later according to your domain requirements while keeping up to date with the plugin without needing your own fork. I could be wrong and others may be inclined to use it, but in that case once it's out then there's no adjusting to suit any later specific need you may have that would break compatibility, you're married to the original implementation and only non-breaking enhancements may be added.

I understand. So if you are not merging my code, how can I use it without forking? This is not clear to me. Unfortunately the plugin-code is not generic enough to write a completely independent plugin backend, I think. But perhaps I misunderstood...

About that. For my application the whole thing works with only one rule. So it is not cumbersome at all. I use the regex to support as many schema's as possible. I assume that in most scenarios there is a clear relation between a topic and a user, so that a single rule can cover all users. But maybe I am wrong in that assumption.
I do agree that an additional file would be better, taking one line per rule. This was easier to implement (I am a bit new in golang, my cup of tee is PHP, javascript and such), but I assume this is also not very difficult to do. I would have had to add another option-value, so it would change your code base more than my current idea, though.

Yeah, with one rule it's clean, but I don't think the assumption of a direct relation between topic and user is a given one. Just look at Mosquitto's static files auth where topics may make use of clientid and/or user patterns, but are absolutely not required too. In fact, in typical IoT applications and such, it's very common to have topics such as rooms/1/lights/1, rooms/1/sound/2, etc., only associated to specific users that may access those topics through static rules that don't mention them explicitly. I know, crappy quick example, but I do believe it could get very cumbersome to write rules with no explicit topics to user association.

Actually, it is a good example. If it is true that most applications work like that, then I can see my code is indeed not suitable. But I guess that every flat-file solution is cumbersome if there are a lot of users. So, I can imagine that there are:
• home-like applications: a couple of topics and a handful of users. These apps won't likely use jwt at all, just flat file acls?
• general many-user apps with a clear relation of user-id and topic, like mine. My code is perfect for that.
• general many-user apps with many custom topics: This would ask for a database-solution, in my opinion, with an admin of some sort, with roles, etc. My code would not be needed, your jwt with database backend-plugin is fine.
So if this is correct, the question is: are there more people like me ;-). If so, let's hope they let themselves be heard, then we can reconsider. For now, I do agree that it seems best not to merge.

Thanks also for the discussion on expiration. I'm still inclined to say that expirations should be honored unless explicitly said not to, I don't believe it to be safe to just allow a listener whose token is no longer valid to keep getting messages. Not the best one for sure, but short expirations are used as a way of quickly revoking access to users that have been compromised: you may not be able to instantly kick the user out by invalidating the token, but at least you can deny them a new one and thus mitigate the situation by ensuring they are forbidden access once the current token expires.

This is actually a good point: compromised users should be handled. Also in my app this is actually a thing: Our users can have a subscription that is expired and should then be inactive.
However, I think you agree that the flow is flawed? If a client is just subscribed and is listening to a socket, how can he know when to renew its token if he never gets a signal to do so? For this client it is as if there are simply no messages...
Perhaps a timeout client-side, like I suggested?
Perhaps I will use the skip_expire and look into a possibility to "tell" Mosquitto to disconnect a user when needed.. Hmm.

So though I understand your reasons, I do think you'd be covered by some jwt_skip_expiration true option to keep your connections open even when tokens expire, while the default behavior would honor said expiration.

Ok. Sound good. Except you should call it something like jwt_skip_expiration_acl_checks, or something, because the other checks should keep check it! Very important!

Those are my thoughts, now you need to sell me this is a general use case and not just specific to your domain and we're on 😅. I kid but half joking only: adding this means maintenance, documentation, tests, etc., so adding domain specific functionality is something I try to avoid.

I do understand. So like I said: lets just wait if others like the regex-acl checks. But I do hope you want to merge the jwt_skip_expiration_acl_checks-thing. How do we proceed for that? Should I write the code for this option and then do another merge-request without the regex-acl-code? I can also document this option for you. However, testing is for me not an option. I have very limited time, sorry. I don't know how much work it is for you to extend your tests with this one option?

Thanks again, I do enjoy our discussion!

Cheers!

[iegomez]

I understand. So if you are not merging my code, how can I use it without forking? This is not clear to me. Unfortunately the plugin-code is not generic enough to write a completely independent plugin backend, I think. But perhaps I misunderstood...

I believe you're thinking about writing an entirely new backend, but what I meant is that you can use the custom plugin backend: https://github.com/iegomez/mosquitto-go-auth/blob/master/plugin/main.go
This one allows you to fill the methods and receive whatever auth_opts_whatever you want to write your own backend while having all the rest of the plugin's infrastructure available. So you can leverage this option to basically copy the JWT backend with its options and your new ones, strip it out of remote and local logic and just use your code. Once you're done, you don't need to maintain a fork that has my repo as an upstream source since you can always just pull latest changes: I guarantee no conflicts because I won't ever change the basic skeleton of the custom plugin, that's unless of course the interface for backends needs modification, but that's a braking change anyway and would be advertised as so. I promise 😉.

Actually, it is a good example. If it is true that most applications work like that, then I can see my code is indeed not suitable. But I guess that every flat-file solution is cumbersome if there are a lot of users. So, I can imagine that there are:
• home-like applications: a couple of topics and a handful of users. These apps won't likely use jwt at all, just flat file acls?
• general many-user apps with a clear relation of user-id and topic, like mine. My code is perfect for that.
• general many-user apps with many custom topics: This would ask for a database-solution, in my opinion, with an admin of some sort, with roles, etc. My code would not be needed, your jwt with database backend-plugin is fine.
So if this is correct, the question is: are there more people like me ;-). If so, let's hope they let themselves be heard, then we can reconsider. For now, I do agree that it seems best not to merge.

Fair point! I'd still fall in the side of making it a file, even if you ever have one rule: the rule will look the same in the file and the option would now only be a path to that file, keeping mosquitto conf clean and allowing for lots and lots of rules without changing that.

This is actually a good point: compromised users should be handled. Also in my app this is actually a thing: Our users can have a subscription that is expired and should then be inactive.
However, I think you agree that the flow is flawed? If a client is just subscribed and is listening to a socket, how can he > know when to renew its token if he never gets a signal to do so? For this client it is as if there are simply no messages...
Perhaps a timeout client-side, like I suggested?
Perhaps I will use the skip_expire and look into a possibility to "tell" Mosquitto to disconnect a user when needed.. Hmm.

Yeah, i totally agree there's an intrinsic flaw in JWT tokens used as an auth method and expecting the plugin to be able to "handle" expirations properly because it simply can't, but on the other hand you don't want a connection gone rogue to stay alive. So sure, there are cases where letting it live is fine or can't really be handled and this would be a pain point, but that's resolved by having the skip_expiration option (or having very long expiration times if you can control/request that). For the other cases in which your client application connecting to Mosquitto is also very aware of the service providing the tokens and can tell when said service renew the token, it can easily issue a reconnection to Mosquitto with the new token, or simply fail to receive messages because its token actually expired and the service did not grant a new one (e.g., those revokations we were talking about).

Ok. Sound good. Except you should call it something like jwt_skip_expiration_acl_checks, or something, because the other checks should keep check it! Very important!

Absolutely!

I do understand. So like I said: lets just wait if others like the regex-acl checks. But I do hope you want to merge the jwt_skip_expiration_acl_checks-thing. How do we proceed for that? Should I write the code for this option and then do another merge-request without the regex-acl-code? I can also document this option for you. However, testing is for me not an option. I have very limited time, sorry. I don't know how much work it is for you to extend your tests with this one option?

Absolutely yes to adding the skip_expiration option! I'd appreciate a lot if you want to open a new PR adding that option and I can add tests on top of it, but I understand if your time is limited (believe me, I'm on the same boat) and in that case I'll make it the top priority in my TODO list for whenever I get a break.

Finally, sorry if it seemed like I'm inclined to discard the contribution; as said, I find it very interesting and clever, but additions come in with a burden and is hard to justify a very targeted change for a general purpose plugin. I'm not sure how to get some other opinions, maybe asking in Chirpstack's forum or Mosquitto related ones?

Thanks again for a very interesting discussion!

[pmous]

Finally, sorry if it seemed like I'm inclined to discard the contribution; as said, I find it very interesting and clever, but additions come in with a burden and is hard to justify a very targeted change for a general purpose plugin. I'm not sure how to get some other opinions, maybe asking in Chirpstack's forum or Mosquitto related ones?

No problem at all, I think I said already that I understand completely, and actually agree not to merge all my changes.

Ok. I will pick up this task of creating an additional option for skipping the expire. I think I will copy the original code and put a big IF around it, and put my code in de ELSE. That way the diff will be small (in favour of rewriting your original code as well, what would give us less code).
I'll test it for my app.

For the other option: I agree it would be nicer to put the regex-acls it in a file, leaving the option jwt_acl_query like it is, just for databases. Probably calling it jwt_acl_regex_file or something. Done that, I will let you know.

Once I am working on it again, I will look for the best option: forking and merging your updates, using your plugin skeleton, or making the third mode (not remote nor database) on another way generic.

But not right now! I am madly busy! Hope to pick this up in a couple of weeks or so.

Cheers!

[sirockin]

Hi @iegomez, @pmous and @intolerance . I have a similar use case - in my case subscribing directly to topics from an angular app.

I do like the idea of having a common syntax for mapping jwt claims to pub/sub/read topics, so I like the concept behind @pmous' work. If we could find common ground on this I'd be keen to help come up with a spec and implementation.

I will admit though to having trouble understanding @pmous' proposed syntax - I think it would benefit from some case driven unit tests/and or examples - again I'd be happy to collaborate.

[iegomez]

@sirockin Could you elaborate on your Angular subscription needs? I first started this project because I needed to subscribe from a React app that served as a frontend for a Postgres backed application that used JWT tokens for auth.

I was looking yesterday into ways of skipping the expiration time in the jwt-go package, but every time I was reminded that giving a 0 expiration time did the trick, while messing with the token parsing to skip expiration wasn't really desirable. Of course, when you're not the one signing the tokens that doesn't cut it, so I'd be very interested in looking at a potential working solution. It shouldn't be that hard and I could do it myself, but there were some lower hanging fruits I wanted o take care of first.

Anyway, please share any ideas you may have.

Cheers!

[sirockin]

@iegomez thank you once again for responding so quickly to my comments and issues, and also for developing and maintaing such a useful project.

My own use case is fairly simple:

I have a set of users each of whom will use the Angular SPA to access and respond to real-time data on one or more sites defined by an array of IDs in their JWT claim. The data for each site is defined under the topic v/{site_id}/# and the user will have read and write access to a a subset of topics under that ID, all with the same pattern. So for example, a user with claims "sites":["site1", "site2"]should have read/subscription access to "v/site1/alerts/#", "v/site3/alerts/'", and publish access to "v/site1/ack", "v/site3/ack".

So of course I could implement this logic fairly easily by using the remote JWT backend but given that the number of checks which need to be done and the fact that all the necessary information to carry out the auth decision is contained in the claim, I thought it may be useful and possible to eliminate the overhead of the http calls.

On the other hand, I get that any solution like that involves inventing a Domain Specific Language (or embedding an existing one like JavaScript) and this has its own pitfalls.

[sirockin]

Regarding the expiry time issue, if I understand correctly, the issue is that the Mosquitto plugin interface doesn't give us a way to feedback an expiry time or request a disconnection at any point. So our only option is to deny publish requests and read requests once the token has expired, leaving the client in silence but not disconnected - definitely not ideal.

I don't know how flexible/inflexible the Mosquitto plugin interface is, but wouldn't the best thing be to submit a PR to allow a disconnection request to be fed back, say by specifying the expiry time when the user is first validated. I'm really surprised this issue hasn't been raised with them before given the widespread use of JWTs and expiry times.

If that's not possible, I do think the default situation should be to honour the expiry time by refusing further reads/writes/subscriptions.

I'd be happy to look into it some more - are you able to point me to any documentation on the Mosquitto plugin interface because so far I've had difficulty finding anything?

[iegomez]

@sirockin Thanks for sharing your thoughts, it's very rewarding an valuable to get that kind of feedback.

• On your first response: I'm wondering if making the JWT backend available somehow (e.g., having kind of a JWT library that the backend uses but's also generally available) for the custom plugin one, to leverage it and write the needed logic for each use case, would solve the problem without needing to add a separate DSL or embed some scripting language. It looks like it's unnecessarily restrictive right now, forcing you to either write your logic in a separate service and hit it by HTTP requests, or having to write it down in stored procedures to use it from the DBs options when a query is not enough to cover all your cases. Thoughts?

• On your second response: Roger pointed out the interface is changing in the upcoming 2.0 version of Mosquitto (Upcoming changes to Mosquitto plugin interface #96). I haven't gone through the changes and current state of dev, and I don't want to make assumptions on code that may change before release, so I don't know what possibilities it will bring. That's problematic in the sense that doing any work on the current situation might be a wasted effort. If that wasn't the case, yeah, contributing to Mosquitto's plugin interface to properly handle JWT would make a lot of sense.

Those are my thoughts. I'll definitely try to come up with a solution to handle expiration to ease that pain point in the meantime, but will also consider my first point of making it easy to use JWT in a custom way.

Cheers!

[iegomez]

Update: I tried a bunch of strategies for skipping the expiration in a sane way and I hated all of them (and JWT too, but not so sure if it's their fault or the library's). I guess I'm gonna go back to my idea of porting the plugin to a better language some day. 😪

So, at least for now, exposing JWT library for use in the custom plugin is my way to go unless anyone of you comes up with something.

Cheers!

[pmous]

I do like the idea of having a common syntax for mapping jwt claims to pub/sub/read topics, so I like the concept behind @pmous' work. If we could find common ground on this I'd be keen to help come up with a spec and implementation.

I will admit though to having trouble understanding @pmous' proposed syntax - I think it would benefit from some case driven unit tests/and or examples - again I'd be happy to collaborate.

Hi @sirockin, glad to hear my idea is supported! I am happy to give you help in how to use it. I did supply a rather complete explanation at my pull request: #90. Did you find that? Could you explain what part you don't understand, so I can update the explanation. So other people understand it too in the future.

[sirockin]

@iegomez thanks for you latest responses. First, regarding the issue of simplifying JWT claims, I've spent some time thinking about this over the weekend and it occurred to me there are potentially two overlapping objectives:

1. Make it possible for users to implement relatively simple claims logic without implementing a DB or http backend.
2. If/where this is not possible, and a user needs to create a remote backend, reduce the number of http calls made in a given session.

For point 1, I like most of what @pmous has done but would suggest (as he has already has) putting the claims logic in a file and keeping the syntax as close as possible to the existing ACL file syntax - perhaps it already is

For point 2 - cases where the file ACL logic wouldn't be sufficient - (for example in my use case, which requires iterating an array of sites and producing an ACL for each of these) we could reduce the number of http calls made by allowing the return of a complete ACL in the response to the jwt_getuser_uriendpoint and making the jwt_aclqueryendpoint optional. So the flow would be as follows:
a. Call jwt_getuser_uri. If response returns an ACL, use this for the duration of the user session and make no further calls to the backend for that user
b. If not, if the jwt_getacl_uriis set, use that as currently. If it's not set, assume universal access for that user.

[sirockin]

@iogomez on the question of jwt expiry, I think the priorities should be:

1. Implement jwt expiry ASAP by the (imperfect) method of no longer approving read/sub/pub requests once the token has expired
2. Raise an issue with the mosquitto project asking them to provide an expiry mechanism in their plugin interface with the intention to implement it on our side as soon as the interface is provided.
3. For the interim 'skip expiration' option, although this isn't necessary for me - I'll write my client to anticipate the expiry, I can see why others may require it. I'm curious about the difficulties you faced? And what are were the language difficulties that made this worse. (I'm not a partisan go developer and have some frustrations about some of its aspects too, but curious about what in particular you've disliked about working with it in this project).

[pmous]

My own use case is fairly simple:

I have a set of users each of whom will use the Angular SPA to access and respond to real-time data on one or more sites defined by an array of IDs in their JWT claim. The data for each site is defined under the topic v/{site_id}/# and the user will have read and write access to a a subset of topics under that ID, all with the same pattern. So for example, a user with claims "sites":["site1", "site2"]should have read/subscription access to "v/site1/alerts/#", "v/site3/alerts/'", and publish access to "v/site1/ack", "v/site3/ack".

This sounds like a use case exactly where my proposal is for. In my syntax it would be something like:
%topic%:v/%site_id% for the whole,
%access% %topic%:subscribe|read v/%site_id%(/#)?
%access% %topic%:publish v/%site_id%/ack
However, you say that site_id is an array. That is a problem. What does that array look like? Comma separated, or something? If you are in control on how the id's are separated, you might want to use a |. Then it would result in a valid regex, since I do no escaping, if I remember correctly. Otherwise, you can suggest some simple pre-processing before the placeholders are replaced.

[pmous]

Regarding the expiry time issue, if I understand correctly, the issue is that the Mosquitto plugin interface doesn't give us a way to feedback an expiry time or request a disconnection at any point. So our only option is to deny publish requests and read requests once the token has expired, leaving the client in silence but not disconnected - definitely not ideal.

This is exactly my point.

I don't know how flexible/inflexible the Mosquitto plugin interface is, but wouldn't the best thing be to submit a PR to allow a disconnection request to be fed back, say by specifying the expiry time when the user is first validated. I'm really surprised this issue hasn't been raised with them before given the widespread use of JWTs and expiry times.

That would indeed be the cleanest solution.

If that's not possible, I do think the default situation should be to honour the expiry time by refusing further reads/writes/subscriptions.

Right now, the default is to honour the expiry. And that results in a listening client to just be ignored. New publishes or subscribes would not be granted, of course, so those are no problem. But listening is on an existing socket, so there is no way the client can be informed other than simply disconnect the socket. Which would, again, be the best solution.

[sirockin]

Hi @pmous thanks for the further feedback. I'm going to try to find time to elaborate on the ACL syntax proposal in the next couple of days. In the mean time, I'll raise a new issue here and on the mosquitto repo, regarding JWT expiry.

[sirockin]

Regarding your syntax I think I understand it now having reread your example and explanation. So from your example above:

auth_opt_jwt_aclquery %scope% %access% %topic%:filter-extension|mqtt-extension subscribe|read myndr/preferences/%sub%
auth_opt_jwt_acl_scope_field scope

A claim such as { "sub":"mike", "scope":"mqtt-extension, ... }" would have read and subscribe access to topic myndr/preferences/mike(since the scope value here (mqtt-extension) matches filter-extension|mqtt-extension

However a claim such as { "sub":"mike", "scope":"another-scope", ... } would have no access to anything?

It's a nice terse syntax but I do agree with @iegomez that it seems quite specific to your use case. It also may not be easy to understand or to explain - it took me a few reads and I can't think of a better way of explaining it than you have.

In your second example, applied to the single-site version of my use case you've allowed the scope_idclaim to be substituted in - which I like as a concept. If the claims can be substituted in directly, presumably we don't need the àuth_opt_jwt_acl_scope_field`parameter?

%topic%:v/%site_id% for the whole,
%access% %topic%:subscribe|read v/%site_id%(/#)?
%access% %topic%:publish v/%site_id%/ack

As I said in an earlier post, I would prefer the logic to be closer to the existing ACL syntax so how about extending the user syntax in the ACL file to allow jwtclaim.{claim_name} and allowing general substitution of claims into paths:

jwtclaim.scope mqtt-extension
topic read myndr/preferences/%jwtclaim.sub%

jwtclaim.scope filter-extension
topic read myndr/preferences/%jwtclaim.sub%

or even

jwtclaim.scope mqtt-extension, filter-extension
topic read myndr/preferences/%jwtclaim.sub%

For my (single-site) use case the file would read

topic read v/%jwtclaim.site_id%/#
topic write v/%jwtclaim.site_id%/ack

I'll be interested to hear your thoughts!

[pmous]

Regarding your syntax I think I understand it now having reread your example and explanation. So from your example above:

auth_opt_jwt_aclquery %scope% %access% %topic%:filter-extension|mqtt-extension subscribe|read myndr/preferences/%sub%
auth_opt_jwt_acl_scope_field scope

A claim such as { "sub":"mike", "scope":"mqtt-extension, ... }" would have read and subscribe access to topic myndr/preferences/mike(since the scope value here (mqtt-extension) matches filter-extension|mqtt-extension

However a claim such as { "sub":"mike", "scope":"another-scope", ... } would have no access to anything?

Correct!

It's a nice terse syntax but I do agree with @iegomez that it seems quite specific to your use case. It also may not be easy to understand or to explain - it took me a few reads and I can't think of a better way of explaining it than you have.

In your second example, applied to the single-site version of my use case you've allowed the scope_idclaim to be substituted in - which I like as a concept. If the claims can be substituted in directly, presumably we don't need the àuth_opt_jwt_acl_scope_field`parameter?

In saying "you've allowed the scope_id claim ...", I think you look at this a bit too much as fields. The fact that I separated the left string with spaces is arbitrary. Think of it as left side of : is a string, right side is a regex. Super simple. So you decide what you like to match against what. So if you need to check the scope, just put it in.
However, since scope needs some processing, it is an array with or-logic, we can't just substitute a field for it (like your site-id). So we need to know which field holds the scope. That is where àuth_opt_jwt_acl_scope_field is for. If you don't check the scope, you don't need that option at all.

Once you understand just how generic it is (arbitrary string matched to regex), you will see that it is not specific to my use case. Like you see in your example, your use case is also simple. Except for the site_id. But that problem is also not solved in your proposal, as far as I can see?

%topic%:v/%site_id% for the whole,
%access% %topic%:subscribe|read v/%site_id%(/#)?
%access% %topic%:publish v/%site_id%/ack

As I said in an earlier post, I would prefer the logic to be closer to the existing ACL syntax so how about extending the user syntax in the ACL file to allow jwtclaim.{claim_name} and allowing general substitution of claims into paths:

jwtclaim.scope mqtt-extension
topic read myndr/preferences/%jwtclaim.sub%

jwtclaim.scope filter-extension
topic read myndr/preferences/%jwtclaim.sub%

or even

jwtclaim.scope mqtt-extension, filter-extension
topic read myndr/preferences/%jwtclaim.sub%

For my (single-site) use case the file would read

topic read v/%jwtclaim.site_id%/#
topic write v/%jwtclaim.site_id%/ack

I'll be interested to hear your thoughts!

I like the thought a lot. My syntax is grown simply from the easiest path to success. Yours is better for the community, I think. However, much much more work! You need to plug into the ACL parse-code (didn't even look at that yet) and write code for several parts of the plugin. Like I suggested earlier: First you need to support placeholders into the ACL syntax (or does that work already??), then you need to write some extension code for jwt-claims. But isn't the ACL syntax specific for the file-backend, I wonder? It is only used there. The database and jwt backends use their own logic... So I wonder if my though is true: first support overal, then expand for jwt.

I think we don't need regex per se, we just might support delimiter-separated multiple values, like you suggest for "jwtclaim.scope mqtt-extension, filter-extension".
But what about array values? Like I said, you conveniently skip over the fact that your example "topic read v/%jwtclaim.site_id%/#" would not work, since the value is json.

We will get there! Nice work so far.

[sirockin]

@pmous thanks for the very positive feedback.

I do agree that this would be harder from an implementation point of view, but hopefuly with judicious unit testing and refactoring, we can resuse a large part of the file-based ACL logic.

Regarding my array use case, I think it's just too specific for this type of approach - the only possible way I could think of incorporating it would be to allow some kind of 'for each' statement in the ACL and I think that would be a departure too far from the core concepts used in this project.

[pmous]

Regarding my array use case, I think it's just too specific for this type of approach - the only possible way I could think of incorporating it would be to allow some kind of 'for each' statement in the ACL and I think that would be a departure too far from the core concepts used in this project.

Well, perhaps not. I think some sort of multi-value claim is not so strange at all. For example, the scope is usually an array.
I was thinking to do some pre-processing before the placeholder are replaced, then together with regexes. But to do the whole ACL with regex is perhaps tricky, for backward compatibility and all. So perhaps we could use regexes when an ACL has placeholders and else fall back. Anyway.
For example something like: topic write v/%listToOr:jwtclaim.site_id%/ack, where site_id is comma separated (id,id,id). The pre-processor listToOr might then replace comma with pipe and put () around it. In general we could make it so that before the : could be any kind of pre-defined decoding, perhaps with extra params too. For example: %jsonField:jwtclaim.data,my_field%, where my_field is the field taken from a json. JsonField might be magical, that if a field result is array, a regex-or is created. So {"my_field":"value"} would give value and {"my_field":["value1", "value2"]} would give (value1|value2).Or perhaps field could rather be a path? With dot-notation? Well, if we would choose such a syntax, the beauty is that anyone can add a suitable decoder later. So we don't have to implement all this at once.

[iegomez]

Hey guys, I don't know what stupidity I was doing the other day, but hereś a PR for skipping expiration: #109.
I'll merge it right away and do a new release.

Now don't mind me, carry on.

[pmous]

Hey guys, I don't know what stupidity I was doing the other day, but hereś a PR for skipping expiration: #109.
I'll merge it right away and do a new release.

Now don't mind me, carry on.

@iegomez @sirockin Good thing you found that. But, if I understand correctly, this option disables the check all together. That is really not the way to go: you want to keep using expire for user-check (connect/publish), since they don't suffer from the problems that the acl-check does.

If I remember correctly, my proposal only disables it when needed: in checkAclLocal() I don't use getClaims(), but use &jwt_go.Parser{
SkipClaimsValidation: true,
}
This we could of course make optional, that SkipClaimsValidation property, but anyway. Other parts of the code still use getClaims() and still use expire.

Anyway, for me it is not really a problem right now. The local-jwt code is for me most important. The expire-problem is perhaps best solved in the client for now. If a better API to Mosquitto becomes available, as we discussed, then perhaps that method would be cleaner and more robust in the future. As long as local-jwt is not formalized and merged, I keep using my fork for the time being. Meanwhile we keep looking for a common solution, of course!

[sirockin]

@pmous - I'm glad we're thinking along similar lines. It would definitely be interesting to discuss extending the ACL syntax for an array but I agree lets keep the scope of this PR to the same as your original one. I may find some time to work on that. If I do, ould you be happy for me to commit to this PR or would you prefer I open a new one?

@iegomez thanks for your replies (and also for dealing with #107 so quickly). What are your thoughts about extending the ACL syntax as @pmous and I have discussed. Do you think you would accept a PR along those lines?

[pmous]

@pmous - I'm glad we're thinking along similar lines. It would definitely be interesting to discuss extending the ACL syntax for an array but I agree lets keep the scope of this PR to the same as your original one. I may find some time to work on that. If I do, ould you be happy for me to commit to this PR or would you prefer I open a new one?

I'm not very experienced in sharing code this way, but I guess that depends on whether you would go from the master repo or fork from mine. Isn't it? As long as I am kept in the loop, I don't mind if you fork your own.

Problem with my code is that the two issues, ACL-extend and expire-issue, are both in there... So perhaps quickest is to just fork from master, then manually diff with my version, if you like. But really that is up to you of course. I think, if I resume working on this, I would actually do the same :-)

[iegomez]

@pmous I could pass the option as an argument to getClaims and control where to effectively skip expiration, that'd be easy enough. I definitely wouldn't skip validating claims though, they may fail in multiple ways.

@sirockin I said earlier in this convo that I'm not sold and I really don't wanna have to test and maintain custom syntax that I might even not fully understand. I get what @pmous says about not everyone being able to fill the custom plugin blanks, but on the other hand I can't really add every way of checking auth for whatever use cases people may come up with. Honestly, though I said I wasn't sure about embedding stuff, I'd prefer throwing a Javascript interpreter in there to lower the barrier and call it a day.

That said, I haven't been following the discussion, too busy this week, so maybe you 2 come up with some beautiful solution and then I'll be happy to add it.

[sirockin]

@iegomez:
interesting you mention the javascript interpreter - if something safe and lightweight could be found I think it would make an excellent alternative to the http and jwt remote endpoints, remove the need for external http calls

I do though think there is still some mileage in allowing some jwt token substitutions into a file-based ACL. Maybe I'll expand on this in a new issue and we can discuss there.

@pmous regarding the PR, agreed. I'll raise an issue first though so that @iegomez can respond

[iegomez]

BTW, you might be interested in the notes about Mosquitto's v2.0 release: https://projects.eclipse.org/projects/iot.mosquitto/releases/2.0

Also, check the develop branch plugin dir for more details: https://github.com/eclipse/mosquitto/tree/develop/plugins

[iegomez]

@pmous I just merged #112 to skip expiration per user or acl check, with options for each one. Will release in a minute.

[iegomez]

OK, I finally read the whole discussion (and the related issues) and it's a lot more clear now, sorry for taking so long and keeping you hanging.

As I mentioned in #113, this #116 PR will make it much easier and cleaner to add this feature. Of course, it's Friday and we all have different timezones, but would you be willing to have a call next week to talk about it?
I'll try to go over it again and have a yet clearer understanding, maybe propose an alternative syntax, maybe not, who knows? I'm just playing there really, it'd be lovely to really get to discuss it in the moment and see how we implement it, because I'm now sold on the idea.

Let me know and we'll schedule a call to get it done.

Cheers, guys, an thanks for your interest!

[pmous]

OK, I finally read the whole discussion (and the related issues) and it's a lot more clear now, sorry for taking so long and keeping you hanging.

No problem at all! We will get there!

As I mentioned in #113, this #116 PR will make it much easier and cleaner to add this feature. Of course, it's Friday and we all have different timezones, but would you be willing to have a call next week to talk about it?

I'm sorry, not quite clear who you want to talk to, me, or sirocking, or both? And what about exactly?

I'll try to go over it again and have a yet clearer understanding, maybe propose an alternative syntax, maybe not, who knows? I'm just playing there really, it'd be lovely to really get to discuss it in the moment and see how we implement it, because I'm now sold on the idea.

That is great news indeed! But again not quite sure what exactly, since this PR is about a couple of things. I assume you mean you now want so support an extended ACL?

Let me know and we'll schedule a call to get it done.

Cheers, guys, an thanks for your interest!

[iegomez]

@pmous Yup, I was referring to your extended ACL suggested addition, but let's just keep it async and I'll try to start implementing it next week, then you guys can tell me what's wrong or missing along the way.

[pmous]

@iegomez That's fine. Fabulous that you want to spend time in this!
So you start off from @sirockin 's ACL-like syntax, I guess? Mine was perhaps a bit too technical. For me the shortest road to success... I like his idea more.

[iegomez]

@pmous @sirockin It's been a while, some other issues came up, work, vacations, etc., so sorry for leaving this discussion hanging.
That work I told you about refactoring the JWT backend and making it easier to add checkers for it has been merged for some time now, meaning adding a files-like option or any syntax we agree upon should be a lot easier. I also squashed some long standing issues, refactored some other stuff and did some cleaning up.

Are you guys still interested in adding it? If so, a proper definition of the format to follow should be enough to implement it rather sooner than later this time. Let me know what's your opinion on this.

Cheers!

[pmous]

@pmous @sirockin It's been a while, some other issues came up, work, vacations, etc., so sorry for leaving this discussion hanging.
That work I told you about refactoring the JWT backend and making it easier to add checkers for it has been merged for some time now, meaning adding a files-like option or any syntax we agree upon should be a lot easier. I also squashed some long standing issues, refactored some other stuff and did some cleaning up.

Are you guys still interested in adding it? If so, a proper definition of the format to follow should be enough to implement it rather sooner than later this time. Let me know what's your opinion on this.

Cheers!

Hi guys, it has been a while for me too. Sorry about that. Lots to do, being the only developer at a startup (besides my bos, who has no time to develop). But he, not complaining, still love my job :-)
Anyway. I hope to pick up this whole OAuth thing soon. What I suggest, is that I will then pull the latest changes, look then what is still needed, consolidate that with @sirockin, and if needed write some code. I'm afraid that is all I can give promise now.
I am still very pleased to see that you are actively maintaining/developing this plugin, @iegomez.

Cheers!

[iegomez]

As with the original issue, I'm closing this for now to avoid noise, but feel free to open it again if needed.

[pmous]

Hi @iegomez and @sirockin
I have been very busy with other stuff here, it's like I said, we have an extremely small team. Besides that, we have been evaluating the use of Mosquitto for our use lately. For now it is ok, but in the future we'll have to expand and it seems Mosquitto is not the best choice for that. Further more, the requirements also changed a bit, so it is less difficult to find a jwt-authorization implementation.
So unfortunately I will not spend time in the development of the jwt-auth plugin anymore. I enjoyed our talks and I hope that my limited efforts have helped you guys in some way! I wish you the best and perhaps we'll cross paths again.

[iegomez]

Thanks for all, @pmous, I enjoyed the talks as well. Best of lucks!

Cheers!

[iegomez]

Btw, @pmous, I'm curious at what scale you'll be operating where Mosquitto (or is it the plugin the bottleneck?) is no longer viable. Was it the plugin, have you looked into Mosquitto's integrated auth since 2.0? Or is it not about throughput from the broker but due to other requirements?

[pmous]

Btw, @pmous, I'm curious at what scale you'll be operating where Mosquitto (or is it the plugin the bottleneck?) is no longer viable. Was it the plugin, have you looked into Mosquitto's integrated auth since 2.0? Or is it not about throughput from the broker but due to other requirements?

@iegomez Well, the plugin is not the problem. The situation is that for now Mosquitto is doing fine, but we are a startup that aims for world domination :-). The product relies for 100% on the correct working of mqtt. If our product grows, then hundreds and later hopefully thousands connections will be made. Mosquitto has pour support for clustering. And we are not sure that Mosquitto is reliable enough in general. Of course we are not addressing this growth right now, but it makes no sense to spend too much time in a product we know is not going to be sufficient.
Besides that, we changed the authorization implementation a bit: mqtt can now have read access publicly and writing is done just by our server. The user based authrization is now handled by the application itself. So you may alway read the data, but the handling is reserved. This way we don't need that complicated jwt-based authorization anymore. Would be a problem for other mqtt brokers as well: It appears it is just not very common to do dynamic authorization based on the username. So... rewriting the plugin is not needed after all, even if we stick to Mosquitto!

[iegomez]

Gotcha! Thanks for sharing. 🙇‍♂️