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

Initial implementation for the ExampleBroker bus and the dbus communication with the daemon #3

Merged
merged 5 commits into from
Aug 1, 2023

Conversation

denisonbarbosa
Copy link
Member

@denisonbarbosa denisonbarbosa commented Jul 24, 2023

Up until now, we only had the example broker running locally since we didn't have the dbus integration with the daemon. This PR implements the dbus calls and the ExampleBroker bus that will handle them.

UDENG-1064

Comment on lines 20 to 21
<method name="SelectAuthenticationMode">
<arg type="s" direction="in" name="requestID"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An other option here, instead of using a request ID would be to instead make this method to return a /com/ubuntu/auth/ExampleBroker/AuthMode/$ID/ object and that then provides the IsAuthorized() and CancelRequest methods.

So basically so you separate the BrokerManager from the BrokerAuthenticationObject.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @3v1n0. thanks for the suggestion! This interface definition was before we decided on how to handle the canceling part of the authentication process. Now that #4 is merged, only the sessionID is enough for us to know which request should be canceled.
However, it's still a very nice suggestion and we will probably draft something around it to see how it would look. Thanks!

@denisonbarbosa denisonbarbosa force-pushed the examplebroker-bus branch 2 times, most recently from 3bfb4e2 to d206208 Compare July 28, 2023 11:31
@denisonbarbosa denisonbarbosa marked this pull request as ready for review July 28, 2023 11:32
@denisonbarbosa denisonbarbosa requested a review from a team as a code owner July 28, 2023 11:32
cmd/authd/main.go Outdated Show resolved Hide resolved
@didrocks
Copy link
Member

Hum, didn’t we decided to do the cleanup before this PR so that the diff is easier to navigate? Things like supportedUiLayouts -> supportedUILayouts, and so on?

@denisonbarbosa
Copy link
Member Author

Hum, didn’t we decided to do the cleanup before this PR so that the diff is easier to navigate? Things like supportedUiLayouts -> supportedUILayouts, and so on?

I thought the cleanup would come after this PR, since this one was already opened (and since you've mentioned that you wanted to show the dbus stuff to Okta on Monday), but I can do the housekeeping first if you prefer.

@didrocks
Copy link
Member

Hum, didn’t we decided to do the cleanup before this PR so that the diff is easier to navigate? Things like supportedUiLayouts -> supportedUILayouts, and so on?

I thought the cleanup would come after this PR, since this one was already opened (and since you've mentioned that you wanted to show the dbus stuff to Okta on Monday), but I can do the housekeeping first if you prefer.

I stated the opposite. I think having this PR smaller will really help!

@denisonbarbosa denisonbarbosa force-pushed the examplebroker-bus branch 2 times, most recently from 9df0f6a to 713f8ed Compare July 28, 2023 15:17
@denisonbarbosa denisonbarbosa requested review from a team and removed request for a team July 31, 2023 13:48
Copy link
Member

@didrocks didrocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor change + 2 food for thoughts and I we can get that in. Once you have the other PR in, think about the API change.

Excellent work otherwise!

internal/brokers/examplebroker/ExampleBroker Show resolved Hide resolved
internal/brokers/examplebroker/configurebroker.sh Outdated Show resolved Hide resolved
The dbusbroker represents what the daemon knows about a dbus broker. It
is responsible for creating and making the dbus calls to the right
interface and method, handle the response and return the desired values
back to our daemon
This is the bus that will receive the calls for the ExampleBroker, call
the apropriate broker function to handle them and then send the
requested values through the bus again back to the dbusbroker caller.
- ExampleBroker: configuration file that will be parsed by authd
  to register the broker;

- .conf: bus configuration file that specifies access policies for the
  bus;

- .xml: defines the bus interface and its methods;

- .sh: copies the files to the right places to make our life easier;
Now examplebroker is an internal package with all of its config files
and the dbus mapping
@denisonbarbosa denisonbarbosa merged commit b3ec083 into main Aug 1, 2023
2 of 3 checks passed
@denisonbarbosa denisonbarbosa deleted the examplebroker-bus branch August 1, 2023 14:29
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

3 participants