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

Allow per-device forward url override. #4630

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

osery
Copy link

@osery osery commented Feb 4, 2021

This is to allow overriding target urls for forwarding position information and events per device or per device group.

Our use case is 1 Traccar server that needs to forward events to one of N target servers depending on a device group.

@tananaev, please let me know if this is something useful for the core Traccar and any changes needed to get it in ✌️.

@tananaev
Copy link
Member

tananaev commented Feb 4, 2021

I'm not sure if it's going to be useful for a wider audience. There is also a bit of security concern. This means that people can "disable" global forwarding by using incorrect URL or their own URL. But maybe let's keep the PR open and see if anyone else is interested in similar functionality.

@jcardus
Copy link
Contributor

jcardus commented Feb 5, 2021

I have the same need and implemented also with device attributes being checked on the code running behind the url being invoked by traccar.

@lukazi
Copy link

lukazi commented Feb 5, 2021

I have the same need. People can overide global forwarding for owned devices only. Global forwarding could run in parallel if You wory about that.

@tananaev
Copy link
Member

tananaev commented Feb 5, 2021

Maybe if the global URL is not set, we can use per device. If it is set, we use that..

@jcardus
Copy link
Contributor

jcardus commented Feb 5, 2021 via email

@osery
Copy link
Author

osery commented Feb 5, 2021

Good point with the possible security issue 🤔.
Let me change the PR as suggested, probably tomorrow 👍.

@osery
Copy link
Author

osery commented Feb 6, 2021

@tananaev so I have changed the behavior as suggested above, i.e., giving precedence to the global forward.url config option when set.

To make that happen, I have added forward.enable and event.forward.enable config options defaulting to true. The motivation is to allow disabling the forwarding overhead when not needed (which now cannot be done solely by the emptiness of the global urls), but to keep this from breaking older config files (missing value would not disable(=break) the forwarding on upgrade).

PTAL and let me know if this is something worth merging and what changes would have to be done.

An alternative is to keep the original behavior and add a config option to allow per-device overrides, but that does not easily cover the "forward only for a subset of devices" scenario described by @jcardus above 🤷.

@tananaev
Copy link
Member

tananaev commented Feb 6, 2021

We just removed those keys a couple of weeks ago. Please don't add them back. Just use URL.

@osery
Copy link
Author

osery commented Feb 6, 2021

Not sure how tbh. If it is filled in, it will have precedence over the per-device settings. If it is not, the components WebDataHandler and EventForwarder don't get even initialized. Do you mean to always have these initialized then? I can surely do that, though it will then mean unnecessary overhead for each position processing. Is that ok? Happy to change if so. Let me know 👍.

@tananaev
Copy link
Member

tananaev commented Feb 6, 2021

I see what you mean. Let's leave it for now. I need to think about it. We are actually in the process of changing device/user configuration.

@osery
Copy link
Author

osery commented Feb 24, 2021

Hi @tananaev, any thoughts on this one?

@tananaev
Copy link
Member

Haven't had time to work on configuration yet. Swarmed with other work.

@jpmens
Copy link
Contributor

jpmens commented Jun 22, 2021

I would like to see this implemented, and I like the idea of the global forward.url config taking precedence.

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.

None yet

5 participants