SafeExamBrowser / seb-win-refactoring

Safe Exam Browser for Windows.
https://www.safeexambrowser.org/news_en.html
Mozilla Public License 2.0
180 stars 121 forks source link

SEB Query String Parameter works on ?? but not on ? #979

Open RoChess opened 5 days ago

RoChess commented 5 days ago

On SEB 3.7.1 activated the Exam > "Allow Query Parameter" option.

Adjusted User Interface options to display address-bar, because SEB log files show no entries for startUrl or the Query-Parameter that end up getting loaded/used.

created "seb-test.seb" with https://google.com/ start URL.

Launching it as sebs://example.com/seb-test.seb??query-params resulted in SEB loading https://google.com/?query-params But using sebs://example.com/seb-test.seb?query-params caused SEB to load https://google.com/

Expected behavior was to see https://google.com/query-params in address-bar.

Without log entries, not sure if I'm doing something wrong.

dbuechel commented 4 days ago

Please make sure to set the log level to "Debug" and then set "Show URLs" to "Before Title" or "Always" to enable logging of URLs (see https://safeexambrowser.org/windows/win_usermanual_en.html#BrowserPane).

@danschlet Is the query string parameter feature supposed to work with single question marks as well? I'd need to go back and check the specification and implementation, but according to my recollection it requires two question marks.

RoChess commented 4 days ago

@danschlet the manual has this reference:

Query String Parameter

Allows to transfer paramters in the URL of a SEB configuration file or seb(s) link to the Start URL by adding ‘?’ or ‘??’ to the link. This feature e.g. allows to personalize the Start URL by adding some user id form another web applikation that is providing the SEB-Link.

@dbuechel DEBUG logging was already active, but to make it easier I've created a quick SEB config for DuckDuckGo, and uploaded it with a little test page for the different scenarios:

https://sandbox.verifyexpress.com/sebparam/

The as-is and the ??query one all work, but it is the ?query one that I'm hoping to get functional to improve end-user experience.

What I meant with lack of logging is that the DEBUG logs on Runtime will reveal how the SEB config was loaded, and you can see entries in Runtime.log such as:

2024-09-20 15:55:36.853 [08] - DEBUG: [NetworkResourceLoader] Sending GET request for 'https://verifyexpress.com/seb/ddg.seb??test'...
2024-09-20 15:55:37.023 [08] - DEBUG: [NetworkResourceLoader] Received response '200 - OK'.

2024-09-20 15:56:00.740 [08] - DEBUG: [NetworkResourceLoader] Sending GET request for 'https://verifyexpress.com/seb/ddg.seb?test'...
2024-09-20 15:56:00.872 [08] - DEBUG: [NetworkResourceLoader] Received response '200 - OK'.

But I was expecting the Runtime.log to then also have a reference to the "StartURL" or "StartURL + QueryParam" URI it would end up loading.

Found it in the Client.log though (must have overlooked it yesterday):

2024-09-20 15:55:45.942 [38] - DEBUG: [KeyGenerator] Successfully calculated browser exam key using integrity module.
2024-09-20 15:55:46.043 [32] - INFO: [Browser Window #1] Navigated to 'https://duckduckgo.com/?test'.

2024-09-20 15:56:04.047 [41] - DEBUG: [KeyGenerator] Successfully calculated browser exam key using integrity module.
2024-09-20 15:56:04.160 [34] - INFO: [Browser Window #1] Navigated to 'https://duckduckgo.com/'.

But as you can see ??test works, but ?test is failing.

danschlet commented 3 days ago

Unfortunately the Windows manual omits one essential information. The SEB for macOS manual is more precise about this:

The seb(s):// link to the config file can contain an additional query string, separated from the main URL by '?' or '??' (if the URL itself doesn't contain a query). SEB will then append this query string to the Start URL. This feature can for example be used to pass an individual login token to an exam system when SEB is started by opening an seb(s) link in another browser, without having to generate an individual config file for each user.

Always the string following after the SECOND '?' is used as the query parameter. If your main URL contains a query string, then you add the query parameter after that and an additional, second '?'. If your URL doesn't contain a query, then you must append two '?' ('??') for SEB to recognize where the query string parameter starts.

danschlet commented 3 days ago

Launching it as sebs://example.com/seb-test.seb??query-params resulted in SEB loading https://google.com/?query-params But using sebs://example.com/seb-test.seb?query-params caused SEB to load https://google.com/

Expected behavior was to see https://google.com/query-params in address-bar.

No, because the feature is exclusively meant to add a query to a Start URL. That's why it's called "Query String Parameter".

dbuechel commented 1 day ago

The seb(s):// link to the config file can contain an additional query string, separated from the main URL by '?' or '??' (if the URL itself doesn't contain a query).

This should actually already work like that according to the code (unless I'm missing something elementary here):

https://github.com/SafeExamBrowser/seb-win-refactoring/blob/0a9a9fa6fe8ed43d1c2dc8a7b3e0ba7b84beccc3/SafeExamBrowser.Runtime/Operations/ConfigurationOperation.cs#L262

dbuechel commented 1 day ago

Terribly sorry, I stand corrected! There is indeed a bug (or rather, if my recollection serves me right, I was not aware that single query separators should be handled as well). The code thus must be changed as follows:

uri.Query.LastIndexOf('?') >= 0
-or-
uri.Query.Contains('?')

We'll fix this for an upcoming version and let you know once the change can be tested.

danschlet commented 1 day ago

Terribly sorry, I stand corrected! There is indeed a bug (or rather, if my recollection serves me right, I was not aware that single query separators should be handled as well). The code thus must be changed as follows:

uri.Query.LastIndexOf('?') >= 0
-or-
uri.Query.Contains('?')

We'll fix this for an upcoming version and let you know once the change can be tested.

@dbuechel Did you read my posting above? Please make sure to handle the specification correctly (unless your implementation already does that). SEB only handles a SEB query string parameter if the sebs:// link contains two '?', it takes the string following the second '?' as the query parameter (and the setting startURLAppendQueryParameter has to be true in the settings used).

The seb(s):// link to the config file can contain an additional query string, separated from the main URL by '?' or '??' (if the URL itself doesn't contain a query). SEB will then append this query string to the Start URL. This feature can for example be used to pass an individual login token to an exam system when SEB is started by opening an seb(s) link in another browser, without having to generate an individual config file for each user.

Always the string following after the SECOND '?' is used as the query parameter. If your main URL contains a query string, then you add the query parameter after that and an additional, second '?'. If your URL doesn't contain a query, then you must append two '?' ('??') for SEB to recognize where the query string parameter starts.

dbuechel commented 1 day ago

@dbuechel Did you read my posting above?

Yes, but maybe the specification isn't very intuitive and also unnecessarily complicated. It would be way easier to simply always use double question marks ?? to mark the beginning of a custom SEB query string instead of having to distinguish between different cases...

So let's try it again:

The seb(s):// link to the config file can contain an additional query string, separated from the main URL by '?' or '??' (if the URL itself doesn't contain a query).

I understand this as follows: The SEB query string can either be marked with a single question mark when there is no query string in a resource locator (example.com?seb_query=true). If there is already a query string in a resource locator, then the SEB query string is marked with a double question mark (example.com?normal_query=true&other_parameters=...??seb_query=true).

Do I understand that correctly or am I mistaken?

danschlet commented 1 day ago

No, the feature description mentions, that it is an "additional query string". So it's very simple:

  1. If a seb(s) link contains one query in the URL (prefixed with '?'), there is no "additional query string", e.g. no SEB query string parameter and the URL is treated as a regular URL.
  2. If a seb(s) link contains more than one query (my code is actually explicitly checking for two), when defining query as a sub-string in a URL which starts with a '?' character, then take the second one as the "additional query string", remove it from the seb(s) URL and cache it for using it as the query string parameter to be attached to the startURL of the configuration linked by the seb(s) URL.

In case there is no regular query in the seb(s) URL, that's the case with two '??': The first "query string" is an empty string, the second, "additional query string" is the one following the second '?'

This definition works both in all cases

  1. a seb(s) URL containing a (regular, not SEB) query string https://safeexambrowser.moodle.ch/lms/mod/quiz/view.php?id=8812
  2. a seb(s) URL containing a (regular) query string and a SEB query string sebs://safeexambrowser.moodle.ch/lms/mod/quiz/view.php?id=8812?id=6
  3. a seb(s) URL containing only a SEB query string sebs://safeexambrowser.org/testing/DemoExamMoodleQueryDebug.seb??id=6

If this is a way more complicated specification than what you suggested lies in the eye of the beholder. I guess always using '??' would have worked as well, but it didn't come to my mind when I specified this feature. I also wanted to mimic the default URL protocol as much as possible, to avoid that systems parsing the URL would report an error. So it seemed most compatible to just append a second query. But that's probably not really a point, as in the case of no query in the seb(s) URL we still end up using two consecutive '?'.

danschlet commented 1 day ago

PS: I'm using a command

arrayOfQueryStrings = URL.query.componentsSeparatedByString:"?"

on the query-part of the seb(s) URL to find out if there is an additional query string (arrayOfQueryStrings.count == 2) arrayOfQueryStrings.lastObject then is the SEB query string parameter. This also works with seb(s) URLs containing '??'

RoChess commented 1 day ago

Much appreciated for the clarification.

Immediately understood implication if startURL would contain a query itself, so you get the https://example.com/?existing-query?seb-query scenario.

My original thinking was old school where https://example.com/index.php?page=about was often relying on .htaccess rules to accept https://example.com/about and that knowledge of ?page= was lost/hidden...

For that reason I was seeking to support /?about as query string to be passed forward as /about, and that ??page=about could be used for the primary reason.

I was willing to accept defeat and contact the website owners to find out what the ??query needs to be, but I would be highly appreciative if it indeed gets fixed/added in future version of SEB.

dbuechel commented 12 hours ago

Thanks for the clarification @danschlet, I meanwhile also went back and studied our internal, original specification (requirement no. 4 of REQ.F.006 for version 3.1.0). I do now understand and also recall how it is supposed to work; as a matter of fact, it indeed already does work exactly as specified, so there is nothing to be done here.

@RoChess The example you provided (see below) is hence not supposed to work: When there is no normal query string, you'll always need to use double question marks ?? to declare the beginning of a SEB query string parameter:

Load query: sebs://verifyexpress.com/seb/ddg.seb?test ❌

dbuechel commented 12 hours ago

For that reason I was seeking to support /?about as query string to be passed forward as /about, and that ??page=about could be used for the primary reason.

@RoChess I can't quite follow the use case. Could you please explain it with an example URL, i.e. what you'd like to use as startup (sebs) link and what you'd expect to resulting start URL to be?

RoChess commented 2 hours ago

For that reason I was seeking to support /?about as query string to be passed forward as /about, and that ??page=about could be used for the primary reason.

@RoChess I can't quite follow the use case. Could you please explain it with an example URL, i.e. what you'd like to use as startup (sebs) link and what you'd expect to resulting start URL to be?

I've accepted the explanation that my use-case was a matter of misinterpretation, because I fully understand @danschlet point of view, which means it was never supposed to work like that for me and I got carried away by the old Windows SEB manual explanation.

Now to answer your question; we made SEB functional for Canvas, Blackboard, and Brightspace.

Student goes through our product via LTI and is instructed that SEB is required to continue and sebs://config.seb download offered when they are for example in https://canvas.instructure.com/course/382 (through crafty JavaScript I can detect if protocol handler is available and show targeted download/install instructions if SEB is not installed).

SEB config startURL = https://canvas.instructure.com/ redirects to = https://canvas.instructure.com/login/canvas to allow student to login (have not investigated any SSO capabilities in SEB where I can forward authentication token) Student ends up at https://canvas.instructure.com/ at their dashboard/home page and has to manually navigate back to the course/382 section.

But we know the https://canvas.instructure.com/course/382 URL at launch in the regular browser, so I can ideally pass "/course/382" as query-param so that end-user is redirected to the same URL they started from and go through a better UX that skips a few clicks to manually get back to the same location.

Now part of the Canvas login method is that it routes via https://sso.canvaslms.com/delegated_auth_pass_through and I can in theory make it work via a SEB query-parameter like: ??target=https%3A%2F%2Fcanvas.instructure.com%2Fcourses%2F382

The problem is that any mistake or undefined workflow will lead to a cryptic error such as "invalid target" or "target must be absolute" that will leave student confused/stuck, whereas if I can keep using https://canvas.instructure.com/ as startURL then the student in worst-case will still end up getting logged into Canvas, and just has to fall back to manually navigation. Students being students, they often wait till 15-minutes before submission deadline to start, so I always strive for a solution that has low risk in blocking them from continuing.

My alternatives are now as follows:

  1. Use the SSO redirect URL with SEB query-param and introduce a cryptic point of failure that would lead to confusion and a spike in support tickets
  2. Request ???query-param option that appends "query-param" as-is to the startURL, but fear that will not be accepted based on the discussion in here on the firm definition of what a query-param represents
  3. Load custom JS into Canvas that supports SEB query param such as ??seb_redirect=/course/382, which introduces a JS redirect bottleneck and potential point-of-failure
  4. Leave it as-is and retain a bad user experience

I'm leaning towards 4th one just to avoid the potential of problems and confusion.