Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#3040]Add script to make all app URLs https. #3100

Merged
merged 2 commits into from
May 21, 2019

Conversation

stellanl
Copy link
Contributor

Before the PR (what is the issue or what needed to be done)

S3 will stop supporting unencrypted fetches.

The solution

Iterate over app URLs and change http: to https:

Screenshots (if appropriate)

Checklist

  • Connect the issue
  • Test plan
  • Copyright header
  • Code formatting
  • Documentation

@stellanl stellanl requested a review from valllllll2000 May 20, 2019 12:14
String newurl = oldurl.replaceFirst("http:", "https:");
String appcode = (String) da.getProperty(APPCODE_PROP);
if (!newurl.equals(oldurl)
// && appcode.equals(APPCODE_VALUE) //Only modern apps?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we maybe remove this commented out code or is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was something I wanted feedback on; should we change only the URLs with the modern code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I have seen there is also fieldSurvey... no idea, who uses those anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

satStat is no longer used in the latest versions of the app

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think fieldSurvey used to be the name of the app. But since the old versions will become unreachable if we do not modify all URLs, I propose to simply change all of them.

$APP_ID \
$SERVICE_ACCOUNT \
$P12_FILE_PATH \
$2 $3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it necessary to pass $3? if I understand correctly $1 is the instance name, $2 is the [--doit] param, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cut-and-paste, obviously.

Copy link
Contributor

@valllllll2000 valllllll2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@valllllll2000 valllllll2000 merged commit fc8966b into develop May 21, 2019
@valllllll2000 valllllll2000 deleted the issue/3040-app-urls-https branch May 21, 2019 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants