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

Can't get Moodle to validate keys OR allow reconfiguration error, please help! #905

Open
spicymoodles opened this issue Jun 6, 2024 · 61 comments
Assignees
Labels
third-party issue This issue is caused by a third-party system (e.g. an LMS). won't fix This issue will or can not be fixed at the moment.
Projects
Milestone

Comments

@spicymoodles
Copy link

Moodle 4.1
SEB 3.7

Hi there,

We have an exam coming up soon that requires allowing applications in SEB.

Normally we use our SEB template in Moodle, but this does not allow the use of applications.
I’ve tried setting up the Quiz activity to use “Yes – Upload my own config” but having issues with keys not validating and “you are not allowed to reconfigure SEB” error.

These are the config settings changes made:

  1. General:
    Start URL:
    link_to_exam.example.com

  2. Config File:
    Use SEB settings file for...
    'Starting an exam'

  3. User interface: No changes

  4. Browser: No changes

  5. Down/Uploads: No changes

  6. Exam:

Tick ‘Use Browser Exam Key and Configuration Key’
Copy the Browser Exam key (only as the last step after I made all the changes in the config tool) to the “Allowed browser exam keys” field in Moodle.

  1. Applications:
    'Choose application':
    • c:\program files\matlab\r2023b\bin\win64\MATLAB.exe (Also, tick the 'Autostart' box)
    • C:\Program Files\Microsoft Office\root\Office16\EXCEL (Also, tick the 'Autostart' box)
    • C:\Windows\System32\notepad.exe (no need for autostart) > Click OK for the 'Signature Load Error' popup window

  2. Security:
    Clipboard policy:
    Change to 'Allow'

  • I then save the config file without a password (Click YES on the popup window “.. without an encrypted password”)
    then copy paste the Browser Exam key to Moodle.

image


Already Moodle is showing me that the keys are not validated (the screenshot below is just in Moodle not in SEB

Capture

When I try to launch SEB I get this:

image

I have tried every setting combination I can just in case I was doing something wrong.

Things I have tried:

  1. Not use the "use browser exam key and config key" option at all.
    In Moodle it says "If no keys are entered, then Browser Exam Keys are not checked"

image

Doesn't work. Moodle insists on validating keys.

  1. Copy paste the Config Key instead of Browser Exam Key to Moodle
  2. Copy both keys to Moodle!

image

  1. Tick the "Allow reconfiguring secure/exam session" box. with no URL in the URL field:

image

All fail.

  1. Use the "Yes - Use a SEB client config" option to access the quiz from.

image

I place the SEB file on the desktop to start the keys, but same issue - the Keys could not be validated.
So there is no "Start Attempt" button showing on the quiz...

===============================================================================

  • What I also notice is that Moodle changes the keys! (I think when it converts the .SEB file into XML it does this)

So after I upload a SEB file, and then download it from Moodle via the "Download Configuration" box:

image

I then open it in SEB's config tool, and suddenly the keys are different!

What am I doing wrong? Or is our Moodle is the issue? (we have Nginx as reverse proxy FYI)

Here are some logs with the option of copy-paste the Browser Exam key to Quiz.

2024-06-06_14h37m36s_Browser.log
2024-06-06_14h37m36s_Client.log
2024-06-06_14h37m36s_Runtime.log

P.S If I lock the config file with a password, Moodle doesn't not let me upload it.

image

Any help would be appreciated! thank you

@dbuechel
Copy link
Member

dbuechel commented Jun 7, 2024

You'll need to save the configuration before copying the CK and BEK, i.e.:

  1. Make all the changes required for your configuration.
  2. Save the configuration file.
  3. Then copy the CK and BEK and paste them into Moodle.

@dbuechel dbuechel self-assigned this Jun 7, 2024
@dbuechel dbuechel added the being investigated This issue is being investigated. label Jun 7, 2024
@spicymoodles
Copy link
Author

Hi Damian,

Thank you for the reply.

I've tried that already.
I made sure I only copy the keys at the very last step of config, as noted in SEB instructions.

I've tried copy-pasting the CK + MEK then the MEK + CK , thinking maybe the it has to be in a certain order, no luck.

I still get the same error - that the keys could not be validated. This appears on the quiz preview page, before even launching SEB. Does that mean Moodle is capable of querying the SEB client?

@abowman-unige
Copy link

We encountred similar problem when uploading SEB 3.7 configuration with Moodle 4.1.9. When the seb config needed to be validated and did contain applications authorization. The problem was always there.

We did rollback with SEB 3.5 and had to redo our seb config in that version while we wait SEB 3.8

@dbuechel
Copy link
Member

@abowman-unige To which problem are you referring exactly? I'm afraid I can't quite follow as I am not too familiar with the inner workings of Moodle itself. Is this then an issue in Moodle when a configuration contains permitted applications, or is it an issue in SEB?

@abowman-unige
Copy link

Validating the key in moodle with SEB after reconfiguring itself when accessing the quiz. It only happens when we use it in moodle like this:
image

So we did the following steps:

  1. Rolled back to SEB 3.5 (knowing that it was working from past experience).
  2. Re/saved the config file in the SEB Configuration Tool in 3.5.
  3. Re-uploaded it to Moodle.
  4. It worked again.

We did not have time to further investigate the root cause of the problem, but we suspect an incompatibility between SEB 3.7 and Moodle 4.1.* when having some elements (apps in our case) in the SEB config file and letting Moodle calculate the key.

The problem should be on Moodle side, but knowing that it works in 3.5 and not in 3.7 we are not certain

@danschlet
Copy link
Member

Sometime ago someone reported a similar issue and in that case it was because of permittedProcesses in their SEB config (seems that some special characters, maybe backslash in the path of such allowed applications, cause SEB Windows to calculate a wrong ConfigKey and that's why it attempts to reconfigure).

The same config was working with SEB for macOS. If this can be confirmed, then the issue is definitely in SEB for Windows and not in Moodle.

@dbuechel
Copy link
Member

dbuechel commented Jun 12, 2024

The code related to the configuration key calculation hasn't changed in years: https://github.com/SafeExamBrowser/seb-win-refactoring/blame/master/SafeExamBrowser.Configuration/ConfigurationData/DataProcessor.cs#L44. If it is an issue, then it has been there since the beginning.

[...] we suspect an incompatibility between SEB 3.7 and Moodle 4.1.* when having some elements (apps in our case) in the SEB config file and letting Moodle calculate the key.

The problem should be on Moodle side, but knowing that it works in 3.5 and not in 3.7 we are not certain.

This would indicate an issue on Moodle side, otherwise it would neither work with version 3.5.x as the code hasn't changed for years (4 years to be exact, whereas SEB 3.5.0 has been released last year).

@nexterday
Copy link

nexterday commented Jun 12, 2024

Hi @spicymoodles @abowman-unige

in HttpProxyModule in your Ngnx conf:


proxy_pass httpS://WHAT IS YOUR HOST;
proxy_set_header HTTP_X_SAFEEXAMBROWSER_REQUESTHASH $http_x_custom_header;
proxy_set_header HTTP_X_SAFEEXAMBROWSER_CONFIGKEYHASH $http_x_custom_header;


Edit: Please also make sure the underscores_in_headers is set to on (by default Ngnx doesnt understand underscores in headers) -> this option is in the ngx_http_core_module directive.


underscores_in_headers on;


Please make sure you put the right things (or ask the system admins: please allow forwarding the custom headers in Ngnx), do NOT just copy paste.

The issue:
In Moodle 2 checks happen (you are not entering allowed exam keys field, it falls back to check the headers). Because in your case, you entered the keys, then it checked the header, then validated the entered keys against hash('sha256', CURRENT_MOODLE_URL + VALID_BORWSER_KEY) <-- the only 2 case it wont match are, either white space or the current url if not matching the url entered in the config (for example, capital letters, extra ? or # etc). Please note that FULLME in moodle (ie the current url) relies on the server globals, so it can be an issue (albeit rarely).. the only way to verify if the ngnx reverse proxy is not matching, is to do (in /mod/quiz/view.php, under require_once(config.php) -> print_r($FULLME); exit;) then it would tell if that returned result is correct or not.

--
What seems to happen on your side, is that your proxy does NOT forward custom headers (namely: HTTP_X_SAFEEXAMBROWSER_REQUESTHASH , and most likely: HTTP_X_SAFEEXAMBROWSER_CONFIGKEYHASH too).

@sehomer
Copy link

sehomer commented Jul 7, 2024

We got a similar issue.

I upload a config file to a moodle quiz, created with SEB 3.5., leaving the start url blank to use it with varius quizzes. It works fine, without entering the browser/exam key (till now I thought/think, that moodle alters the file, filling in the Start URL and recalculating the Keys).
SEBs not using this config get the error message that the Browser key could not be validated.

I updated to SEB 3.7, altered the exit password and saved the file -> Moodle displays that the key is not working.

I created a fresh new file, same error.

I switched back to SEB 3.5, opened the original SEB file and changed the password -> Uploaded to moodle -> the quiz starts as expected.

We are using Moodle 4.1

Best regards
Sebastian

@nexterday
Copy link

nexterday commented Jul 7, 2024

Hi Sebastian,

Validation process for Browser keys in moodle:
is HTTP_X_SAFEEXAMBROWSER_REQUESTHASH (or JS API) sent via seb client equals to the sha256 hashing of the URL* + Actual valid key entered in quiz settings).. if not equal, then fail with an error.
URL: URL of the page on which the SEB JS API generated the keys. i.e usually at the quiz view page (/mod/quiz/view.php). <-- I have the feeling that the URL is the core reason for the failure of the validation. <-- for example: you leave the starturl empty, but moodle try to match the generated key in seb client against the quiz url then it fails (only if the keys are entered in the quiz, otherwise, it works fine)

If the use of modern browser is set in seb client, then the key is being sent from seb client via JS API (instead of headers). if you wish to debug further and help us find out why it doesnt work on your side, disable the usage of JS API (via unticking of use modern browser setting in seb client config) and try again -> this time its being sent via header instead of JS API.

if it still gives you an error, then dig further:

  1. browser keys in moodle are validated on basis of what you have in the quiz settings (Browser keys, one per line) against what is being sent from seb client via a http header called (HTTP_X_SAFEEXAMBROWSER_REQUESTHASH). You should: 1. verify that the browser keys you entered in moodle are actually correctly generated. 2. make sure that custom headers are allowed in your web server (especially being forwarded and accepted if you are using a proxy), the header namely called HTTP_X_SAFEEXAMBROWSER_REQUESTHASH and also HTTP_X_SAFEEXAMBROWSER_CONFIGKEYHASH is required too.

  2. Moodle does the validation if, and only if, you fill the browser keys in quiz settings.. may I suggest you read the browser keys documentation.

When you alter anything in the config, then you technically need also to regenerate the the config keys and the browser keys too, and then update them in moodle.

Ps Because you are using moodle 4.1 (the plugin, while changed structure-wise, fundamentally is the same since technically, however, it makes it harder for us to pin point precisely due to our usage of moodle 4.3).

@danschlet
Copy link
Member

Sometime ago someone reported a similar issue and in that case it was because of permittedProcesses in their SEB config (seems that some special characters, maybe backslash in the path of such allowed applications, cause SEB Windows to calculate a wrong ConfigKey and that's why it attempts to reconfigure).

The same config was working with SEB for macOS. If this can be confirmed, then the issue is definitely in SEB for Windows and not in Moodle.

IMPORTANT READ THIS IF YOU USE PERMITTED PROCESSES

in that case a bug in current versions of SEB for Windows is the reason for failing key. Try it with the Mac SEB version or remove permitted processes from your settings to confirm it.

AGAIN, FIRST TRY IF IT IS BECAUSE IF PERMITTED PROCESSES BEFORE LOSING TIME WITH OTHER TESTS!

@sehomer
Copy link

sehomer commented Jul 7, 2024

If the use of modern browser is set in seb client, then the key is being sent from seb client via JS API (instead of headers). if you wish to debug further and help us find out why it doesnt work on your side, disable the usage of JS API (via unticking of use modern browser setting in seb client config) and try again -> this time its being sent via header instead of JS API.

Hi Nexterday, on what pane of the config tool can I find this setting? I do not see anything similar to that. I searched the documentation for "modern", "header" and "js api", did not find anything.

Best regards
Sebastian

@nexterday
Copy link

Apologies, I mixed it with Seb for Mac.. for windows: settings key sendBrowserExamKey = true

Again, pls match the starturl in the config with an actual quiz url. Js api sends the keys at that page only, so it checks against current url, hence the likeliness of failure

Ps please read Dani’s comment in case this has anything to do with the browser keys. Dani says it might well be the reason, this saves time in testing.

@sehomer
Copy link

sehomer commented Jul 7, 2024

Hi nexterday and Daniel,

-> prohibited processes: my configuration does not allow any third party apps and I also removed all prohibited processes -> no success

-> I also tried unticking "send browserExamKey", combined with entering the URL of the Quiz in the config -> no success.

--
From what the UI of moodle writes, it does key validation even with no keys entered in the quiz setting, but only if you upload a SEB file.

I have the feeling that the URL is the core reason for the failure of the validation. <-- for example: you leave the starturl empty, but moodle try to match the generated key in seb client against the quiz url then it fails (only if the keys are entered in the quiz, otherwise, it works fine)

When I leave the quiz URL empty, moodle alters the SEB file and fills in the URL of the Quiz where it is uploaded (the version of the config that can be downloaded by students). I used the same (version 3.5) old file on different quizzes, even on different moodle plattforms, it works fine. When I open this file with a config tool greater than 3.7 and save it again, Moodle claims that the key validation fails (no matter if I enter a browser key or not).

(The last paragraph may be a repetition of what I wrote, except that I checked the files that are actually downloaded to configure the SEB).

Thanks guys for answering on a sunday evening :D

@nexterday
Copy link

nexterday commented Jul 7, 2024

Interesting. Btw sendexamkeys should be ticked (not unticked).

Ok, can you share the url of the quiz, along with the generated exam key in seb, so I can check if generation is right? sharing the seb config too is good.

@sehomer
Copy link

sehomer commented Jul 7, 2024

I tried both ways, ticked and unticked.

What I suppose to send:
Old config that I upload / altered version from Moodle (working)

new config/altered version (not working)

I am going to mail it right now.

Best regards

@nexterday
Copy link

please include the exam browser keys you entered in moodle (and actual url to that quiz including the domain etc). NO user/pass access pls

@nexterday
Copy link

nexterday commented Jul 7, 2024

@dbuechel could you please verify that the following keys are included in the hashing calculation in 3.7

`

<key>batteryChargeThresholdCritical</key>

<key>batteryChargeThresholdLow</key>

<key>allowDownloads</key>

<key>allowUploads</key>

<key>disableSessionChangeLockScreen</key>

<key>enableCursorVerification</key>

<key>displayAlwaysOn</key>

<key>systemAlwaysOn</key>

`

@dbuechel dbuechel added this to To do in SEB 3.8.0 via automation Jul 8, 2024
@dbuechel dbuechel added this to the 3.8.0 milestone Jul 8, 2024
@dbuechel
Copy link
Member

dbuechel commented Jul 8, 2024

Thanks for the input and further investigation. We meanwhile stumbled upon this as well internally, it indeed appears that there has been a change in version 3.7.0 which seems to cause this issue.

I shall investigate it as soon as possible and report back here.

@dbuechel
Copy link
Member

dbuechel commented Jul 8, 2024

@nexterday These are new keys added in version 3.7.0 - that should however not be an issue, as the algorithm should be version agnostic (unless there is a bug in the algorithm on Windows side, which unfortunately appears to be the case).

@nexterday
Copy link

@dbuechel please note that it might not necessarily be the hashing algo, but potentially the client component <-- any addition to the URI here (even if its simple ? or #) it renders the keys invalid. If you have made a change in that component, its worth to have a look at.

@dbuechel
Copy link
Member

dbuechel commented Jul 8, 2024

@nexterday Thanks for the hint, yes will do so, I currently also cannot explain the behavior.

@dbuechel
Copy link
Member

dbuechel commented Jul 8, 2024

Primarily thanks to @nexterday, it turns out that in Moodle, when calculating the configuration key they convert the new configuration values batteryChargeThresholdCritical and batteryChargeThresholdLow of PLIST type real introduced with version 3.7.0 as follows:

"batteryChargeThresholdCritical":0.10000000000000001,"batteryChargeThresholdLow":0.20000000000000001

Whereas SEB for Windows will convert the values the same way as they're actually represented in the raw XML data:

"batteryChargeThresholdCritical":0.1,"batteryChargeThresholdLow":0.2

So we have here a classic floating-point precision issue, which I reckon always existed but now manifested due to the two new configuration values.

@danschlet I could not find a particular point in the specification under https://safeexambrowser.org/developer/seb-config-key.html with respect to floating-point precision numbers of type real. How are you converting them in your code? In SEB for Windows, these are currently converted using NumberFormatInfo.InvariantInfo.

Again many thanks to @nexterday, without whom I'd still be turning my code upside down looking for an issue or bug...

@sehomer
Copy link

sehomer commented Jul 8, 2024

Please note that we are using Moodle 4.1, later versions may have changed.

Thanks a lot for having a look on that issue.

@dbuechel dbuechel moved this from To do to In progress in SEB 3.8.0 Jul 8, 2024
@dbuechel
Copy link
Member

dbuechel commented Jul 8, 2024

Most likely, the issue has always existed. I thought there already were configuration keys of type real when adding the new keys for version 3.7.0, but checking our current default configuration, it appears that's not the case.

@nexterday
Copy link

nexterday commented Jul 10, 2024

In 2017, php came up with the idea that floats should be defaults to precision of 14 digits and some function to 17 digits just to standardise like other scripting languages etc.

now since that time we never treated these floats as strings because its meant for mathematical calculations rather than being taken as if they are strings like the case of hashing.

why it worked with seb (until 3.5)? Because you never had real (float) keys in your plist, but then it got added (battery and co) and then calculation of json encoding assumed them as extra bytes anyway which ended up not matching the config keys generated by seb.

Now that we dont care about precision in our moodle (many would, especially with certain plugins where float precision is very important such as for math, chemistry physics and precise point drawing etc) - we simply can change the precision in core moodle (or at php level) and move on.. the question raised then: what happens with other lms’s (illias edx or whatever)? They have to do the same (or being told or supported etc) which causes an overhead on seb team.. what we shall do then? My inquiry was, what if the real key is converted to string or array or, or then its quoted and calculation would work on any system. Its just a wonder and not a request.. just to lower overhead due to number of systems connected/using seb.

ps java (in our case seb server) encode them on standard 14 digits precision too. Maybe @anhefti needs to truncate them or they wont work. Basically on at least 7 main languages, the precision of floats is 14 digits

@danschlet
Copy link
Member

I do understand this, although I wonder why it previously worked with floating point values like 1.0 (which I had since SEB 3 for macOS in my configs and I assume I uploaded such files to Moodle 4.1 before).

You're surely right that converting those floats to strings would be safer and would save time for other people implementing support for SEB, so I guess we will have to go that path. Looks like @anhefti will have issues with that, as he cannot just change value types for existing setting keys in SEB Server. I actually also can't do that on Mac. So I guess we will have to implement a conversion just when creating the JSON for Config Key, which is significant additional work.

I still don't understand though why a floating point value of 0.1 is rounded to 0.10000000000000001 by Moodle. Seems like a bug to me.

@nexterday
Copy link

Its not a bug in php, its designed as a standard - almost all languages have 14 digits auto precision or else java too has a bug. Look at the comment above again (I also add the seb server case in Ps)

@nexterday
Copy link

nexterday commented Jul 10, 2024

Here is what AHE is doing. He is basically treating floats as doubles (defines the type of real as a double; ie does sort of implicit casting) in order to remove the precision. Basically he suffers from precision too due to < real /> in plist..

@nexterday
Copy link

Screenshot 2024-07-10 at 13 22 16

See the "special case" code plus casting et al.

@nexterday
Copy link

And here is it also converted to double in c# (seb win):

return Convert.ToDouble(node.InnerText, System.Globalization.NumberFormatInfo.InvariantInfo);

Basically precision is correct.. and we all try to convert for plist real.. problem is in real.

@danschlet
Copy link
Member

I don't even know the difference between real and double to be honest. The problem is, that Apple uses only for floating point numbers in plist. And I just converted it to JSON with standard system APIs. Looks like this doesn't work on other platforms.

But anyways, we have to solve the issue in SEB somehow, I agree with that now. We will see how to achieve that best.

@nexterday
Copy link

I fully understand.

If you look for example here https://github.com/SafeExamBrowser/seb-mac/blob/634c8ff5d179b4ddd834b9b231159fc64c6d2efd/SEB/SEBViewController.m#L346

you see that the float def got truncated to 2 digits in the log printing which means even c has the 14 digits.. so maybe we all truncate in all LMS’s in similar fashion or seb changes these real values to strings, then json encode them with double quotes and then no one in lms side has to worry about it.

This is not a matter in my hands because I influence neither - I just wanted to give an opinion so ye make a better and informed decision

@anhefti
Copy link
Member

anhefti commented Jul 10, 2024

In SEB Server I deal with 14 digits precision with the FORMATTER in code:

private static final DecimalFormat FORMATTER = new DecimalFormat("0.##############");

Java basically knows:
float : 32-bit precision IEEE 754 floating point
double : 64-bit precision IEEE 754 floating point

The formatter used now cuts the original double value (from the database) to a 14 digits precision floating point number and cutting also away tailing '0' values.
SEB Server can adapt to other formatting of floating point values for XML or JSON output (since these are simply text files/format)
So if I have to adapt to another format I can do that by changing the FORMATTER above while values in DB still are double values. This would be possible without any migration or so, and would apply immediately to all generated and downloaded SEB Configs from SEB Server.

So if we need some change on this SEB Server formatting, just tell me and I can change it for the release.

@nexterday
Copy link

I think this is something seb team should agree on. From moodle side, and now that its in integration phase, it becomes dangerous to play with precision because then moodle wont understand it again (we keep chasing our tails). Only safe move at the moment (if ever decided on by seb team) is that they avoid < real > at all costs and consider changing it to stringie type of key, then no one should/would worry about non compliance. Its only a suggestion and it doesnt even address any potential mess in the coding part at seb side, so please ignore it if its not possible

@dbuechel
Copy link
Member

We had an internal meeting and decided the following:

  1. The data type real is a valid and official data type of the underlying PLIST format used for SEB configurations. Its usage is thus correct and intended.
  2. The representation of floating-point numbers of type real for the configuration key calculation is however currently missing in our official documentation, but appears to be implicitly working the same way on macOS, Windows as well as SEB Server. We'll thus agree upon a format internally, update the documentation and inform here as well as on our website respectively.
  3. Floating-point numbers must henceforth be represented as specified when building the JSON for the calculation of the configuration key. This may require changes and adaptions in LMS or other web applications using the configuration key functionality.

@danschlet
Copy link
Member

I did some testing with a Moodle quiz on our public demo server.

I used a configuration with default settings of SEB 3.7 for Windows, which I got from Damian (although I wonder why enableScreenProctoring is true in these settings, unless that was caused by SEB for Mac, which I used to convert the client config to an unencrypted starting an exam config).

CK calculation didn't work when these two settings were contained:

	<key>batteryChargeThresholdCritical</key>
	<real>0.1</real>
	<key>batteryChargeThresholdLow</key>
	<real>0.2</real>

When I removed those two lines from that config file and uploaded it to the Moodle quiz again, I could correctly access the quiz with SEB (CK calculation worked correctly).

Note that that config still contains these three value settings:

<key>screenProctoringImageDownscale</key>
<real>1</real>
<key>defaultPageZoomLevel</key>
<real>1</real>
<key>defaultTextZoomLevel</key>
<real>1</real>

These don't cause a wrong CK calculation, so apparently that happens only when there is a dot/fraction in the real value. This explains why this error never occurred with the two default...ZoomLevel settings, which I introduced in SEB 3.0 for macOS/iOS, but never actually used a config file with a non-default value of 1 with Moodle.

So the workaround until the calculation of the CK can be fixed in Moodle Deeper Integration would be to remove all value settings with fractions from SEB configuration files before uploading those. SEB clients will then use the default value (or a non-fraction value like 2).

Unfortunately it would definitely be too much effort to change the value types for these settings in SEB clients. Also config files saved with current SEB versions containing fractions (I guess this would mostly apply to SEB 3.7 with the newly introduced battery values) would still cause a wrong CK calculation.

Also we cannot just use string for real values. These values must be numbers, so we would need to code special cases for specific settings where strings are converted into floating point numbers. That's not the idea of SEB configuration files, we explicitly chose the plist XML format because each setting value has a fixed value type which is identifiable by the SEB client!

@danschlet
Copy link
Member

And using only scaled integer values for floating point numbers again would cause more effort and defy logic/the purpose of using a strongly typed XML format.

@danschlet
Copy link
Member

danschlet commented Jul 10, 2024

The calculation of a CK is not a standard XML to JSON conversion. It's a custom algorithm which has to be performed following our specification. Unfortunately we missed to specify how to handle <real> values though when Deeper Integration was implemented. We will add this now to the specification and communicate accordingly, as suggested by @dbuechel.

@nexterday
Copy link

nexterday commented Jul 10, 2024

Hi Dani
Natural and integers do not have precision hence it works when there is no fraction even when its defined as float.

the precision in moodle is not seb specific but moodle wide specific and even if moodle doesnt change it, sysadmins are able to change it via php env ini file (same applies to all scripting languages). Only issue there is that sysadmins (and others involved) should know that this is a prerequisite. Some ppl might complain because it might have an effect on their 3rd party plugins (I yet to see anyone who can visually recognise 0.0000001 difference though).

it occurred to me that not as it can be complex to change (according to Damian and you) but also many users might already have a config here and there that might stop working due to any changes in seb client.

I think a bold “do it as in documentation” note should suffice for non moodle users, or old moodle versions users. I hope this wont cause further inquiries or support tickets though (maybe a small note to current users of main lms systems to notify them of the necessity to do so, should help reduce surprises).. At the end of the day seb shall dictate the mechanism when an easy fix is not possible.

@danschlet
Copy link
Member

It's really not a question of precision from the view of the specification of SEB settings (and configuration files), but of identifiable value types. It only becomes an issue in the implementation of the ConfigKey check. It's up to the developer who's implementing this check to consider how to handle values in a reproducible way.

We could even change the specification of these floating point values to cover a much smaller precision. Until now none of the existing settings require a 14 or 17 digit precision, likely a two or three digit places/fractions precision would be sufficient (for example 1.75 or 1.125). Still don't think that rounding 0.1 to 0.1000001 makes any kind of sense, seems like really bad implementation to me).

@nexterday
Copy link

nexterday commented Jul 10, 2024

I think too that precision is stupid and should only be explicitly requested (via extra param) but sometimes the guys behind interpreters (or feature requests) come up with a release based on a horse blinker view.

I get always precision via google apis, but I never knew if they had an effect because (in golang for example) 2.34 is equal to 2.39988888887 if the variable is defined as float). There must be a use case for it, but still doesnt justify defaulting precision to beyond initial value assignment. Maybe some day I’ll see or hear of a real life case where this precision made a difference.

there is no point of making a precision of less than “standard”.. 3 or 4 digits still wont make a difference. As per Damians outcome of the meeting, keep as is and others have to change accordingly

@aspark21
Copy link

aspark21 commented Jul 10, 2024

I think this might be resolved by https://tracker.moodle.org/browse/MDL-82286 which we have been funding a fix for

@danschlet
Copy link
Member

danschlet commented Jul 10, 2024

I think too that precision is stupid and should only be explicitly requested (via extra param) but sometimes the guys behind interpreters (or feature requests) come up with a release based on a horse blinker view.

I get always precision via google apis, but I never knew if they had an effect because (in golang for example) 2.34 is equal to 2.39988888887 if the variable is defined as float). There must be a use case for it, but still doesnt justify defaulting precision to beyond initial value assignment. Maybe some day I’ll see or hear of a real life case where this precision made a difference.

You're right that precision is an issue and I guess we were naive not to expect any rounding issues (which could happen with weird fractions any time when the values are read into high precision floating point values and converted back into strings in a JSON). It's just too late to change it in all SEB clients/server I think (unless we accept that old configuration files won't be fully compatible and may still cause issues down the line).

It probably would have been the easier solution to refrain from using floating point numbers and use multiplied integers. For new settings that's a choice we can take when specifying those settings. Although I don't see a natural specification
for example for a scaling factor (3, 2.5, 1, 0.5, 0.25, 0.125 etc.) which could be easily human readable without having to do calculations.

@dbuechel dbuechel added won't fix This issue will or can not be fixed at the moment. upstream This issue is caused by a framework or library used by the software. and removed being investigated This issue is being investigated. labels Aug 5, 2024
@dbuechel
Copy link
Member

dbuechel commented Aug 5, 2024

We'll update the specification as previously stated, but the cause of the issue was in Moodle and has just recently been resolved. Affected users may update to Moodle versions 4.3.6 or 4.4.2 as indicated in the issue: https://tracker.moodle.org/browse/MDL-82286.

@dbuechel dbuechel added third-party issue This issue is caused by a third-party system (e.g. an LMS). and removed upstream This issue is caused by a framework or library used by the software. labels Aug 5, 2024
@aspark21
Copy link

word of warning, we have been re-testing this with the MDL-82286 fix but it seems the issue is still present

@nexterday
Copy link

Hi @aspark21
Mind to share which exact moodle version you are using? The fix is integrated starting from 4.3 and 4.4

@aspark21
Copy link

yeah 4.4 both by cherry-picking the fix and using the integrated version. we'll keep investigating but just a heads up

@nexterday
Copy link

nexterday commented Aug 19, 2024

Interesting! To rule out wrong cherry pick: can you verify that /lib/setup.php line 259 has -1 value?

Also if you create a file (say test.php on your server) and add

**add php opening tag here

ini_set('precision', 14);
ini_set('serialize_precision', -1);
$val = 5.5;
echo json_encode($val);

and call script - does it give 5.5 or 5.4999999999?

if the above pass, then defo something else in the json encoding in seb plugin need to be checked).

@nexterday
Copy link

Just a heads-up - the keys are cached so maybe also verify that you are not using old configs/quizzes (ie test on new quizzes to be on the safe side)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
third-party issue This issue is caused by a third-party system (e.g. an LMS). won't fix This issue will or can not be fixed at the moment.
Projects
SEB 3.8.0
In progress
Development

No branches or pull requests

8 participants