search

wpsharks / s2member-kb

The s2Member® and s2Member® Pro Knowledge Base
9 stars 4 forks source link

Testing s2Member Access Control with User Switching #285

Closed renzms closed 8 years ago

renzms commented 8 years ago
KB Article Creation Checklist

KB Article Published @ s2member.com
:page_with_curl: See: Testing s2Member Access Control with User Switching

:octocat: View Markdown File | :pencil2: Edit Markdown File

jaswrks commented 8 years ago

@renzms Great work on this! I see you closed the other issue in favor of this one. Awesome!

I'm skimming over it real quick to see if I can spot any areas of improvement. I'll leave some quick comments here as I do.

The current work around

Check for consistency with this word. "work around" should be one word in all places; i.e., workaround. I'm guilty of spelling this wrong myself, so I just looked it up. haha

Logging in or Logging out to switch between users

"Logging out" does not need a capital L within the sentence.

Using the User Switching Plugin

You quoted something from their plugin I believe. I suggest making it clear that you are quoting what their plugin says before you introduce the quote, and then also provide a link to the source of that quote.

For instance, right before the quote you might have:

The developers of this plugin write...
Source: <URL here>

Go to User > Add New User,

Please use the → symbol for all software and/or Dashboard paths. Just for consistency in our articles.

For more information please see: [s2If /] Simple Shortcode Conditionals How do WordPress Roles/Capabilities work? Video: Custom Capabilities for WordPress

I suggest making that a set of list items in Markdown.

For more information please see:
- [s2If /] Simple Shortcode Conditionals
- How do WordPress Roles/Capabilities work?
- Video: Custom Capabilities for WordPress

Note also that any instance of a WP shortcode must always be encapsulated by backticks to explicitly declare that it is code that should not be rendered once this is published at s2member.com.

For more information please see:
- `[s2If /]` Simple Shortcode Conditionals
- How do WordPress Roles/Capabilities work?
- Video: Custom Capabilities for WordPress
renzms commented 8 years ago

@jaswsinc

Thanks for review!

Check for consistency with this word. "work around" should be one word in all places; i.e., workaround. I'm guilting of spelling this wrong myself, so I just looked it up. haha

What's funny is I spelled it correctly the first time around. Lol

Note also that any instance of a WP shortcode must always be encapsulated by backticks to explicitly declare that it is code that should not be rendered once this is published at s2member

Right, I almost forgot s2Member is actually running s2Member on the site!

Changelog:
- corrected spelling of "workaround"
- de-capitalisation of logging
- User Switching explanation quoted and cited
- Adjusted to use the `→` symbol for all software and/or Dashboard paths.
- Made set list of items for other KBA links
- added " ` " to make sure `[s2If]` shortcode is not rendered on site when published
renzms commented 8 years ago

@raamdev Ready for review, thanks.

raamdev commented 8 years ago

@renzms Nice! This looks like an article that will get referenced frequently. :-)

I did an initial review of your draft. Here's some feedback. Please try to review your drafts carefully for spelling/grammar mistakes and organizational flow and layout. Each section needs to be more well-rounded.

Testing s2Member Level Access / CCaps Enabled using User Switching

Please change the title to "Testing s2Member Access Control with User Switching" (in the GitHub issue and the YAML markup).

The following methods allows

Should be "The following method allows".

s2Member admins

Please use "site owners"; that's the term we use throughout our KB articles when referring to the owner of the website administering s2Member. If you say "s2Member admins", it might be misunderstood as "WebSharks/s2Member support reps".

as one of their customers

Please use "users" or "subscribers" when referring to the site owners users; saying "customers" can be confusing.

ccaps.

Please use "Custom Capabilities" instead of ccaps when referring to Custom Capabilities. The only time you should use it in a code style is when referring to the shortcode attribute itself (ccaps="") or when referring to a specific (or example) Custom Capability (e.g., pro or pro,unlimited).

you can easily test and switch between different users with different membership level or

Should be "levels".

Using the User Switching Plugin

This section is a bit unorganized. It starts with a list of bullet points by the plugin developer without a sufficient introduction of the User Switching plugin itself. I would spent a few more sentences summarizing why the User Switching plugin is useful and how it can be used.

I would also preface the list of security-related things with something like, "Is the User Switching plugin safe? Do you need to know users passwords? Here's what the User Switching plugin developers have to say about security:"

The paragraphs that explain how to use the User Switching plugin are separated from the screenshots, which makes it feel like you're just repeating yourself with the screenshots. I suggest creating a H3 section in there with something like "How to Use the User Switching Plugin" that outlines the steps.

Custom Code Method WARNING - MU plugin file for testing purposes only.

The double-headings look wrong. Maybe use bold text for the warning instead of a header.

The Custom Code Method section also has no introduction; it starts with a path to a file, which just doesn't make sense. Each section should start with a sentence or paragraph introducing what the section is about.

NOTE We highly suggest using the plugin over the custom solution. It is likely to offer better security and it will be easier to use.

This should go somewhere near the very top, in the introduction. And it shouldn't be a "NOTE"; it should just be a sentence in its own paragraph.

renzms commented 8 years ago

@raamdev , Updated according to your notes.

Thanks!

Please change the title to "Testing s2Member Access Control with User Switching" (in the GitHub issue and the YAML markup).

Should be "The following method allows".

Please use "site owners"; that's the term we use throughout our KB articles when referring to the owner of the website administering s2Member. If you say "s2Member admins", it might be misunderstood as "WebSharks/s2Member support reps".

Please use "users" or "subscribers" when referring to the site owners users; saying "customers" can be confusing.

Please use "Custom Capabilities" instead of ccaps when referring to Custom Capabilities. The only time you should use it in a code style is when referring to the shortcode attribute itself (ccaps="") or when referring to a specific (or example) Custom Capability (e.g., pro or pro,unlimited).

Should be "levels".

This section is a bit unorganized. It starts with a list of bullet points by the plugin developer without a sufficient introduction of the User Switching plugin itself. I would spent a few more sentences summarizing why the User Switching plugin is useful and how it can be used.

The double-headings look wrong. Maybe use bold text for the warning instead of a header.

This should go somewhere near the very top, in the introduction. And it shouldn't be a "NOTE"; it should just be a sentence in its own paragraph.

raamdev commented 8 years ago

@renzms Nice! Here's some more feedback:

short code

  • [ ] Should be 'shortcode'.

Current Workaround

  • [ ] I would consolidate this section into a paragraph at the top, as part of the intro. We don't need a whole separate section for this. Also, no need to make a list--just summarize the steps in a single sentence. Users coming to this article will likely already be familiar with this tedious process, which is why they're looking for another way of doing it.

Using the User Switching Plugin, you can easily test and switch between

  • [ ] Link to the 'User Switching' plugin here as well. Also, it should be "User Switching plugin", not Plugin with a capital P.

Features Security

  • [ ] In the Features and Security lists that you quote from the User Switching plugin developers, remove those headers ('Features' and 'Security'). Just the list items themselves should be quoted.

Using the User Switching Plugin

  • [ ] This should just be "About the User Switching Plugin", as your next header is called "How to Use the User Switching Plugin".

How to Use the User Switching Plugin

  • [ ] Add a short introduction to this section, one or two sentences that summarize how to use the plugin. Also, convert the list items in this section to an ordered list (1., 2., etc.). These appear to be steps to follow, so bullets don't really make sense here. (Note: To make images part of a list item, add 5 spaces in front of the image markup.)

Common concerns of users may be:

  • [ ] Add an H3 header right above this called "Is it safe?"

Common concerns of users may be:

  • [ ] The person reading this article is "the user", so this sentence doesn't sound right. Address the reader ("concerns you may have"), not 'users'.

To switch back, you can switch back via the Dashboard Notification or the Admin Bar.

  • [ ] The screenshot here has a big red bar through something. Please remove this. If you need to hide the name of something before taking a screenshot, just use the Chrome inspector to edit the markup and change it to something else (e.g., "Example Admin").
raamdev commented 8 years ago

@jaswsinc My feeling is that we shouldn't even be posting the MU Plugin example ("Custom Code Method" in this article) in a KB Article on our website. In this case, that seems like a bad idea from a general web security perspective (i.e., makes it too easy for a novice site owner to try using and end up with a potential security hole). I recommend that we move that code to a GitHub Gist and convert the entire "Custom Code Method" section into a little note at the bottom of this article that says something like "If you're a developer and you need more control over user switching, take a look at this method that uses an MU-Plugin.". Thoughts?

renzms commented 8 years ago

I recommend that we move that code to a GitHub Gist and convert the entire "Custom Code Method" section into a little note at the bottom of this article that says something like "If you're a developer and you need more control over user switching, take a look at this method that uses an MU-Plugin.". Thoughts?

@raamdev @jaswsinc

I agree with Raam, I think it would be better to remove that section altogether. I see how it can be useful for a developer as a starting point for some other custom solution they may have, but the point of the article was mainly to simplify the testing process via the User Switching plugin.

jaswrks commented 8 years ago

@raamdev writes...

I recommend that we move that code to a GitHub Gist and convert the entire "Custom Code Method" section into a little note at the bottom of this article that says something like "If you're a developer and you need more control over user switching, take a look at this method that uses an MU-Plugin.". Thoughts?

@renzms writes...

I agree with Raam, I think it would be better to remove that section altogether. I see how it can be useful for a developer as a starting point for some other custom solution they may have, but the point of the article was mainly to simplify the testing process via the User Switching plugin.

Great points. That sounds like a plan to me; i.e., moving it to a Gist or excluding it entirely.

renzms commented 8 years ago

@raamdev

Ready for review, again!

Thanks for tips especially the following:

(Note: To make images part of a list item, add 5 spaces in front of the image markup.)

If you need to hide the name of something before taking a screenshot, just use the Chrome inspector to edit the markup and change it to something else (e.g., "Example Admin").

As for the Custom Code section, what is the final decision on that?

Checklist

short code

  • [x] Should be 'shortcode'.

Current Workaround

  • [x] I would consolidate this section into a paragraph at the top, as part of the intro. We don't need a whole separate section for this. Also, no need to make a list--just summarize the steps in a single sentence. Users coming to this article will likely already be familiar with this tedious process, which is why they're looking for another way of doing it.

Using the User Switching Plugin, you can easily test and switch between

  • [x] Link to the 'User Switching' plugin here as well. Also, it should be "User Switching plugin", not Plugin with a capital P.

Features Security

  • [x] In the Features and Security lists that you quote from the User Switching plugin developers, remove those headers ('Features' and 'Security'). Just the list items themselves should be quoted.

Using the User Switching Plugin

  • [x] This should just be "About the User Switching Plugin", as your next header is called "How to Use the User Switching Plugin".

How to Use the User Switching Plugin

  • [x] Add a short introduction to this section, one or two sentences that summarize how to use the plugin. Also, convert the list items in this section to an ordered list (1., 2., etc.). These appear to be steps to follow, so bullets don't really make sense here. (Note: To make images part of a list item, add 5 spaces in front of the image markup.)

Common concerns of users may be:

  • [x] Add an H3 header right above this called "Is it safe?"

Common concerns of users may be:

  • [x] The person reading this article is "the user", so this sentence doesn't sound right. Address the reader ("concerns you may have"), not 'users'.

To switch back, you can switch back via the Dashboard Notification or the Admin Bar.

  • [x] The screenshot here has a big red bar through something. Please remove this. If you need to hide the name of something before taking a screenshot, just use the Chrome inspector to edit the markup and change it to something else (e.g., "Example Admin").
raamdev commented 8 years ago

and create your test Users.

  • [x] Change this to "and create one or more test users with the appropriate s2Member Level and Custom Capabilities (optional)."

Is it safe?

  • [x] Change this to "Is User Switching Safe?" and move that entire section underneath "How to Use the User Switching Plugin"; it makes more sense for it to come after.

About the User Switching Plugin

  • [x] Change this to "The User Switching Plugin"

I removed the Custom Code Method section and updated that for you.

renzms commented 8 years ago

I removed the Custom Code Method section and updated that for you.

@raamdev

Okay, thanks and update for review!

raamdev commented 8 years ago

@renzms I'm marking this Approved as of 2016-03-10. Thank you! I'll do some final editing and publish this soon. :-)