Facades and is_safe option (0.6.0-beta2)

[piotr-cz]

I'm having problems with using/ understanding the is_safe option in facades.

I've set up the Auth facade like so:

'facades' => [
    'Auth' => [
        'is_safe' => true
    ],
]

and in the template I'm using it by

Hello {{ Auth.user.fullname }}

But I'm facing an ErrorException: Method "fullname" for object "Twig_Markup" does not exist.

Because the Auth.user is actually an Twig_Markup object evaluated to string (see TwigBridge\Extension\Loader\Facade\Caller::__call).

Problem was solved when I've disabled the is_safe option and the User object is fine, I can access it's properties in template. But I have a feeling that it's a workaround and I'm doing it wrong or didn't understand something.

I'd say that safe Facades/ methods should be left as they are and not being wrapped in the Twig_Markup, but this functionality works as intended for the Form Facade.

[barryvdh]

The problem is that in order to make them safe, Twig wraps them in that Markup object. This works for the first call, but kills the chaining. Might need to make a note of that.

[piotr-cz]

I was thinking so, but is_safe option means that output doesn't need to be escaped (and wrapped in Twig_markup)?

Anyway maybe there a way to escape only output of last piece in chain?

[barryvdh]

Yeah I did try something like that but it didn't feel nice. But for most things you don't need to use the is safe option, do you? For example, your user name isn't safe, because it has user input and should be escaped. Or you can explicitly make it safe by using the raw filter. It's mostly useful for form/html helpers but they have safe functions because of extensions.

[piotr-cz]

Good point. Maybe there could be some Laravel facades set up by default in the extensions.php. I took me some time to realize connection between exception and safe option.

[barryvdh]

What facades do you need? There are extensions set up for most common functions, so yo don't need the facades for forms, html, auth, etc.

[piotr-cz]

After switching to 0.6 I've decided to use Facades instead of extensions because this is more similar to 0.5 underscore syntax, Blade syntax and seems more flexible overall (prefer objects than aliases).

I see I can use {{ auth_user().fullname }} too

[barryvdh]

Well, I would think the extensions are actually more similar to the 0.5 facades, in syntax atleast. But Facades are indeed more Blade/Laravel like, but the concept doesnt really exist in the Twig itself. Op 28 jun. 2014 15:32 schreef "Piotr" [email protected]:

After switching to 0.6 I've decided to use Facades instead of extensions because this is more similar to 0.5 underscore syntax, Blade syntax and seems more flexible overall (prefer objects than aliases).

I see I can use {{ auth_user().fullname }} too

— Reply to this email directly or view it on GitHub https://github.com/rcrowe/TwigBridge/issues/120#issuecomment-47427414.

[unitedworx]

i prefer to use facades myself since its more clear that you are calling a facade and not a function.

{{ auth.user.fullname }} is much cleaner and twig like

{{ auth_user().fullname }} is a bit ugly and not so clean!

adding |raw at the end is not a problem but since is_safe can eliminate it mostly with html and input facades i think this is ok.

[barryvdh]

Yeah in that case indeed. But you could also use the View::share or view composers /creators to inject the user object directly. Possibilities are endless, just what you prefer :) Op 3 jul. 2014 21:39 schreef "Paris Paraskeva" [email protected]:

i prefer to use facades myself since its more clear that you are calling a facade and not a function.

{{ auth.user.fullname }} is much cleaner and twig like

{{ auth_user().fullname }} is a bit ugly and not so clean!

adding |raw at the end is not a problem but since is_safe can eliminate it mostly with html and input facades i think this is ok.

— Reply to this email directly or view it on GitHub https://github.com/rcrowe/TwigBridge/issues/120#issuecomment-47975239.

[barryvdh]

This is perhaps a similar situation to: https://github.com/thephpleague/plates/issues/24 Perhaps that solution could be combined with the idea here: https://github.com/rcrowe/TwigBridge/issues/68#issuecomment-38257043 to make it a bit more reliable.

Let's see how that works out for Plates.

[mmodler]

I'm facing a similar situation right now. With barryvdh/twigbridge i had this working piece of twig logic in my template:

{% set foundDate = Carbon.create(2003, 8, 19, 0) %}
{% set foundYears = foundDate.diffInYears %}
....

Now i get

Method "diffInYears" for object "Twig_Markup" does not exist in...

Yes i know i can insert Carbon to my view in many other ways, but i would prefer a smart twigbridge solution. It feels like a step back right now with v0.6

With this config in extensions.php i would expect that i can use all carbon methods without limitations:

'Carbon' => [
    'is_safe' => true
],

Any ideas to handle the security feature a little smarter?

[mmodler]

Next thing with "Twig_Markup" security. In my tpl i tried to use dump (with twigbridge debug true):

{{ dump(foundDate) }}

Result:

An exception has been thrown during the rendering of a template ("Argument 1 passed to twig_var_dump() must be an instance of Twig_Environment, instance of Twig_Markup given") in

[barryvdh]

With my bridge, the behavior was probably similar to is_safe => false.

@rcrowe What you think about the is_safe option. We should either make it behave as expected (by creating a more advanced Twig_Markup wrapper) or remove the option, imho.

[mmodler]

Atm its very confusing, i digged a little deeper into it with autoescape = true:

'facades' => [
...
    'Form' => [],
...
],

I would expect that every form html output would be escaped as long as i don't mark some methods or the whole facade with is_safe = true.

What happens in Caller.php is: is_save is false, the methods are not wrapped with Twig_Markup but their output is raw, unescaped html!

My general thoughts on this feature:

• I like the idea of "whitelisting" facades and/or methods to increase security a little
• "is_safe" is completely missleading because wo need to take case of two things: availability/accessability of facades/methods and escaping settings for facades/methods
• For availability i would prefer very intuitive config definitions like allowed, forbidden, only, except, perhaps with * support
• For escapeing the default should be twig.environment.autoescape and then i would like to enable/disable it for methods perhaps like 'autoescape' => ['getFoo', 'doBar'] or 'raw' => ['doFoo', 'getBar']

[barryvdh]

When I call {{ Form.open() }}, with is_safe = false, I get the escaped html. When is_safe = true, the html is parsed as raw html. So that is as was expected. Did you set your global config to disable autoescaping perhaps?

is_safe is based on the default Twig is_safe functions, which wraps functions in a Twig_Markup object, but this doesn't handle chaining methods.

[mmodler]

I only had Form [] withour any is_safe setting. So i would expect that my autoescaping = true was considered, but all html output was raw.

[barryvdh]

So in your config/packages/rcrowe/twigbridge/twig.php, you have environment.autoescape => true? And in your extensions.php just 'Form'? And it doesn't matter what you do with is_safe, they both render the form controls, instead of showing the html as plain text? Strange, when I leave out the is_safe option, the plain html is shown instead of rendered.

[mmodler]

Ah its too hot here - i have used the raw filter in my form view.

But in general, what is the purpose of is_safe? Minimizing the risk that template designers do something stupid? If so, raw should not work in tpls i think?

[barryvdh]

is_safe is for XSS protection. So if I have a function show_username($name), Twig doesn't know if it should trust $name (because it could be from the user), so unless you specify that you know that show_username only receives safe input, or the input is checked in the output so that it's safe anyways, the output is escaped.

[mmodler]

I always have autoescaping true and use raw when i'm sure what i'm doing - this is the most fail-proof approach in my opinion.

So when should i need is_safe? I can't think of a real use case?

[barryvdh]

If you're too lazy to use |raw ;)

[mmodler]

:) For security considerations i prefer whitelisting over blacklisting.

So with is_safe = true for a method i would get the same output as |raw - but i won't have the possibility to decide it depending on use case in the twig tpl, right?

And what about making 1 method of 5 accessable or 1 of 5 not accessable, how is this done?

[barryvdh]

Yes, and you can't chain methods.

That's not possible. And that's also why I'm not a favor of adding this to facades. Imho the correct way is to create an extension, where you have more flexibility to define what is safe and what isn't.