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

Session moodle #3504

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mcassisa
Copy link

Added the authentication through Moodle session on the model of remote user (auth = none because it is assumed that authentication is already managed by moodle itself). In order to easily get the moodle's session (i.e. without any modification in moodle) , MRBS should be in a subdirectory.
The users that have the capability to create courses in a specified category in moodle ("moodle/course:create") - tipically this is true for teachers - are granted an Access level = 1 (further adjustments on this can be done to meet the diverse installation requirements)

@jberanek
Copy link
Member

Thanks for this @mcassisa - we'll take a look at this and get back to you.

Copy link
Contributor

@campbell-m campbell-m left a comment

Choose a reason for hiding this comment

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

This looks useful. I have a few comments:

In AUTHENTICATION line 851 is

$auth['joomla']['rel_path'] = '..';   // Path to the Joomla! installation relative to MRBS.

I'm not familiar with Moodle. Is Joomla necessary to run it?

In systemdefaults.inc.php line 1047:

$auth['moodle']['rel_path'] = '..'; //MRBS should be in a subdirectory of moodle

Does the comment mean that MRBS has to be in a subdirectory of Moodle, or is this comment just explaining the '..' setting? Could you make the comment clearer please.

Also, the config settings and comments in AUTHENTICATION are not the same as those in systemdefaults.inc.php.

@mcassisa
Copy link
Author

I fixed the documentation in AUTHENTICATION file and cleaned it up

@campbell-m
Copy link
Contributor

campbell-m commented Sep 12, 2023

I fixed the documentation in AUTHENTICATION file and cleaned it up

Thanks.

One other comment: does Moodle offer a user API so that you can get details of users other than the currently logged in user? That would enable you to create AuthMoodle.php with methods such as getUserFresh() and getUsernames().

@mcassisa
Copy link
Author

Moodle offers some way to retrieve users, but apparently the operation is very heavy for large sites. What would the benefits to retrieve a list of moodle users?

@campbell-m
Copy link
Contributor

Getting a single user's details, through getUserFresh(), would allow you to see a user's display name rather their username when viewing booking details. I assume that this operation is not database intensive.

Getting all the users, through getUsernames(), would allow admins to create bookings on behalf of other users by selecting them from a drop-down list of users. The drop-down is populated in the background by an Ajax call, so should not slow down MRBS. But you could always choose not to implement this and just do the single user case above.

@Neustradamus
Copy link

Any progress on this PR?

@mcassisa
Copy link
Author

mcassisa commented Jan 7, 2024

No, I didn't work on it in the meantime (we chose to manually create users) - the contribution is usable, although it can be further improved

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

Successfully merging this pull request may close these issues.

4 participants