Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Allow to assign a default role to LDAP users #3823

Closed

Conversation

tograss
Copy link

@tograss tograss commented Apr 18, 2024

https://bugs.limesurvey.org/view.php?id=17227
New feature #17227: Allows to select a role, wich is assigned to all LDAP users. The default is to do nothing so existing setups still work.

To keep the plugin decoupled and facilitate plugin development I also somewhat expanded the plugin api to work with roles.
I'm a bit unsure if how I set the timestamps in addRole is correct and weather it should be called role or permissiontemplate.

gabrieljenik and others added 16 commits April 19, 2024 00:33
Updated translation: Czech by jelen1
Updated translation: Finnish by Jmantysalo
Updated translation: Japanese by d_inoue, nomoto
Updated translation: Polish by elissa
Updated translation: Slovak by jelen1
Updated translation: Turkish by bulent, kayazeren
Updated translation: Czech (Informal) by jelen1
Updated translation: Polish (Informal) by elissa
Updated translation: German by c_schmitz
Updated translation: Greek by kajetan
Updated translation: Malay by aidi.ahmi
Updated translation: Nepali by limeguy
…uestions (#3808)

Co-authored-by: lapiudevgit <devgit@lapiu.biz>
…ess phpinfo page (#3801)

Co-authored-by: lapiudevgit <devgit@lapiu.biz>
Updated translation: Finnish by Jmantysalo
Updated translation: Korean by modernity4r
Updated translation: German (Informal) by Akaer, c_schmitz
Co-authored-by: lapiudevgit <devgit@lapiu.biz>
@TonisOrmisson
Copy link
Collaborator

you probably meant to target develop, not master branch?

@tograss
Copy link
Author

tograss commented Apr 19, 2024 via email

@TonisOrmisson TonisOrmisson changed the base branch from master to develop April 19, 2024 06:25
@TonisOrmisson TonisOrmisson changed the base branch from develop to master April 19, 2024 06:31
@TonisOrmisson TonisOrmisson changed the base branch from master to develop April 19, 2024 06:31
@Shnoulle
Copy link
Collaborator

When i need specific system by plugins : i create plugin event in plugin.

I don't know if this feature must be in plugin or in a new plugin.

If it's a needed feature : maybe we need a clear API function for other plugin too ?

@tograss
Copy link
Author

tograss commented Apr 22, 2024

Hi @Shnoulle
I'm not quite sure if you asking me and if you asking me what exactly the question / task is :-)
I think it makes sense to have this setting in the current plugin for the following reasons:
1.) When using LDAP you more likely than not have a lot of users so you want to use roles any way. Needing to find a new plugin for this makes it more complicate.
2.) Something similar already exists with the "assign new users the create permission option" . However this can not be used if you want to assign more than just the create permission by default.
3.) I / someone might extend the plugin further to allow assigning roles based on LDAP groups.

@Shnoulle
Copy link
Collaborator

I'm not quite sure if you asking me and if you asking me what exactly the question / task is :-)

No decision here. Just start a discussion about this feature :) . I am not a team leader here. Only an external developer.

1.) When using LDAP you more likely than not have a lot of users so you want to use roles any way. Needing to find a new plugin for this makes it more complicate.

I use SAML a lot, and I just use Permission (automatically set).

If I use Roles : I surely set roles in LDAP/SAML attribute ! New setting : get role from : I already work on a little plugin who create/set user group according to LDAP group. If we can have from LDAP : i think we must allow it.

3.) I / someone might extend the plugin further to allow assigning roles based on LDAP groups.

It's the reason I like more to have :

  1. A new plugin event in LDAP OR globally : NewUserCreated
  2. Create a little plugin to set Roles in NewUserCreated event

If it's a needed feature : maybe we need a clear API function for other plugin too ?

Here : if final decision is to get in LDAP plugin : i like to have a function in LimesurveyApi to assign role.

@tograss
Copy link
Author

tograss commented Apr 22, 2024

No decision here. Just start a discussion about this feature :) . I am not a team leader here. Only an external developer.

Ahh... I did not know that. You are so active here and in the Bugtracking forum I mistook you for an maintainer :-)

I use SAML a lot, and I just use Permission (automatically set).

I you not mind sharing: What SAML plugin are you using? I might be interested as well.

Here : if final decision is to get in LDAP plugin : i like to have a function in LimesurveyApi to assign role.

This is part of this pull request. I named the function addUserInRole. But naming things is hard :-)

@Shnoulle
Copy link
Collaborator

This is part of this pull request. I named the function addUserInRole. But naming things is hard :-)

Oh yes ! Lost in the partial update from master to develop too :)

Great 👍

Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be applied to develop only (without all the other update).

Maybe easier to redo it for develop ?

@tograss
Copy link
Author

tograss commented Apr 23, 2024

Ok I open a new pull request based on develop. Sorry for the trouble.

@tograss tograss closed this Apr 23, 2024
@Shnoulle
Copy link
Collaborator

Ok I open a new pull request based on develop. Sorry for the trouble.

No need to be sorry here :)

PS : before , please :

  1. Create a Feature request on https://bugs.limesurvey.org/main_page.php (see https://community.limesurvey.org/feature-request/)
  2. Use New feature #XXXX : for the 1st commit an dthe title

Others advise on https://manual.limesurvey.org/How_to_contribute_new_features

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants