heroku / legacy-cli

Heroku CLI
https://cli.heroku.com
MIT License
1.36k stars 381 forks source link

Use heroku.com domain for SSL-Doctor #2041

Closed rdsharma closed 7 years ago

rdsharma commented 7 years ago

herokuapp.com address has been deprecated.

rdsharma commented 7 years ago

@dickeyxxx can we please get this looked at ASAP? We are planning on doing the switchover within a couple weeks.

cc @heroku/secdev @da3da1us

jdx commented 7 years ago

this code shouldn't be in use anymore cc @ransombriggs

garyposter commented 7 years ago

@rdsharma (quote from @ransombriggs) "that has been ported to v3 and fixed in v3, so unless someone has a setup that has not transitioned at all from the Ruby code, they should not hit this." Do we need to address this in the v2 code as well?

/cc @da3da1us

rdsharma commented 7 years ago

@garyposter if you look at this search you can see that there are a bunch of people who have imported this code: https://github.com/search?utf8=%E2%9C%93&q=https%3A%2F%2Fssl-doctor.herokuapp.com&type=Code&ref=searchresults

I haven't actually looked through all the results to determine which ones are using the v2 code, but I can tell you that I can see requests still coming in against the herokuappp.com subdomain:

Dec 05 00:51:51 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=f08d6a55-dcf2-4d95-9e8a-b4819e63cbed fwd="103.68.105.254" dyno=web.1 connect=0ms service=33ms status=200 bytes=5433 
Dec 05 00:52:06 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=33f52d42-26ef-4c80-bd70-0b0004e4c871 fwd="103.68.105.254" dyno=web.1 connect=0ms service=32ms status=200 bytes=5433 
Dec 05 01:35:49 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=ee27f8f5-efcb-40ff-9b88-1dd72f46ab22 fwd="171.5.241.80" dyno=web.1 connect=0ms service=73ms status=422 bytes=295 
Dec 05 06:08:37 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=92289fe5-1de7-4fb7-8792-5635cc6865e1 fwd="104.236.6.150" dyno=web.1 connect=0ms service=48ms status=200 bytes=6011 
Dec 05 08:07:56 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=94996e87-309c-4a57-b643-9268b75bc47b fwd="104.236.6.150" dyno=web.1 connect=0ms service=48ms status=200 bytes=6031 
Dec 05 09:57:29 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=214cfc53-a748-4467-8e96-a93b6cbd0c10 fwd="83.144.101.238" dyno=web.1 connect=1ms service=184ms status=200 bytes=5387 
Dec 05 09:57:38 ssl-doctor heroku/router:  at=info method=POST path="/resolve-chain-and-key" host=ssl-doctor.herokuapp.com request_id=317980f5-02e6-4116-a42e-03308de500bf fwd="83.144.101.238" dyno=web.1 connect=2ms service=194ms status=200 bytes=5387 

While of course updating the v2 code won't explicitly fix this, I feel like it's easier for support to tell our customers that they can just update their Heroku dependencies and their code will start working again, rather than support telling them they have to switch to v3 entirely? The fix is only a one line string constant change, but I can understand why you would rather avoid updating an abandoned legacy library. Please let me know what you think.

garyposter commented 7 years ago

Thanks for the data @rdsharma.

On the one hand, I think we're cool with merging and releasing. I'm verifying with @ransombriggs and @dickeyxxx that we can just go ahead and do this.

On the other hand, IMO we actually really don't want Support encouraging people to stick with v2. Keeping them on the old codebase will cause more support problems down the road, when we could remind them to migrate now.

The reason I'm cool with merging and releasing is because I want customers who encounter troubles migrating to be able to have a way to stick with the old code in an emergency. In that case, Support should emphasize that the affected customers are using old, unsupported code, and they should make an effort to migrate ASAP.

Do you buy that? If so, will you be the one advising support on this, or should we reach out?

I believe Ransom and Jeff are out for the day, and I'm out now, so this may not wrap up till tomorrow.

rdsharma commented 7 years ago

@garyposter sounds good to me. I was planning on talking to support soon to give them a heads up of the sunset of the herokuapp.com subdomain anyways, I'll make sure I mention the v2 vs v3 part as well. Thanks!

jdx commented 7 years ago

do you guys mean CLI v3/ CLI v5? or do you mean the api version? If you mean the CLI, this won't likely fix anything since those requests are most likely coming from users that haven't updated their CLIs at all, so shipping a new version won't result in anything changing for them. Definitely fine with releasing, but I don't expect anything to change.

garyposter commented 7 years ago

@dickeyxxx yes, we are talking CLI, but this is about support.

AIUI, Security is going to kill the old URL within a week or two. That will stop the problem!

But Security wants Support to be able to have a good answer to give customers who write in to complain about the CLI after the URL has been shut down. Our strongly preferred answer is that they upgrade the CLI to the Node version. If they can't for some reason, perhaps because they are rarely but importantly using an old local Ruby plugin, then Security wants Support to have a short-term, unsupported option of using this old code, while they migrate.

Thanks to @ransombriggs who has now merged and is making a release.

ransombriggs commented 7 years ago

@rdsharma like @dickeyxxx this may not actually fix those issues, but went ahead and released because it would at least put them on an easy path to a fix when it is fully deprecated.

rdsharma commented 7 years ago

Thanks @ransombriggs @garyposter @dickeyxxx !