moodle-auth_saml2 icon indicating copy to clipboard operation
moodle-auth_saml2 copied to clipboard

Adds role mapping from Idp

Open jvinolas opened this issue 4 years ago • 5 comments

We've implemented the admin, course creator and manager role mapping. Tested working against keycloak role mapping.

#405

jvinolas avatar Mar 31 '21 15:03 jvinolas

hi @jvinolas this looks really great and this feature is well overdue. A few comments in the pr, and can you please also touch the README.md where it mentions roles not being managed?

brendanheywood avatar Mar 31 '21 23:03 brendanheywood

Hi @jvinolas, just edit PR (click edit button above near the title) and make it target MOODLE_39_STABLE rather than master. Thanks!

kabalin avatar Jun 10 '21 21:06 kabalin

thanks @kabalin - I've just adjusted that - ideally I'd prefer @brendanheywood to come back and finish off the PR, but if not @nhoobin do you want to look at this one?

danmarsden avatar Jun 10 '21 21:06 danmarsden

hmm - I'd add a -1 to merging this with hard-coded role shortnames (coursecreator/manager) - I think we'd need to tidy up the way roles are mapped before merging this.

danmarsden avatar Jun 10 '21 21:06 danmarsden

With regard to role assigning, I would suggest using approach similar to one in auth_ldap: https://github.com/moodle/moodle/blob/master/auth/ldap/auth.php#L1838-L1859, so that the actual component that does role allocation is recorded and those roles do not interfere with other roles user may have. This will simplify role unassigning as well (those roles not listed in attribute will be removed as long as they were allocated through this plugin) and you would not need to hardcode roles (any assignable role will work). IMO site admin allocation though this is not a good idea, I would suggest removing this option.

kabalin avatar Jun 10 '21 22:06 kabalin