search

catalyst / moodle-auth_saml2

SAML done 100% in Moodle, fast, simple, secure
https://moodle.org/plugins/auth_saml2
70 stars 132 forks source link

Unusual Behavior Plugin Setting Page Moodle 4.1 #738

Open katcher opened 1 year ago

katcher commented 1 year ago

Recently installed moodle 4.1 on Dev server. Updated the saml2 plugin to version 2022111700

Log in appears to work as expected. When editing the setting page noticed some anomalies:

  1. The edit Private certificate key password had 2 text areas - Actual value appears to be a username possibly stored in a cookie. The editable link had the actual password but when it was switched to edit, the "username" appeared as password. The XX contained the current saml privateKey password.

<input type="password" name="s_auth_saml2_privatekeypass" id="id_s_auth_saml2_privatekeypass" value="XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" data-size="30" class="form-control d-inline-block d-none" data-initial-value="XXXXXXXX">

image

Clicked on the edit "pencil", the following was displayed: image

Unhid password to edit: image

This is a student name. The source has

  1. The Data mapping section had an entry of a username as well.

image

Hope that makes sense.

brendanheywood commented 1 year ago

The first one looks to be the same as this core bug which I just happened to fix by pure coincidence:

https://tracker.moodle.org/browse/MDL-76478

The second is the same kinda of problem in a different place

katcher commented 1 year ago

Hey Brendan, Is there any movement on this behavior or is it a core moodle issue bleeding into saml2? I think I have seen a similar issue with unwanted/invalid information being auto-populated in unexpected fields.

danmarsden commented 1 year ago

the handling of the password field is an issue with the core moodle password field as mentioned in MDL-76478 - if you cherry pick that into your site it should fix the password issues... the "username" field mapping might not get fixed though so if you get a chance to cherry-pick that upstream patch and test to see if the username field is still an issue please let us know!

katcher commented 1 year ago

Hi dan, merry and Happy to you and yours. I hate to say this but I do not know how to cherry-pick across branches. We have our own copy if you will of MOODLE_41_STABLE and I don't really know how to cheryy-pick to my version. I would love if you could explain it. But this should wait until after the holidays!

Have a good one.

Eric K

From: Dan Marsden @.> Sent: Thursday, December 22, 2022 6:45 PM To: catalyst/moodle-auth_saml2 @.> Cc: Eric Katchan @.>; Author @.> Subject: Re: [catalyst/moodle-auth_saml2] Unusual Behavior Plugin Setting Page Moodle 4.1 (Issue #738)

Attention This email originates from outside the concordia.ca domain. // Ce courriel provient de l'extérieur du domaine de concordia.ca

the handling of the password field is an issue with the core moodle password field as mentioned in MDL-76478 - if you cherry pick that into your site it should fix the password issues... the "username" field mapping might not get fixed though so if you get a chance to cherry-pick that upstream patch and test to see if the username field is still an issue please let us know!

- Reply to this email directly, view it on GitHubhttps://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcatalyst%2Fmoodle-auth_saml2%2Fissues%2F738%23issuecomment-1363442603&data=05%7C01%7Ceric.katchan%40concordia.ca%7C58fb9d0ffc3d46fbb56508dae47684a6%7C5569f185d22f4e139850ce5b1abcd2e8%7C0%7C0%7C638073494933510050%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bzEj%2FBbXi7NRrxqIDFKy18C%2F%2Bk3L8RvhnS%2Bu1%2FxfIRA%3D&reserved=0, or unsubscribehttps://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABBZXKJHVHQJ5LRFJE3YHEDWOTRW5ANCNFSM6AAAAAASPUT56Q&data=05%7C01%7Ceric.katchan%40concordia.ca%7C58fb9d0ffc3d46fbb56508dae47684a6%7C5569f185d22f4e139850ce5b1abcd2e8%7C0%7C0%7C638073494933510050%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RWq4%2F4ixhK4z4WOxKoa0z9hAIFf3EPH0PSyspLnc4mM%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.**@.>>

danmarsden commented 1 year ago

Thanks Eric, you'll probably just need to wait for moodle hq to integrate the fix in mdl-76478 (or ask your moodle support provider to do that for you) ;-)

katcher commented 1 year ago

Hi Dan and Brandon, funny, the issue indicated appears to be quite "Dangerous". For sh*ts and giggles, added the original private key and tried to resave - Broke Authentication as the incorrect value was saved. When I clicked on the pencil to view/edit it presented an invalid password - password for a test user. When inspected the element i saw a data-initial-value which was the password in question. The html value= "CORRECT-CERT-PWD" was still correct. When I tried the same edit in incognito no issue and in firefox no issue. Does/Did Brandon's core update prevent this issue or is it a bug I should add to moodle.org. PS I just update our dev moodle 4 to 4.1.1.

image

katcher commented 1 year ago

Funny, just removed all passwords saved for our microsoft adfs server where the username matched the password and mysteriously, problem disappeared. I assume it is something with chrome and stored passwords and initial value in data-initial-value field in form????

danmarsden commented 1 year ago

Hi Eric - yes - the patch in MDL-76478 will fix that, iI agree t's quite nasty and has forced several people I'm aware of to require a password reset on their admin accounts as it's resulted in their passwords being stored in raw text within settings that have since been exposed.

katcher commented 1 year ago

Hi Brendan and Dan, we are currently using

Moodle Snip version.php

$version = 2022112801.07; // 20221128 = branching date YYYYMMDD - do not modify! $release = '4.1.1+ (Build: 20230303)'; // Human-friendly version name $branch = '401'; // This version's branch. $maturity = MATURITY_STABLE; // This version's maturity level.

The bug still appears. We have tested on Chrome and Edge. Passwords set to autofill and save.

If we use the login form with username and password, the password is saved and displayed on for example the AUTH_SAML2 plugin Private certificate key password field.

If the autofill password is not set, the form works as expected. Firefox has not been affected with this bug.

I verified the git log of the 3 files which were modified for [MDL-76478] and the log shows the fix has been applied

Has anyone else tried and found the issue since reported as fixed?

Thanks Eric Katchan