Add service module to write PolKit agents #302

Cu3PO42 · 2025-10-11T19:37:16Z

This implements a new service module that enables users to write PolKit agents. To this end, I'm integrating with the existing polkit-qt-1 library that is also used by the KDE agent.

A relatively minimal example configuration using this new service can be found here.

There's a minor TODO left around cleanup of the Identity objects that I haven't been able to get quite right due to my inexperience with Qt. I'm hoping you might have a clean solution for this.

Closes #271.

outfoxxed · 2025-10-12T00:45:56Z

While I am not opposed to the addition of a polkit integration, I would rather avoid all KDE dependencies as including them leads to massive dependency trees on most distros, and they're usually pretty replaceable.

polkit-qt-1 appears to be a very thin wrapper around the polkit api that doesn't provide much. I would prefer to avoid it unless you have a good reason to pull it in.

Cu3PO42 · 2025-10-12T08:56:14Z

polkit-qt-1 is indeed a very thin wrapper. The 'good' reason for using it is that it deals with some of the more error-prone aspects of the upstream libraries and doesn't really bring anything that we don't need. While it is maintained by the KDE team, the package doesn't actually pull in anything but Polkit itself and Qt on the distros I checked (Debian, Ubuntu, Arch, Fedora, RHEL, NixOS, or Gentoo). I find it unlikely that others would have wildly different dependencies.

That said, it should also be fairly easy to not use it at the cost of a few hundred LOC, which would have the benefit of me getting to correct one of the limitations of polkit-qt-1.

outfoxxed · 2025-10-12T10:21:05Z

It sounds like one of the more reasonable ones if we had to use it then, though I would personally still prefer not to.

Cu3PO42 · 2025-10-12T10:51:48Z

Alright. I'll see what I can do.

Cu3PO42 · 2025-10-12T21:39:36Z

I removed the dependency on polkit-qt, interacting with the PolKit libraries directly, instead.

outfoxxed

Partial review on the interface.

Before continuing further, I've noticed the polkit agent interface appears to be available directly over dbus. Is there a reason to use any library at all for it? (Especially a gobject library)

outfoxxed · 2025-10-13T05:23:50Z

src/services/polkit/qml.hpp

+//! Represents a user or group that can be used to authenticate.
+class Identity: public QObject {
+	Q_OBJECT;
+
+	// clang-format off
+	/// The Id of the identity. If the identity is a user, this is the user's uid. See @@isGroup.
+	Q_PROPERTY(id_t id READ id CONSTANT);
+
+	/// The name of the user or group.
+	///
+	/// If available, this is the actual username or group name, but may fallback to the ID.
+	Q_PROPERTY(QString string READ name CONSTANT);
+
+	/// The full name of the user or group, if available. Otherwise the same as @@name.
+	Q_PROPERTY(QString displayName READ displayName CONSTANT);
+
+	/// The full path to a file containing an icon representing this identity, if available.
+	Q_PROPERTY(QString icon READ icon CONSTANT);
+
+	/// Indicates if this identity is a group or a user.
+	///
+	/// If true, @@id is a gid, otherwise it is a uid.
+	Q_PROPERTY(bool isGroup READ isGroup CONSTANT);
+
+	QML_UNCREATABLE("Identities cannot be created directly.");
+	// clang-format on
+
+public:
+	explicit Identity(
+	    id_t id,
+	    QString name,
+	    QString displayName,
+	    QString icon,
+	    bool isGroup,
+	    PolkitIdentity* polkitIdentity,
+	    QObject* parent = nullptr
+	);
+	~Identity() override;
+
+	[[nodiscard]] id_t id() const;
+	[[nodiscard]] const QString& name() const;
+	[[nodiscard]] const QString& displayName() const;
+	[[nodiscard]] const QString& icon() const;
+	[[nodiscard]] bool isGroup() const;
+
+	PolkitIdentity* polkitIdentity;
+
+private:
+	id_t mId;
+	QString mName;
+	QString mDisplayName;
+	QString mIcon;
+	bool mIsGroup;
+};
+
+//! A message to present to the user as part of an authentication request.
+class SubMessage: public QObject {
+	Q_OBJECT;
+
+	// clang-format off
+	/// The text to present to the user.
+	Q_PROPERTY(QString text READ text CONSTANT);
+
+	/// If true, the message should be presented as an error.
+	Q_PROPERTY(bool isError READ isError CONSTANT);
+
+	QML_UNCREATABLE("SubMessages cannot be created directly. They are populated during authentication.");
+	// clang-format on
+
+public:
+	explicit SubMessage(QString text, bool isError, QObject* parent = nullptr);
+	~SubMessage() override;
+
+	[[nodiscard]] const QString& text() const;
+	[[nodiscard]] bool isError() const;
+
+private:
+	QString mText;
+	bool mIsError;
+};
+
+//! A request for input from the user as part of an authentication request.
+class InputRequest: public QObject {
+	Q_OBJECT;
+
+	// clang-format off
+	/// The message to present to the user.
+	Q_PROPERTY(QString message READ message CONSTANT);
+
+	/// If the user's response should be visible. (e.g. for passwords this should be false)
+	Q_PROPERTY(bool echo READ echo CONSTANT);
+
+	QML_UNCREATABLE("InputRequests cannot be created directly. They are populated during authentication.");
+	// clang-format on
+
+public:
+	explicit InputRequest(QString message, bool echo, QObject* parent = nullptr);
+	~InputRequest() override;
+
+	[[nodiscard]] const QString& message() const;
+	[[nodiscard]] bool echo() const;
+
+private:
+	QString mMessage;
+	bool mEcho;
+};


Why not use Q_GADGETs?

outfoxxed · 2025-10-13T05:26:19Z

src/services/polkit/qml.hpp

+    /// The D-Bus path that this agent listener will use.
+    ///
+    /// > [!INFO] This value can be set to any valid path and has no impact on
+    /// > the functionality of the listener.
+    Q_PROPERTY(QString path READ path WRITE setPath REQUIRED);


We should have a default value for this or not expose it at all. I can't think of a reason to have more than one polkit agent running at once, in which case we should just put it on a standard path for quickshell. If there is a reason to run more than one, we should still provide a default because most people wont want to or know how to configure this.

Formatting broke here

outfoxxed · 2025-10-13T05:33:00Z

src/services/polkit/qml.hpp

+	QSDOC_TYPE_OVERRIDE(ObjectModel<Identity>*)
+	Q_PROPERTY(UntypedObjectModel* identities READ activeIdentities NOTIFY authenticationRequestStarted);


Models are best used on lists that are either very long or subject to change in a way that we'd like users to be able to handle individually (e.g. item removed from list animation). A simple QList<Identity*> should suffice here.

outfoxxed · 2025-10-13T06:51:32Z

src/services/polkit/qml.hpp

+	/// Emitted whenever @@isActive changes.
+	void isActiveChanged();
+
+	/// Emitted after the identity that is authenticating has changed by user request.
+	void selectedIdentityChanged();
+
+	/// Emitted whenever the authentication conversation generates a new message to present.
+	///
+	/// This is always additional to the authentictation request's base message.
+	void subMessageChanged();
+
+	/// Emitted when the user needs to provide input to continue authentication.
+	void inputRequestChanged();


Change signals don't need docstrings and are hidden by default

Cu3PO42 · 2025-10-13T07:35:47Z

Thanks for the review!

I don't like using a gobject library here either, but yes, there is a good reason for using it instead of interacting directly over DBus. PolKit requires the client that sends the confirmation message to be privileged. To this end, libpolkit has a setuid helper binary that it invokes, which handles the actual PAM conversation, and shuffles all of the data between the parent and the helper.

To avoid libpolkit, Quickshell would either need to run as root or we would need our own setuid helper. Besides the implementation overhead, this would break simply running it from your Flake via Nix or installing it via Home-Manager. I imagine (although I don't positively know) many distros might also look more sceptically at any setuid binaries.

Finally, the DBus interface isn't actually stable because it is currently considered an implementation detail. I don't think it has changed significantly in recent years, but I would still prefer the more stable solution.

outfoxxed · 2025-10-13T07:38:02Z

That's horrible on so many levels.

Cu3PO42 · 2025-10-13T07:43:03Z

I... yeah. But that's Polkit for you. There's many things I wish I didn't know about it.

If, given these circumstances, you'd rather not have the code upstream, I'm just going to package it up as a separate library.

outfoxxed · 2025-10-13T07:44:07Z

I'm fine with the gobject code since its an actual requirement, and this is a feature many people want. I'll finish the review tomorrow probably.

Cu3PO42 · 2025-10-13T07:47:15Z

Thank you! I'm not in any rush. Also, fair warning: this is the first 'not insignificant' thing I've done with Qt, so while I tried to do things right, some things might be the way they are because I didn't know any better. I'll happily take any pointers.

outfoxxed · 2025-10-13T08:02:28Z

Thats fine. One further thing that would be helpful would be to add a manual test qml file similar to what exists in other parts of the source. Just cover a reasonable amount of the api for a simple manual regression test

Cu3PO42 changed the title ~~scope/polkit: add service module to write PolKit agents~~ Add service module to write PolKit agents Oct 11, 2025

service/polkit: add service module to write PolKit agents

68ea7e9

Cu3PO42 force-pushed the polkit branch from 78a68fb to 68ea7e9 Compare October 11, 2025 22:31

service/polkit: integrate with polkit libraries directly

20a8d3a

outfoxxed requested changes Oct 13, 2025

View reviewed changes

		QSDOC_TYPE_OVERRIDE(ObjectModel<Identity>*)
		Q_PROPERTY(UntypedObjectModel* identities READ activeIdentities NOTIFY authenticationRequestStarted);

Add service module to write PolKit agents #302

Are you sure you want to change the base?

Add service module to write PolKit agents #302

Conversation

Cu3PO42 commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

outfoxxed commented Oct 12, 2025

Uh oh!

Cu3PO42 commented Oct 12, 2025

Uh oh!

outfoxxed commented Oct 12, 2025

Uh oh!

Cu3PO42 commented Oct 12, 2025

Uh oh!

Cu3PO42 commented Oct 12, 2025

Uh oh!

outfoxxed left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

outfoxxed Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

outfoxxed Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

outfoxxed Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

outfoxxed Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Cu3PO42 commented Oct 13, 2025

Uh oh!

outfoxxed commented Oct 13, 2025

Uh oh!

Cu3PO42 commented Oct 13, 2025

Uh oh!

outfoxxed commented Oct 13, 2025

Uh oh!

Cu3PO42 commented Oct 13, 2025

Uh oh!

outfoxxed commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Cu3PO42 commented Oct 11, 2025 •

edited

Loading

outfoxxed left a comment •

edited

Loading