selvinortiz / craft.patrol Goto Github PK

Hey Selvin,

We've had a client (and a few other people, including myself) notice the new yellow line on the front-end while logged in with Patrol enabled. It's caused some confusion.

I'd vote for it to be removed or at least to become a setting that is off by default. For people who aren'te expecting Patrol to do this, it seems to surprise us as like a yellow line that somehow got added to the stylesheet!

PatrolService.php line 114-122
body {border-top: 5px solid #fc0;}

When the Patrol plugin is installed in a Craft 2.4 instance and the yiic console command is issued (craft/app/etc/console/yiic), the following error is thrown:

CHttpRequest is unable to determine the request URI. in /home/teverts/web/mysite.dev/craft/app/etc/errors/ErrorHandler.php (95) exception 'CException' with message 'CHttpRequest is unable to determine the request URI.' in /home/teverts/web/mysite.dev/craft/app/framework/web/CHttpRequest.php:554 Stack trace: #0 /home/teverts/web/mysite.dev/craft/app/framework/web/CHttpRequest.php(320): CHttpRequest->getRequestUri() #1 /home/teverts/web/mysite.dev/craft/plugins/patrol/services/PatrolService.php(45): CHttpRequest->getUrl() #2 /home/teverts/web/mysite.dev/craft/plugins/patrol/services/PatrolService.php(29): Craft\PatrolService->protect(Array) #3 /home/teverts/web/mysite.dev/craft/plugins/patrol/PatrolPlugin.php(44): Craft\PatrolService->watch(Array) #4 /home/teverts/web/mysite.dev/craft/app/services/PluginsService.php(138): Craft\PatrolPlugin->init() #5 /home/teverts/web/mysite.dev/craft/app/etc/console/ConsoleApp.php(80): Craft\PluginsService->loadPlugins() #6 /home/teverts/web/mysite.dev/craft/app/framework/base/CApplication.php(169): Craft\ConsoleApp->init() #7 /home/teverts/web/mysite.dev/craft/app/framework/YiiBase.php(121): CApplication->__construct('/home/teverts/w...') #8 /home/teverts/web/mysite.dev/craft/app/etc/console/yiic.php(78): YiiBase::createApplication('Craft\ConsoleAp...', '/home/teverts/w...') #9 /home/teverts/web/mysite.dev/craft/app/etc/console/yiic(4): require_once('/home/teverts/w...') #10 {main}www-data@slave5:/home/teverts/web/mysite.dev

The Patrol.json import is great, but right now this requires that you store your settings file in the Patrol plugin folder. I'm not sure how this will play out when plugins are able to be auto-updated and the plugin folder might be over-written, or when someone might copy the Patrol plugin from one project to another and overlook a custom settings file being present.

Would you consider supporting placing the Patrol.json file in the default /craft/config folder? Maybe Patrol just checks both locations (config and plugin directory) for a settings file. I think my vote would be to only have the settings file only exist in the config folder. This places Patrol's settings alongside the Craft overrides and Redactor settings and let's the Patrol plugin standalone without any project specific settings in it.

The utility buttons on the settings page are very useful to make the primary use cases easy to implement. I have no issue with the way they work, though thought I would share my initial reactions to some of the language as I feel the following changes would make the interface a bit more clear:

I feel it is not obvious what the +Authorize My IP button does. "To Authorize" is an action in itself, where the action taking place on the settings page is actually the addition of an IP address to the list of IPs that will be authorized. I think it would be more intuitive if the "+Authorize my IP" button was labeled "+Add My IP".

Another personal preference is with the "Secure The CP" button. If space isn't an issue, maybe it's worth using the full phrase "Secure the Control Panel". I think "CP" is pretty recognized language, but prefer to use full phrases where possible so there is no confusion. I'm not sure how CP would translate to other languages and would probably be harder for someone coming from a different language to look up if Patrol hadn't been translated into a particular language yet.

Importing settings from a file may not work in some environments where $_FILES may be manipulated differently by Craft or certain permissions are required.

I'll look into this as soon as I get some free time...

I would love a way to enforce a main domain for an environment where you have multiple domains connected, eg. domain.com and www.domain.com.

Before, I handled this in .htaccess, but I feel that this requires a unnecessary performance hit, however small.

Do you think this would be something that fits within the realm of Patrol?

I'm currently using Hearty to handle environments.

Hey selvin,

Just wanted to get your thoughts about putting together an open source craft plugin guide conventions.

For example I have been naming all my repo's craft-name and i am starting to see different conventions like your craft.name. I thinks its important name are all consistant to make it clear its a craftcms plugin. I also would like to make basic readme template and guide for naming.

Im thinking a document that is very simple like https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-1-basic-coding-standard.md but not focused on coding standard but plugin structure. Of course we would roll in any future requirements P&T add when the store launches.

Let me know what you think and Ill send you a google docs to collab on. Also invite ben to see if he is interested.

Luke.

Steps

1. Install Craft and the Patrol plugin on an instance
2. Create a craft migration file in the craft_app/migrations folder
3. Attempt to run ./craft migrate --interactive=0
4. Observe that Patrol expects a getUrl method on the current request in src/services/PatrolService.php in the watch() method.

Expected

Patrol should not watch console requests.

It'd be great to have the plugin hooked up to a release feed to stay up to date with changes.

Switching an existing site over to https and so far Patrol has made it way easier. I'm just wondering, though - is the redirect like a 301 .htaccess redirect or do I need to do something else to let search engines know that the pages have moved?

The plugin docs specifically say "When maintenance mode is on, you will see a yellow border at the top of the page so that you don't forget to turn it off after testing." I have never turned maintenance mode on, but I still see the yellow bar across the top of my site's web pages. I would expect to only see the bar when in maintenance mode. I am using the secure connections feature, but I don't think that should activate the yellow bar. Am I missing something? I'll gladly give more info about my setup if needed. Thanks!

Hi there! First off, awesome plugin. Exactly what I needed.

I'm trying to configure Patrol in my settings file so I won't have to worry about it accidently getting turned on / off. I've got multiple environments going. When I set set the forceSsl directive to true, it doesn't care about the pages I have listed in restrictedAreas, it forces the whole site to https.

If I remove my patrol settings from general.db and do it in the admin tab it works as I expect. Any idea what's going on here?

Here's my settings file:

return array(
  '*' => array(
    'errorTemplatePrefix' => 'errors/',
    'patrol' => array(
      'forceSsl' => false,
      'enableCpTab' => true,
      'restrictedAreas' => array(
        '/{cpTrigger}',
        '/join-us'
      )
    )
  ), # *

  '.dev' => array(
    'devMode' => true,
    'siteUrl' => 'http://example.dev',
    'imageDriver' => 'imagick',
    'environment' => 'local'
  ), # .dev

  '.org' => array(
    'siteUrl' => 'https://example.org',
    'environment' => 'production',
    'imageDriver' => 'imagick',
    'generateTransformsBeforePageLoad' => true,
    'patrol' => array(
      'forceSsl' => true
    )
  )  #.org
);

And a quick side note, I think the name Restricted Areas is a little misleading. Sounds like pages I want to keep users away from. I would maybe call it Secured Areas or Secured Pages. Seems a little more clear.

Let me know if you need anything else from me to debug. Thanks again!

A frequent scenario for a site in development is:

• multiple admins are working on site
• a dev (me) will needs to shut everyone out temporarily while I pull the database down locally and do something.

Is there anyway to achieve this with Patrol? It seems like admins can always log in, but I guess I'm looking for a way to block everyone (obviously this would need to be done via config and not in the CP).

Hi, looking at using Patrol thanks to it's environment settings. Very useful.
One thing that would be great if it could be implemented is the ability to generate a url with a code so that it can be sent to someone to access whilst the sites offline.
IP Whitelist for Craft does this. Typically we want to keep our staging sites in maintenance mode, but it would be great if we could send a client a link that adds there IP to the permitted list.

Hey Selvin,

I just tried to forceSsl on a site and ran into a few issues.

There is a type in a variable on line 30 of this file:
https://github.com/selvinortiz/craft.patrol/blob/master/services/PatrolService.php

This:
$this->getEnvSetting('foreceSsl')

Should be:
$this->getEnvSetting('forceSsl')

On top of that, as I understand it, the same line has the following logic:
if ($settings->getAttribute('forceSsl') && $this->getEnvSetting('foreceSsl'))

Which means that SSL will not be forced unless we have the forceSsl setting set to true AND have the forceSsl environment setting set to true. I feel the priority should first be given to the general.php override and second to the setting in the CP (or the setting in the CP should reflect the value set by the override in general.php).

Finally, enabling the env setting and the CP setting works. However, I can't seem to override the setting in my local general.php file. Patrol continues to read the primary general.php as the one it takes it's forceSsl setting from for me locally.

It might be good for Patrol to not do any SSL redirects if devMode is on and/or if the IP address matches 127.0.*

Since 2.0.2524, plugins can have their own config.php file instead of sharing with general.php.
Would be nice to get set these in their own config.php, (then overridable by a user craft/config/patrol.php)

Hey Guys,
I just pushed the latest version of Patrol (0.9.4 RC3) and would love to get some feedback if you've gotten a chance to install it and play with it.

This is the last release before going into public beta so if you've found any bugs, questionable behavior, unintuitive features, typos or anything else... please create an issue or send me an email ([email protected]) so I can get to it before going into public beta.

Thank you so much, your feedback is very much appreciated!

ATT: @takobell @lukeholder @keithmancuso @nicholasleblanc @BenParizek @putyourlightson

Hey there

Just enabled the forceSSL option and am now getting this error across my site:

It seems to be removing the trailing /. Can anyone confirm that this is happening to them? Have I not configured something correctly ?

Hey.
I have my page on a subdomain to test the site on the server, but since the plugin uses the getServerName() it is redirected to the main domain. Any possibility to get it to use HTTP_HOST instead or another fix for it?

Hi, just saw this today pop up today.

Saving the Patrol configuration page in the Craft CP, when there is an array of authorised IP addresses set in general.php causes an error:

trim() expects parameter 1 to be string, array given

/vagrant/craft/plugins/patrol/services/PatrolService.php(253)

241         return $this->dynamicParams;
242     }
243 
244     /**
245      * Parses authorizedIps to ensure they are valid even when created from a string
246      *
247      * @param array|string $ips
248      *
249      * @return array
250      */
251     public function parseAuthorizedIps($ips)
252     {
253         $ips = trim($ips);
254 
255         if (is_string($ips) && ! empty($ips))
256         {
257             $ips = explode(PHP_EOL, $ips);
258         }
259 
260         return $this->filterOutArrayValues(
261             $ips,
262             function ($val)
263             {
264                 return preg_match('/^[0-9\.\*]{5,15}$/i', $val);
265             }

Hey it's me again :)

It's just been brought to my attention that linking to a non craft directory in my root is causing redirect loops (i.e. https://fortysevenmedia.com/tuts/css3buttons/) where tuts is a folder in the root not part of craft. I can load individual files by linking directly to them, but linking to a directory with an index.html causes a redirect loop. Any ideas why this would happen?

Using it on Craft 2 - I have added new users in a user group which has the following permissions
Under General
Access Patrol - Ticked

Under Patrol
Access the site when maintenance is on - Ticked

I have enabled it so the plugin is available from the side menu.

The user is not an Admin.

When the user clicks the Patrol link on the side menu it says:

Unauthorized
You don’t have the proper credentials to access this page.

Can non Admins access the patrol page?

If I install a fresh copy of Patrol and just turn on the Secure Connections setting and try to save (with no restricted areas, 1 restricted area, or 2 restricted areas) I get the following error:

Argument 1 passed to Craft\PatrolService::filterOutArrayValues() must be of the type array, string given, called in /path/craft/plugins/patrol/services/PatrolService.php on line 222 and defined

Hi. I just tried your great plugin for my server for a maintenance operation. But I can still login and use the CP with a user profile that should not be able to do it (Patrol checkbox is not clicked in profile rights).
Did I forget someting to add in the config files ?