On 2015-04-07 17:13, Wim Taymans wrote: > This is a new patch series that preplaces the previous patches regarding > access control. > > After some experimentation, this patch series adds support for async access > control checks, like when we need to ask for user input. The async support > is implemented by exiting the command without reply and then when the reply > becomes available, re-execute the command. > > There is an example access control module that shows how one could implement > client specific access control checks. > > The patch also contains a small typo fix that can probably be applied > independently. Thanks for this work. Apparently it's more relevant to me than I first thought. A few design thoughts that I think we need to resolve first before reviewing details: 1. Coupling between core and native-protocol. Right now the hooks are in core, and mapped 1-to-1 (or? are there exceptions?) to PA_COMMAND_*. Other options would be: - Just have a "pa_hook access[PA_COMMAND_MAX]" in pa_core instead? Then we could skip the long list of PA_ACCESS_HOOK_ constants. However, the increased dependency between the native protocol and the pa_core object might be undesirable. - On the other side, we could have the "pa_hook access[PA_COMMAND_MAX]" in pa_native_protocol instead. However, native protocol instances could - at least in theory - come and go, so they probably need stored somewhere more reachable anyhow... To sum up, I feel that since the hooks are specific to the native protocol, they should be put closer to that protocol, but I can't point to a better place to put it. 2. In general, I like the async mechanism. We might want to consider special constants, say, PA_HOOK_ASYNC and PA_HOOK_DENY to not abuse the existing PA_HOOK_CANCEL and PA_HOOK_STOP constants, but I'm not sure. Any opinions? 3. I also wonder if we need to make more information available to the hooks. E g, for allowing or blocking a stream moving somewhere, the hook does now only know what stream it's blocking, not to where moving is suggested. Maybe this is something that can be solved later though. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
