georchestra / datadir

geOrchestra configuration directory for generic wars & Debian packages (eg: /etc/georchestra)
5 stars 33 forks source link

Rabbitmq support - spring rabbitmq events console and gateway #320

Closed marwanehcine closed 10 months ago

marwanehcine commented 11 months ago

On this PR, we add new template to be sent to admin users when new OAuth2 account is created

fvanderbiest commented 11 months ago

Thanks. But do we need a dedicated template ? I'm sure we can reuse the existing one with an additional variable for the origin (OAuth, local ...)

In addition, we should add a link to the console app in order to manage this user.

marwanehcine commented 11 months ago

Thanks. But do we need a dedicated template ? I'm sure we can reuse the existing one with an additional variable for the origin (OAuth, local ...)

In addition, we should add a link to the console app in order to manage this user.

I think we need custom template, cause file structure is different, we do remove and add entries. Link to management web page is added

fvanderbiest commented 11 months ago

I'd rather have https://github.com/georchestra/datadir/blob/master/console/templates/newaccount-requires-moderation-template.txt renamed into newaccount-notification.txt and the current process (when a new OAuth2 user connects) making use of it, if possible.

The message would be:

Dear admin,

A new account was created on {instanceName}:

User name: {name}
User email: {email}
User organization: {org}
Identity provider: {provider}

Direct account management: {publicUrl}/console/manager/users/{email}/infos 

---
Sent by {instanceName} ({publicUrl}/)
pmauduit commented 11 months ago

I'd rather have https://github.com/georchestra/datadir/blob/master/console/templates/newaccount-requires-moderation-template.txt renamed into newaccount-notification.txt and the current process (when a new OAuth2 user connects) making use of it, if possible.

There are already both templates (one for the moderation, one for the notification):

As it also makes sense for me to reuse the same template if this is possible, after having discussed with @marwanehcine, I think that in order to be able to do so, we would need to have the same templated variables in both cases (oauth2 or regular users). And for now, the oauth2 users don't have any organisation attached, which can be an issue, I think we rely on a 1:1 relation between users and orgs elsewhere in geOrchestra (not related to this PR though).

landryb commented 11 months ago

And for now, the oauth2 users don't have any organisation attached, which can be an issue, I think we rely on a 1:1 relation between users and orgs elsewhere in geOrchestra (not related to this PR though).

definitely an issue.. for me, an user without org is alien, because many things/rights/access depend on the org it belongs.. :)

fvanderbiest commented 11 months ago

definitely an issue..

This does not belong to this issue, but would you be in favor of a default organization for users from remote identity providers ? (let's open a github discussion on the subject if it makes sense)

landryb commented 11 months ago

im not sure what is best between a default dummy org or no org.. in all cases, if they need to access resources restricted to an org that means a platform admin has to contact the user to gather info about its real org, create it and assign it to the oauth user, eg some manual work as when a platform admin validates pending users. but yeah, discussion doesnt belong here :)

marwanehcine commented 10 months ago

Is it possible to add the support for rabbitmqPort too? In case someone is running rabbitmq on another port.

I have to add it to .envs-rabbitmq on PR "https://github.com/georchestra/docker/pull/213/files" which is merged already.

The unique solution right now is to added directly on "default.properties" file. I would prefer to add it to " .envs-rabbitmq" like other parameters.

fvanderbiest commented 10 months ago

Is it possible to add the support for rabbitmqPort too? In case someone is running rabbitmq on another port.

I have to add it to .envs-rabbitmq on PR "https://github.com/georchestra/docker/pull/213/files" which is merged already.

You can still do another PR :-)

fvanderbiest commented 10 months ago

Someone ports this to docker-master ? Thx.

edevosc2c commented 10 months ago

Someone ports this to docker-master ? Thx.

oops I haven't seen this was targeting the master branch, master do not support environment variables! I'm not sure if this was a good idea to merge into master then.

I'll create a PR for docker-master.

fvanderbiest commented 10 months ago

I'll create a PR for docker-master.

Thanks Emilien. May I ask you to fix master too ?