ARETEedu / moodle-mod_arete

A repository service for Augmented Reality Learning Experience Models (compliant with IEEE p1589-2020)
GNU General Public License v3.0
3 stars 2 forks source link

Please review permission checking in custom webservice files #72

Open danmarsden opened 2 years ago

danmarsden commented 2 years ago

One of the first things that should happen - before any functions are called are checks to see if the user is logged it or if the token is valid - some of the functions called here in this page do not appear to do this. splitting the code out like mentioned in #64 should help to make this clear.

https://github.com/ARETEedu/moodle-mod_arete/blob/main/classes/webservice/getdata_from_db.php

Make sure you do the same with your other files too..

fwild commented 2 years ago

Related to this? https://github.com/ARETEedu/moodle-mod_arete/issues/57

jafarigit commented 2 years ago

@danmarsden This class is used on MirageXR and always the requests(POST) will be sent together with the token. If the token is valid means the user is logged in MirageXR.

danmarsden commented 2 years ago

@jafarigit can you please run this past a senior developer - this page does not check that the token is valid, (I can pass any junk I want into the "token" field) - from "abcd" to "1234" If this token is a Moodle webservice token, (which can also be generated by any user in Moodle) It should also be performing a capability check - can "this" user, see or do the thing they are trying to do.

You also seem to use Moodle code to call a Moodle web service as a webservice in get_user_arete_modules_ids() in a very strange way.

Unfortunately this code violates our security guidelines \

Personally I think you should delete this file completely and implement a Moodle webservice within your plugin to do this rather than in this custom file but you should probably reach out to another developer to help you do this properly (unfortunately I don't have the time to provide mentoring support for developers writing their own plugins.)

thanks!

fwild commented 2 years ago

Update: users with admin privileges should receive the full list of ARLEM resources, including all private ones from all users.

Callustian commented 2 years ago

Is this related to #94 ?

fwild commented 2 years ago

yes.

Callustian commented 1 year ago

Also linked to #113

Callustian commented 1 year ago

Also linked to #115

Callustian commented 1 year ago

@danmarsden Could you explain "You also seem to use Moodle code to call a Moodle web service as a webservice in get_user_arete_modules_ids() in a very strange way.". Do you mean that that function should do a SQL query instead? Thanks

danmarsden commented 1 year ago

You are using Curl to make a call to a local Moodle api function. https://github.com/ARETEedu/moodle-mod_arete/blob/main/classes/webservice/getdata_from_db.php#L152-L174

you could almost completely replace that code with a simple call to: $data = core_enrol_external::get_users_courses($userid);