Adds role mapping from Idp
We've implemented the admin, course creator and manager role mapping. Tested working against keycloak role mapping.
#405
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?
Hi @jvinolas, just edit PR (click edit button above near the title) and make it target MOODLE_39_STABLE rather than master. Thanks!
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?
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.
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.