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

Username is not sanitized on registration #955

Closed
apps-caraga opened this issue Feb 1, 2023 · 6 comments
Closed

Username is not sanitized on registration #955

apps-caraga opened this issue Feb 1, 2023 · 6 comments
Assignees

Comments

@apps-caraga
Copy link
Contributor

apps-caraga commented Feb 1, 2023

I'm using dbAuth for a project and one issue I encounter is that the username is not sanitized even if the sanitation middleware is active. For example, a user can input usernames with html tags such as <h1>bigname or <marquee>runningname</marquee> and this gets inserted to the database as-is. I'm using the sanitation middleware per basic example. Any idea how to sanitize username on registration?

SanitationMiddleware config

'middlewares'=>'sanitation,dbAuth,authorization',
'sanitation.tables'=>'all',
'sanitation.handler' => function ($operation, $tableName, $column, $value) {
	return is_string($value) ? strip_tags($value) : $value;
},

The middleware sanitizes/strips html tags from other inputs during updating but not on user creation.

@mevdschee mevdschee self-assigned this Feb 1, 2023
@mevdschee
Copy link
Owner

The SanitationMiddleware uses the types it reflects from the tables to sanitize. These types are not available for the dbAuth calls. Maybe the dbAuth middleware should do it's own sanitizing? What do you think?

@apps-caraga
Copy link
Contributor Author

The SanitationMiddleware uses the types it reflects from the tables to sanitize. These types are not available for the dbAuth calls.

I see. I thought the SanitationMiddleware handles all tables. Anyway, for username, it is usually a string so maybe we don't need to reflect or check the column type from the database table.

Perhaps it would be enough to check if the nominated username is a correctly formatted email address (thru FILTER_VALIDATE_EMAIL) as some users seems to prefer to use their email address as username.

If it's not an email address, check if it contains only the characters @,_.-,.letters and numbers, but should always start with a letter. Check also on the minimum and maximum length to prevent super-long usernames. Will these be ok as default username validation?

@apps-caraga
Copy link
Contributor Author

Hi @mevdschee , I checked my PR#911 and I think some commits on that can be applied here. I just don't know how to unbundle the multiple commits on that PR if it can be done. Still learning my way around github. 😢

Particularly, these changes. It sets new properties, $usernameMinLength, $usernameMaxLength, and $usernamePattern.

image

@mevdschee
Copy link
Owner

mevdschee commented Mar 22, 2023

I'm using dbAuth for a project and one issue I encounter is that the username is not sanitized even if the sanitation middleware is active. [...] Any idea how to sanitize username on registration?

First of all, I agree and this is something that dbAuth should somehow support.

a user can input usernames with html tags such as <h1>bigname or <marquee>runningname</marquee> and this gets inserted to the database as-is

Whether that is a desirable or not depends on your application.

Perhaps it would be enough to check if the nominated username is a correctly formatted email address

For some applications that is true.

If it's not an email address, check if it contains only the characters @,_.-,.letters and numbers, but should always start with a letter. Check also on the minimum and maximum length to prevent super-long usernames.

For some applications that is desirable..

I think we need some clever applying of SanitationMiddleware to the dbAuth calls.. I just don't know how yet.

@apps-caraga
Copy link
Contributor Author

apps-caraga commented Mar 27, 2023

On html tags in username,

Whether that is a desirable or not depends on your application.

I think it would be good to have a sane default, for example by excluding html tags in usernames. If for some reason html tags in username is desirable in the application, the application should purposely override the default.

we need some clever applying of SanitationMiddleware to the dbAuth calls

I agree and I also can't figure it out yet. 😁

@apps-caraga
Copy link
Contributor Author

Just a quick note, in case anyone else is having same problem:
It is possible to use the customization beforeHandler to do simple sanitation of the username. So after loading the sanitation middleware, we can detect if the current $path is register, then do whatever sanitation check you need to do before sending the $username to the endpoint. In the following simple example, we're just stripping any html tags in the submitted username.

'customization.beforeHandler' => function ($operation, $tableName, $request, $environment) {
			$body = $request->getParsedBody();
			$path = RequestUtils::getPathSegment($request, 1);
			if($path === 'register'  ){
				$body->username = strip_tags($body->username); // assuming the $usernameColumnName is username
				return $request->withParsedBody($body);
			}
		},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants