Add API Flag That Instructs s2Member to Require and Validate Email Address on create_user

[patdumond]

Reference WP Sharks Forum Topic: https://forums.wpsharks.com/t/pro-api-for-remote-operations-is-creating-users-with-out-a-valid-email/1801.

User reports that the Pro API for Remote Operations is not validating email addresses. He can create Users without an email address and Users with an invalid address format (no @ symbol, etc...).

[raamdev]

@patdumond Thank you for cross-referencing that issue here. We'll get this tested soon.

[renzms]

Bug Confirmed

@raamdev / @jaswrks

Invalid emails / blank email fields, still allow users to be created via Pro API for Remote Operations.

Tested Using

WordPress Version: 4.7.3
Current WordPress Theme: Twenty Seventeen version 1.1
Theme Author: the WordPress team - https://wordpress.org/
Theme URI: https://wordpress.org/themes/twentyseventeen/
Active Plugins: s2Member Version 170221 + s2Member Pro v170221
PHP Version: 7.1.3-3+deb.sury.org~xenial+1
MySQL Version: 10.0.29-MariaDB-0ubuntu0.16.04.1

• scripts from s2Member Pro → API / Scripting → Pro API For Remote Operations

Tested with invalid email:

User is created successfully

Tested with no email

User is created successfully

[jaswrks]

I realize this can be seen as a bug, but it's actually a feature. The function underlying this API call is wp_create_user, which uses wp_insert_user. Neither of these require that you enter an email address, or even that it's a valid email address.

Would you really want to insert a user with an invalid email address? Or no email address?

No, not likely. There are very few cases where that would be desirable.

However, for the sake of avoiding unnecessary validation and to allow for easier programmatic access, WordPress does very little validation. The API for both WordPress and s2Member are often designed to maximize flexibility, which is different from a UI form where validation is necessary.

The expectation is that a developer will do their own validation and the API will stay out of your way as best it can — just doing what you tell it to do.

[krumch]

Sorry to join this thread without invitation...

Imagine a case: someone creates a user by API, and grants some privileges (if possible, but with s2M API should be possible), because forms don't allow to set privileges, they are monitored closely. Later someone uses that account to crack the site.

And this is not "just a case"... I spend last week to investigate this "in person", while clean a site from a very vital virus. This site runs s2M Pro with 12 levels, two "ghost accounts" without emails was set to level 6, which is not free...

So be aware. For me it's better to patch that hole.

[jaswrks]

Sorry to hear that. But what 'hole' are you referring to, exactly? Was there a problem with the API calls being made in the application, or are you stating that you feel s2Member should be patched in some way?

[krumch]

NP, that happens...

I think s2M API should test incoming values like they are form values. Thus to not allow to exploit "the feature". I understand that this adds limitations, but increases security.

[seancon2]

I beg to differ, wp_create_user & wp_insert_use may not do the validation but In a generic basic install of WordPress it won't let you duplicate an email, enter a malformed email or leave it blank so there is validation. The api should be able to use this as well. All of the error responses seem to be in place so it should be usable. -- Sean On March 29, 2017 8:55:34 AM EDT, jaswrks <notifications@github.com> wrote: I realize this can be seen as a bug, but it's actually a feature. The function underlying this API call is [wp_create_user](https://developer.wordpress.org/reference/functions/wp_create_user/), which uses [wp_insert_user](https://developer.wordpress.org/reference/functions/wp_insert_user/). Neither of these require that you enter an email address, or even that it's a valid email address. Would you really want to insert a user with an invalid email address? Or no email address? No, not likely. There are very few cases where that would be desirable. However, for the sake of avoiding unnecessary validation and to allow for easier programmatic access, WordPress does very little validation. The API for both WordPress and s2Member are often designed to maximize flexibility, which is different from a UI form where validation is necessary. The expectation is that a developer will do their own validation and the API will stay out of your way as best it can — just doing what you tell it to do.

…

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #1072 (comment)

[jaswrks]

Thank you, to you both.

I'm not convinced this is a security issue at all though.

To clarify further, there is already validation in place. It's just not catching an empty or invalid email address. That's because WordPress doesn't require an email address to create an account, and it also doesn't require a valid email address to create an account.

Most UI forms for WordPress are designed to require one, but that doesn't mean that the same set of rules should be applied to an API call. In fact, even in WordPress core the wp_insert_user() function is specifically designed not to trigger an error in that scenario, and that is intentional.

Why? Because while not prevalent, there are use cases for having no email address, and even for using fake, bogus, or otherwise invalid email addresses. This is something that some developers specifically use the s2Member Pro Remote OPs API for in fact; e.g., to create or sync accounts with another application that is outside of WordPress. An email is not always applicable to the application on the other end. So in some cases, they will not send one, or they will just make one up.

So I think the s2Member API, like wp_insert_user(), should not require the email address or perform any additional validation above and beyond what already exists in WordPress core. Doing so may lead to problems for existing installations of s2Member out-in-the-wild, or prevent someone from completing an integration they need.

Just to note also... the lack of an email address, or having an invalid email address, is not a security issue. On a site that's been FUBARd, ghost accounts will often be found that have no email address, but that's a symptom and not the cause; and not a security hole so far as I have seen or heard about. In s2Member, if a user doesn't have an email address that's not great, but for the purpose of using s2Member's API to insert rows into the user DB, not having an email address is harmless.

[jaswrks]

Tip: If you're using WordPress to make the API connection, the is_email() function can be used to perform your own validation before POSTing data to the API.

Or, if you're outside of WordPress, you can use this in native PHP code.

$email = 'bogus';

if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
    exit('Invalid email address.');
}

Referencing: http://php.net/manual/en/filter.filters.validate.php

[krumch]

OK, we, the developers, can build our filters. How to deal with others, intruders, who use the fact that there is not need validation in the (otherwise perfectly working) API?

Sorry, but for me is a security hole, as a user can not be created manually (by forms) without email and at level6.

[jaswrks]

@krumch The s2Member Pro Remote OPs API requires a developer like us to integrate it, and any and all connections to the s2Member Pro Remote OPs API also require your API key.

See: Dashboard → s2Member → API / Scripting → Pro API For Remote Operations

So the security associated with the API instance running on your server will be as secure as your API key is, and then as secure as the code that you write, which is what interacts with the API. If you want to make sure a valid email address has been given, just be sure to run your validations before you POST data to the API using the create_user action.

[seancon2]

I was the original person that brought this up in the forum, someone else brought it here. I Never felt it was a security issue, although I would prefer a little less s2member referencing in the code, I'll save that for the forum. The flexibility of s2member is its biggest advantage but My point was and still is that the api would be expected to follow the basic functionality of WordPress in regards to programmatically creating users. I'm not sure in what instance not having a valid email is in a scenario for a membership site on the Internet but if you say it exists then it must exist. I would like to see the api natively validate emails in the same way it validates the username and I don't think this is unnecessary but a necessary validation. As you can see by the tests there is something going on because the api creates the account without an email even though one was submitted even if it does not recognize the field as a valid email address or a duplicate email. So something says... This email isn't correct and I am not adding it to the database but I'm not reporting it. I believe the api is not going far enough. If it won't be corrected because 1 out of 1,000,000 that wants empty emails in their membership site I'll look into adding my own validation.

…

-- Sean On March 29, 2017 11:15:24 AM EDT, jaswrks <notifications@github.com> wrote: @krumch The s2Member Pro Remote OPs API requires a developer like us to integrate it, and any and all connections to the s2Member Pro Remote OPs API also require your API key. See: Dashboard → s2Member → API / Scripting → **Pro API For Remote Operations** So the security associated with the API instance running on your server will be as secure as your API key is, and then as secure as the code that you write which makes the API call. If you want to make sure nobody can create an account through the API, then just be sure to run your validations before you POST data to the API.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #1072 (comment)

[jaswrks]

Thanks for the follow-up. I will leave this open for further discussion/consideration. I don't see any reason why we can't add a flag that tells s2Member to validate the email and make that an option.

[jaswrks]

follow the basic functionality of WordPress in regards to programmatically creating users

Just to point out again. That is precisely what we are doing. https://developer.wordpress.org/reference/functions/wp_insert_user/

[seancon2]

And I'll point out wordpress will NOT allow you to do this in the normal registration process. Email address is expected to be there and at least following an accepted format. Just because wp_create_user and wp_insert_use don't actually validate emails does not mean it's not done. I'll bet they don't do the username validation either, I have never had reason to look. I'll assume this means it will remain unchanged?

-- Sean

On March 29, 2017 12:24:31 PM EDT, jaswrks ***@***.***> wrote: follow the basic functionality of WordPress in regards to

programmatically creating users Just to point out again. That is _precisely_ what we are doing. https://developer.wordpress.org/reference/functions/wp_insert_user/

…

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #1072 (comment)

[jaswrks]

And I'll point out wordpress will NOT allow you to do this in the normal registration process.

See: #1072 (comment)

I'll bet they don't do the username validation either.

Username validation is critical, and yes, that is performed.
See: https://developer.wordpress.org/reference/functions/wp_insert_user/

[jaswrks]

I'll assume this means it will remain unchanged?

See: #1072 (comment)

[seancon2]

Very good, I'll look for that flag in an upcoming release. Good debate ;) -- Sean

On March 29, 2017 12:41:47 PM EDT, jaswrks ***@***.***> wrote: I'll assume this means it will remain unchanged?

See: #1072 (comment)

…

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #1072 (comment)

[seancon2]

a ran across this plugin and i am using it for regular membership registrations, we were getting some misspelled domain names and it checks for an MX record. the plugin has been working with the normal registration process and mailgun has an email validation API. https://www.mailgun.com/ . might be something that could be implemented? says it works with any form that use is_email(). might be as simple as adding is_email() to the API?

[seancon2]

need some direction, I added some code to require the email address and verify it is formatted correctly with mailgun, that is working fine. what I need help with is... when the email is submitted, I get a success message. the problem is if this is a new API registration and the email address is in use, the email field in the database is blank. if there is an existing user and the updated email is already in use, the existing email is left unchanged.

what I need to have happen is the API to return an error and stop the insert/update IF the email is already in use.

where would be the best place to start in s2member code to insert this check?

Thanks

Sean