Fix valet share over TLS secure #156

RafaelPlantard · 2016-09-19T20:27:16Z

Now when we have a link secured over TLS the command valet share will create other link for an unsecure link and this unsecure link will be used for ngrok.

When the ngrok url is closed that unsecure link is destroyed again.
The stack for this PR is at #148.
Please, see this: @adamwathan @drbyte

This folder is generated by PhoStorm IDE.

Check if a given domain in valet is secure or not.

Now when we have a link secured over TLS the command `valet share` will create other link for an unsecure link and this unsecure link will be used for ngrok. When the ngrok url is closed that unsecure link is destroyed again.

Fix valet share over TLS secure

drbyte · 2016-09-19T21:44:00Z

cli/valet.php

+            info('The ['.$item.'] site has been secured with a TLS certificate.');
+        }
+    }
+})->descriptions('List all secured domain with a trusted TLS certificate');


"secured domain" should probably be "secured domains"

Thanks for your point

What do you think about it?
valet secured: Returns a list with all secured domains.
or
valet secured: Try the following syntax command: valet secured [domain]

While looking at your additions of secured [domain] and secures it strikes me that now there are 4 variants of secure in valet commands, and this might not be a good thing.

secure [domain]

unsecure [domain]

secured [domain]

secures

That feels like it may breed confusion.

Is there a way to just call isSecured without needing the secures/secured commands?

I guess, it's possible I remove secures command, Can I continue with secured [domain] command?

I already removes valet secures.

drbyte · 2016-09-19T21:45:49Z

I guess it would become more clear when actually using valet share that the temporary (unsecured) link it's creating for foo.dev is valet.foo.dev.

This method check if a given url is secured over TLS or not.

valet secured [domain]: Will check if a url or the current path is secured over TLS. valet secures: List all secured sites.

Once that the valet.php secured now returns YES or NO, instead of a string when the domain is secured.

Update valet share command over TLS

RafaelPlantard · 2016-09-19T23:19:05Z

I had update some things on my code to better readability.

For prevent some breed confusion.

Removes valet secures

drbyte · 2016-09-20T15:06:45Z

Question: doesn't unlinking the temporary domain break all the ngrok activity, since all the serving it's doing happens after the unlink?

RafaelPlantard · 2016-09-20T15:33:30Z

@drbyte I will test it. But I guess will don't break, once that unlink command it's executed only after the ngrok is stopped.

RafaelPlantard · 2016-09-20T15:49:45Z

Hey @drbyte the behavior of my implementation link and unlink is the same of the current valet implementation, briefing the unlink is just made after ngrok is stopped (Ctrl + C).

drbyte · 2016-09-20T17:49:26Z

Oh, right. The script stays running until ngrok is stopped. ;)

Given that using https requires ngrok pro, I'm okay with this workaround ... for non-pro anyway.

I don't have an ngrok pro account to test any of this with, but I'm guessing that those who do might want it to still serve secured, instead of bypassing it with this.

RafaelPlantard · 2016-09-20T17:54:51Z

I bought a PRO version just for tests all commands using TLS, but not works, then I mail to its founder, and his answer was:

Hey Rafael -

Yeah, that means your application is redirecting to shop.app. 
Usually this means you need to instruct your application to accept requests from a
different hostname or at the very least to not issue a redirect. 
There's unfortunately not much ngrok can do to help with this

- alan
Founder, ngrok.com & equinox.io

drbyte · 2016-09-20T18:51:05Z

@RafaelPlantard Do you know how to do a rebase of all your commits here, so that the PR is just a clean single commit?

RafaelPlantard · 2016-09-20T20:11:23Z

@drbyte When PR is done there is an option to do it. I guess is here.

RafaelPlantard · 2016-09-21T17:19:47Z

@drbyte @adamwathan @taylorotwell there is some date to accept/refuse this PR?

KorvinSzanto · 2016-09-21T20:41:13Z

@RafaelPlantard is correct, github has built in squashing @drbyte

drbyte · 2016-09-21T20:43:37Z

Yes, it does, but squashing your own commits is a much nicer way to play. Anyway, it was just a question, leading to a suggestion.

adamwathan · 2016-09-24T14:52:19Z

Hey folks,

So my instinct with this one is to just not support valet share with locally secured domains.

If that feature of ngrok requires a paid ngrok account, I think the easiest thing to do is just output a warning that lets the user know the site can't be shared unless they unsecure it.

My reasoning is just that if you are actually using TLS locally, there's probably a reason for it. And if there's a reason for it (like your won't function properly without it), then sharing the site unsecured through another alias is just going to cause confusion when it doesn't work properly for the person it's being shared with.

If someone has a paid ngrok account that does support this, I think I would just recommend they use ngrok directly instead of relying on Valet to create the share URL.

Again I think creating the unsecured alias silently and sharing that will just lead to more bug reports and confusion. Better to tell the user they need to unsecure manually before sharing, so if their app actually requires TLS to work properly, it'll be very obvious to them why they might have issues with the share URL.

Sorry!

msonowal · 2016-10-13T06:39:29Z

same issue here valet share does not work if the valet secure is applied for the project

adamwathan · 2016-10-13T12:12:19Z

@msonowal Yup as you can read in my comment directly above yours, it's a known limitation.

To version: 2.1.18

This makes it possible to still access the site via http if it is secure This touches laravel#382, laravel#148, laravel#156, laravel#504

Rafael da Silva Ferreira and others added 4 commits September 19, 2016 15:11

Ignore .idea folder

2cdf161

This folder is generated by PhoStorm IDE.

Add a command to check TLS certificate in a domain

295fec3

Check if a given domain in valet is secure or not.

Modify valet share behavior

6946102

Now when we have a link secured over TLS the command `valet share` will create other link for an unsecure link and this unsecure link will be used for ngrok. When the ngrok url is closed that unsecure link is destroyed again.

Merge pull request #3 from Swift-Yah/feature/fix-valet-share

d487b14

Fix valet share over TLS secure

drbyte reviewed Sep 19, 2016

View reviewed changes

Rafael da Silva Ferreira and others added 4 commits September 19, 2016 20:12

Add isSecured method to Site class

88ed556

This method check if a given url is secured over TLS or not.

Add valet secured and secures commands

9521021

valet secured [domain]: Will check if a url or the current path is secured over TLS. valet secures: List all secured sites.

Update checks for valet.php secured

8769a6c

Once that the valet.php secured now returns YES or NO, instead of a string when the domain is secured.

Merge pull request #4 from Swift-Yah/feature/valet-secured

745cb9d

Update valet share command over TLS

Rafael da Silva Ferreira and others added 2 commits September 20, 2016 10:04

Removes valet secures

f9eb8ad

For prevent some breed confusion.

Merge pull request #5 from Swift-Yah/feature/remove-secures-command

2c781e4

Removes valet secures

Update Ngrok

2e8f61d

To version: 2.1.18

driesvints mentioned this pull request Dec 17, 2016

Add a note about share not supporting secure sites laravel/docs#2923

Merged

adamwathan closed this Feb 20, 2017

sjelfull added a commit to sjelfull/valet that referenced this pull request Mar 10, 2018

Add —not-redirected argument to secure command

b86812c

This makes it possible to still access the site via http if it is secure This touches laravel#382, laravel#148, laravel#156, laravel#504

sjelfull mentioned this pull request Mar 10, 2018

Add —not-redirected argument to secure command #534

Closed

drbyte mentioned this pull request Sep 18, 2018

Site domains different than test #599

Closed

lukastom mentioned this pull request Apr 24, 2023

Both Mailhog and Mailpit Do Not Work in version 1.6.6 genesisweb/valet-linux-plus#98

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix valet share over TLS secure #156

Fix valet share over TLS secure #156

RafaelPlantard commented Sep 19, 2016

drbyte Sep 19, 2016

RafaelPlantard Sep 19, 2016 •

edited

Loading

RafaelPlantard Sep 19, 2016 •

edited

Loading

drbyte Sep 20, 2016 •

edited

Loading

RafaelPlantard Sep 20, 2016 •

edited

Loading

RafaelPlantard Sep 20, 2016

drbyte commented Sep 19, 2016 •

edited

Loading

RafaelPlantard commented Sep 19, 2016

drbyte commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

drbyte commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

drbyte commented Sep 20, 2016 •

edited

Loading

RafaelPlantard commented Sep 20, 2016 •

edited

Loading

RafaelPlantard commented Sep 21, 2016 •

edited

Loading

KorvinSzanto commented Sep 21, 2016

drbyte commented Sep 21, 2016 via email

adamwathan commented Sep 24, 2016

msonowal commented Oct 13, 2016

adamwathan commented Oct 13, 2016

Fix valet share over TLS secure #156

Fix valet share over TLS secure #156

Conversation

RafaelPlantard commented Sep 19, 2016

drbyte Sep 19, 2016

Choose a reason for hiding this comment

RafaelPlantard Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

RafaelPlantard Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

drbyte Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

RafaelPlantard Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

RafaelPlantard Sep 20, 2016

Choose a reason for hiding this comment

drbyte commented Sep 19, 2016 • edited Loading

RafaelPlantard commented Sep 19, 2016

drbyte commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

drbyte commented Sep 20, 2016

RafaelPlantard commented Sep 20, 2016

drbyte commented Sep 20, 2016 • edited Loading

RafaelPlantard commented Sep 20, 2016 • edited Loading

RafaelPlantard commented Sep 21, 2016 • edited Loading

KorvinSzanto commented Sep 21, 2016

drbyte commented Sep 21, 2016 via email

adamwathan commented Sep 24, 2016

msonowal commented Oct 13, 2016

adamwathan commented Oct 13, 2016

RafaelPlantard Sep 19, 2016 •

edited

Loading

RafaelPlantard Sep 19, 2016 •

edited

Loading

drbyte Sep 20, 2016 •

edited

Loading

RafaelPlantard Sep 20, 2016 •

edited

Loading

drbyte commented Sep 19, 2016 •

edited

Loading

drbyte commented Sep 20, 2016 •

edited

Loading

RafaelPlantard commented Sep 20, 2016 •

edited

Loading

RafaelPlantard commented Sep 21, 2016 •

edited

Loading