Fix example broker D-Bus XML file

[adombeck]

"a{s}s" is not a valid GVariant type, it should be "a{ss}" instead.

[3v1n0]

Oh... Indeed!

[michaelwildvarian]

Only thing that worries me a bit is that it also shows up in the Entra ID broker: https://github.com/ubuntu/authd-oidc-brokers/blob/2c443ba1791f326e70e0d5d042ca7aa5d13017ce/internal/dbusservice/dbusservice.go#L29.

[3v1n0]

I've the feeling that dbus variant parsing isn't doing things properly?

But we're definitely wrong there too, in fact:

$ gdbus introspect --system --dest com.ubuntu.authd.MSEntraID --object-path /com/ubuntu/authd/MSEntraID 
node /com/ubuntu/authd/MSEntraID {
  interface com.ubuntu.authd.Broker {
    methods:
      NewSession(in  s username,
                 in  s lang,
                 in  s mode,
                 out s sessionID,
                 out s encryptionKey);
      GetAuthenticationModes(in  s sessionID,
                             in  a{s}s supportedUILayouts,
                             out a{s}s authenticationModes);
      SelectAuthenticationMode(in  s sessionID,
                               in  s authenticationModeName,
                               out a{s}s uiLayoutInfo);
      IsAuthenticated(in  s sessionID,
                      in  s authenticationData,
                      out s access,
                      out s data);
      EndSession(in  s sessionID);
      CancelIsAuthenticated(in  s sessionID);
      UserPreCheck(in  s username);
    signals:
    properties:
  };
  interface org.freedesktop.DBus.Introspectable {
    methods:
      Introspect(out s out);
    signals:
    properties:
  };
};

Now, this doesn't seem to happen in the example broker we've in tests:

$ gdbus introspect --system --dest com.ubuntu.authd.ExampleBroker --object-path /com/ubuntu/authd/ExampleBroker 
node /com/ubuntu/authd/ExampleBroker {
  interface org.freedesktop.DBus.Introspectable {
    methods:
      Introspect(out s out);
    signals:
    properties:
  };
  interface com.ubuntu.authd.Broker {
    methods:
      CancelIsAuthenticated(in  s arg_0);
      EndSession(in  s arg_0);
      GetAuthenticationModes(in  s arg_0,
                             in  aa{ss} arg_1,
                             out aa{ss} arg_2);
      IsAuthenticated(in  s arg_0,
                      in  s arg_1,
                      out s arg_2,
                      out s arg_3);
      NewSession(in  s arg_0,
                 in  s arg_1,
                 in  s arg_2,
                 out s arg_3,
                 out s arg_4);
      SelectAuthenticationMode(in  s arg_0,
                               in  s arg_1,
                               out a{ss} arg_2);
      UserPreCheck(in  s arg_0,
                   out s arg_1);
    signals:
    properties:
  };
};

And they both work fine with the same server, so I've the feeling that in some way the go dbus implementation handles this in the same way... mhmh

[3v1n0]

@adombeck FYI the racy failure that you got in the (coverage) job is fixed by #616 (I was imagining it to happen again! :))

e.g. https://github.com/ubuntu/authd/actions/runs/11708304677/job/32609750206?pr=619

[michaelwildvarian]

The .NET implementation of https://github.com/tmds/Tmds.DBus at least fails with a{s}s. And when using busctl monitor to listen in on the chatter, I actually get a{ss} on the "wire".

[adombeck]

Only thing that worries me a bit is that it also shows up in the Entra ID broker: https://github.com/ubuntu/authd-oidc-brokers/blob/2c443ba1791f326e70e0d5d042ca7aa5d13017ce/internal/dbusservice/dbusservice.go#L29.

I opened ubuntu/authd-oidc-brokers#193 to fix that as well

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.98%. Comparing base (c91dd5b) to head (d020006).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #619      +/-   ##
==========================================
+ Coverage   82.93%   82.98%   +0.04%     
==========================================
  Files          80       80              
  Lines        8534     8534              
  Branches       75       75              
==========================================
+ Hits         7078     7082       +4     
+ Misses       1127     1124       -3     
+ Partials      329      328       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[michaelwildvarian]

Thanks for the prompt resolution on both issues!