Skip to content
GitLab
Projects
Groups
Snippets
/
Help
Help
Support
Community forum
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in
Toggle navigation
Menu
Open sidebar
integration
prechecker
Commits
d9d8e9c3
Commit
d9d8e9c3
authored
Jan 18, 2016
by
Simey Lameze
Committed by
Eloy Lafuente
Mar 09, 2016
Browse files
MDL-50705 auth_db: apply standard cleaning to all fields
Also unit tests were added to cover the new clean_data() method.
parent
4ee7394c
Changes
2
Hide whitespace changes
Inline
Side-by-side
auth/db/auth.php
View file @
d9d8e9c3
...
...
@@ -314,6 +314,7 @@ class auth_plugin_db extends auth_plugin_base {
$updateuser
=
new
stdClass
();
$updateuser
->
id
=
$user
->
id
;
$updateuser
->
suspended
=
1
;
$updateuser
=
$this
->
clean_data
(
$updateuser
);
user_update_user
(
$updateuser
,
false
);
$trace
->
output
(
get_string
(
'auth_dbsuspenduser'
,
'auth_db'
,
array
(
'name'
=>
$user
->
username
,
'id'
=>
$user
->
id
)),
1
);
}
...
...
@@ -400,6 +401,7 @@ class auth_plugin_db extends auth_plugin_base {
$updateuser
=
new
stdClass
();
$updateuser
->
id
=
$olduser
->
id
;
$updateuser
->
suspended
=
0
;
$updateuser
=
$this
->
clean_data
(
$updateuser
);
user_update_user
(
$updateuser
);
$trace
->
output
(
get_string
(
'auth_dbreviveduser'
,
'auth_db'
,
array
(
'name'
=>
$username
,
'id'
=>
$olduser
->
id
)),
1
);
...
...
@@ -422,6 +424,7 @@ class auth_plugin_db extends auth_plugin_base {
$trace
->
output
(
get_string
(
'auth_dbinsertuserduplicate'
,
'auth_db'
,
array
(
'username'
=>
$user
->
username
,
'auth'
=>
$collision
->
auth
)),
1
);
continue
;
}
$user
=
$this
->
clean_data
(
$user
);
try
{
$id
=
user_create_user
(
$user
,
false
);
// It is truly a new user.
$trace
->
output
(
get_string
(
'auth_dbinsertuser'
,
'auth_db'
,
array
(
'name'
=>
$user
->
username
,
'id'
=>
$id
)),
1
);
...
...
@@ -563,6 +566,7 @@ class auth_plugin_db extends auth_plugin_base {
}
if
(
$needsupdate
)
{
require_once
(
$CFG
->
dirroot
.
'/user/lib.php'
);
$updateuser
=
$this
->
clean_data
(
$updateuser
);
user_update_user
(
$updateuser
);
}
return
$DB
->
get_record
(
'user'
,
array
(
'id'
=>
$userid
,
'deleted'
=>
0
));
...
...
@@ -888,6 +892,30 @@ class auth_plugin_db extends auth_plugin_base {
error_reporting
(
$CFG
->
debug
);
ob_end_flush
();
}
/**
* Clean the user data that comes from an external database.
*
* @param array $user the user data to be validated against properties definition.
* @return stdClass $user the cleaned user data.
*/
public
function
clean_data
(
$user
)
{
if
(
empty
(
$user
))
{
return
$user
;
}
foreach
(
$user
as
$field
=>
$value
)
{
// Get the property parameter type and do the cleaning.
try
{
$property
=
core_user
::
get_property_definition
(
$field
);
$user
->
$field
=
clean_param
(
$value
,
$property
[
'type'
]);
}
catch
(
coding_exception
$e
)
{
debugging
(
"The property '
$field
' could not be cleaned."
,
DEBUG_DEVELOPER
);
}
}
return
$user
;
}
}
auth/db/tests/db_test.php
View file @
d9d8e9c3
...
...
@@ -374,4 +374,77 @@ class auth_db_testcase extends advanced_testcase {
$this
->
cleanup_auth_database
();
}
/**
* Testing the clean_data() method.
*/
public
function
test_clean_data
()
{
global
$DB
;
$this
->
resetAfterTest
(
false
);
$this
->
preventResetByRollback
();
$this
->
init_auth_database
();
$auth
=
get_auth_plugin
(
'db'
);
$auth
->
db_init
();
// Create users on external table.
$extdbuser1
=
(
object
)
array
(
'name'
=>
'u1'
,
'pass'
=>
'heslo'
,
'email'
=>
'u1@example.com'
);
$extdbuser1
->
id
=
$DB
->
insert_record
(
'auth_db_users'
,
$extdbuser1
);
// User with malicious data on the name.
$extdbuser2
=
(
object
)
array
(
'name'
=>
'user<script>alert(1);</script>xss'
,
'pass'
=>
'heslo'
,
'email'
=>
'xssuser@example.com'
);
$extdbuser2
->
id
=
$DB
->
insert_record
(
'auth_db_users'
,
$extdbuser2
);
$trace
=
new
null_progress_trace
();
// Let's test user sync make sure still works as expected..
$auth
->
sync_users
(
$trace
,
true
);
// Get the user on moodle user table.
$user2
=
$DB
->
get_record
(
'user'
,
array
(
'email'
=>
$extdbuser2
->
email
,
'auth'
=>
'db'
));
// The malicious code should be sanitized.
$this
->
assertEquals
(
$user2
->
username
,
'userscriptalert1scriptxss'
);
$this
->
assertNotEquals
(
$user2
->
username
,
$extdbuser2
->
name
);
// User with correct data, should be equal to external db.
$user1
=
$DB
->
get_record
(
'user'
,
array
(
'email'
=>
$extdbuser1
->
email
,
'auth'
=>
'db'
));
$this
->
assertEquals
(
$extdbuser1
->
name
,
$user1
->
username
);
$this
->
assertEquals
(
$extdbuser1
->
email
,
$user1
->
email
);
// Now, let's update the name.
$extdbuser2
->
name
=
'user no xss anymore'
;
$DB
->
update_record
(
'auth_db_users'
,
$extdbuser2
);
// Run sync again to update the user data.
$auth
->
sync_users
(
$trace
,
true
);
// The user information should be updated.
$user2
=
$DB
->
get_record
(
'user'
,
array
(
'username'
=>
'usernoxssanymore'
,
'auth'
=>
'db'
));
// The spaces should be removed, as it's the username.
$this
->
assertEquals
(
$user2
->
username
,
'usernoxssanymore'
);
// Now let's test just the clean_data() method isolated.
// Testing PARAM_USERNAME, PARAM_NOTAGS, PARAM_RAW_TRIMMED and others.
$user3
=
new
stdClass
();
$user3
->
firstname
=
'John <script>alert(1)</script> Doe'
;
$user3
->
username
=
'john%#&~%*_doe'
;
$user3
->
email
=
' john@testing.com '
;
$user3
->
deleted
=
'no'
;
$user3
->
description
=
'<b>A description <script>alert(123)</script>about myself.</b>'
;
$user3cleaned
=
$auth
->
clean_data
(
$user3
);
// Expected results.
$this
->
assertEquals
(
$user3cleaned
->
firstname
,
'John alert(1) Doe'
);
$this
->
assertEquals
(
$user3cleaned
->
email
,
'john@testing.com'
);
$this
->
assertEquals
(
$user3cleaned
->
deleted
,
0
);
$this
->
assertEquals
(
$user3
->
description
,
'<b>A description about myself.</b>'
);
$this
->
assertEquals
(
$user3
->
username
,
'john_doe'
);
// Try to clean an invalid property (fullname).
$user3
->
fullname
=
'John Doe'
;
$auth
->
clean_data
(
$user3
);
$this
->
assertDebuggingCalled
(
"The property 'fullname' could not be cleaned."
);
$this
->
cleanup_auth_database
();
}
}
Write
Preview
Supports
Markdown
0%
Try again
or
attach a new file
.
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment