prose-im / prose-pod-system

Prose Pod system configurations and build rules. Used to package everything together.
https://prose.org
Mozilla Public License 2.0
0 stars 0 forks source link

Should we get rid of the `admin.prose.org.local` `VirtualHost`? #18

Open RemiBardon opened 2 weeks ago

RemiBardon commented 2 weeks ago

@valeriansaliou Following some discussions with the Prosody team yesterday evening, I had a look at mod_groups_internal, which they recommended me over mod_groups. Here is a screenshot where they explain why mod_groups won’t suit our needs and the other one is better:

Screenshot 2024-06-19 at 11 32 09

mod_groups_internal doesn’t use a config file, but rather needs to be invoked from a plugin. We have mod_admin_rest for that, I just need to add a few routes. However, while looking at the code I noticed the exposed functions don’t take bare JIDs as arguments but just local parts (usernames). They use the current host to construct a JID. mod_admin_rest was exposed via the admin.prose.org.local VirtualHost… which means we won’t be able to use mod_groups_internal as we’d expect.

mod_admin_rest uses basic auth + prosody roles to authenticate insecure HTTP requests, and the “main” VirtualHost already exposes one http_port for mod_rest (sending stanzas via HTTP REST). Would it be okay if I migrate mod_admin_rest to the “main” VirtualHost (getting rid of admin.prose.org.local altogether) or should I modify mod_groups_internal to support bare JIDs? The latter seems very tricky, while the former doesn’t seem like a security breach since calls are authenticated… WDYT?

RemiBardon commented 2 weeks ago

I added support for mod_groups_internal in mod_admin_rest and I am migrating prose-pod-api from mod_groups to mod_groups_internal (via mod_admin_rest). As I was getting rid of the admin.prose.org.local VirtualHost, I realized we can't get rid of it because it's the only host we have during the Pod initialization phase.

RemiBardon commented 2 weeks ago

mod_admin_rest doesn't enforce its user to be from the same host as the module runs on, as long as it has the appropriate permissions. When creating the "main" VirtualHost, Prose Pod API will grant those permissions to prose-pod-api@admin.prose.org.local and we should be able to use a second mod_admin_rest instance running on the "main" host without too much trouble. I'll try this out tonight.

RemiBardon commented 2 weeks ago

Medium-term, we could also switch to the new implementation of mod_admin_rest which I proposed in https://github.com/prose-im/prose-pod-server/issues/5 if it is accepted.

valeriansaliou commented 1 week ago

Short answer: yes (we should). Reading explanations from Zash, I believe we should. I'll be honest, I've initially been used mod_groups for proof-of-concept purposes only, although it's worked quite well so far.

RemiBardon commented 1 week ago

We should "use mod_groups_internal" or we should "get rid of admin.prose.org.local"? I suppose it's the former since the latter wouldn't be possible[^1] but I want to make sure.

[^1]: We could but it would require a lot of issue-prone work to switch the admin account from a bootstrapping host to the "main" host.

valeriansaliou commented 1 week ago

I was referring to the former indeed 😄