Examplebroker without dbus policy

[denisonbarbosa]

The current implementation of the examplebroker dbus service requires that we configure it in the system in order for it to work.
These changes should improve the current behavior by making the cmd/main func create its own bus and then exporting the broker on that bus instead.

Also, some quality of life improvements were made in order to properly clean up the system after the tests are done.

UDENG-1159

[GabrielNagy]

Had some comments, let me know what you think

[didrocks]

I think we need to create a new Jira card to fix this branch behaviour. I see a lot of issues with it:

1. this branch will always starts a local system bus and create /etc/authd/brokers.d, and will clean them up on shutdown. It means:

• we always run a local system bus and never connect to the real one (having the dbus broker env variable or not)
• we are removing the system configuration we are developping on after exiting as /etc/authd/brokers.d is unconditionnally removed (what about a real system with real brokers?).
• we are forced to always run the service as root, even for testing (we can have a --socket option to write the socket somewhere else, and thus, not having the requirement to run as root)

So, basically, if you run this binary right now on a production system even just for testing, you are breaking it (also, the example broker is not available outside of the env variable)

2. If we don’t run the daemon as root, here is what happens:

 didrocks@casanier:~/work/authd$ ./authd
Mock system bus started on unix:path=/tmp/authd-system-bus-mock4169769742/bus.sock
didrocks@casanier:~/work/authd$

No error, immediately exits.

3. you are importing testutils in the main package, so we inherit from every imported dependencies like testify, and thus testing, for instance.

4. the permissions on brokers.d is incorrect. As with any configuration files which are not sensitive, it should be world-readable.

I think this should be revisit with this goal in mind:

• running the binary should not impact the system in a permanent way: it should not impact the configuration directory for instance.
• ensure we can in the long term not running it as root if we only want the example broker (this will need another change for the socket directory)
• ensuring that we never exit silently without printing an error (non root execution return exit code 1, but no error is printed)
• not importing the testutils directory with a normal build. If you still want to avoid code duplication, I would suggest only spawing this dbus with a special build tag then instead of an environment variable. For instance, we can imagine using with-example-broker build tag to include this "please run a special dbus, and then, it means we are going to discare any other brokers as they are not going to listen on the same bus".

Does it make sense?

[denisonbarbosa]

Well, this definitely was a misunderstanding then. I expected this code only to be used during development (and while we still don't have integration tests), as we were always running the examplebroker no matter what and that we would clean the cmd package before deploying it (that's why I developed it this way with no regard to the actual production code in mind, similar to what you did with the main func in pam.go, so we wouldn't need to always compile and update the module).

[didrocks]

Sorry, there is probably something I’m missing then, how do you run the production code right now? The one not spawning this additional dbus daemon, loading system brokers and keeping not cleaning them up? It seems there is only one binary to me here, no?

[denisonbarbosa]

Sorry, there is probably something I’m missing then, how do you run the production code right now? The one not spawning this additional dbus daemon, loading system brokers and keeping not cleaning them up? It seems there is only one binary to me here, no?

Yeah, this is the only binary. I expected this change to be temporary (during this early stage of development) and that's why I did it like this. I didn't know we wanted to keep the option to run the examplebroker even after release, so now I can definitely update this with all the flow in mind (not removing the configuration directory, not importing testutils outside a test environment, and so on...).

[didrocks]

Yeah, I think we are at a stage of development now where we should not introduce technical debts (temporary things that we will remove), since transitioned the PAM module to something more robust and definitive.

I think the example broker will always be there (just not compiled) in the binary for our own manual testing without breaking any user’s machine. The fact in the example broker case to only load it (+local ofc), is not an issue, as this is really to be able to mainly test the rest of the logic manually (pam module, database, soon hopefully gdm and other in session cases…)

I’m not on the mock broker use case, but this one should have a similar logic to run on its own bus, right? Maybe we should have both mechanism in common, wdyt? (a way to run everything with mock for tests and its own bus, independent from machine’s config seems really similar to using the example broker, with its own bus, independent from the machine’s config).

[denisonbarbosa]

Yeah, I think we are at a stage of development now where we should not introduce technical debts (temporary things that we will remove), since transitioned the PAM module to something more robust and definitive.

I think the example broker will always be there (just not compiled) in the binary for our own manual testing without breaking any user’s machine. The fact in the example broker case to only load it (+local ofc), is not an issue, as this is really to be able to mainly test the rest of the logic manually (pam module, database, soon hopefully gdm and other in session cases…)

I’m not on the mock broker use case, but this one should have a similar logic to run on its own bus, right? Maybe we should have both mechanism in common, wdyt? (a way to run everything with mock for tests and its own bus, independent from machine’s config seems really similar to using the example broker, with its own bus, independent from the machine’s config).

I'll wait for #22 to be merged, as it changes some things in the daemon that will be very useful for us here. Then I'll get started on this right away.