phetioAPIChangeCheck requires authentication

zepumph commented 3 years ago

phetioAPIChangeCheck was created in https://github.com/phetsims/perennial/issues/181

Since phet-io api jsons are now password protected (which they weren't when this feature was added), phetioAPIChangeCheck fails, as @jessegreenberg found while trying to publish GFL 2.2 for phet-io. The error he encountered during the auto MR process was:

Error: Failure with production deploy for gravity-force-lab to 2.2: StatusCodeError: 401 - "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML 2.0//EN\">\n<html><head>\n<title>401 Unauthorized</title>\n</head><body>\n<h1>Unauthorized</h1>\n<p>This server could not verify that you\nare authorized to access the document\nrequested.  Either you supplied the wrong\ncredentials (e.g., bad password), or your\nbrowser doesn't understand how to supply\nthe credentials required.</p>\n</body></html>\n"
    at Function.deployProduction (C:\Users\Jesse\Documents\Development\phetsims-secondary\perennial\js\common\Maintenance.js:818:17)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

I'm not sure how to navigate this generally. For this particular deploy, I will remove this check.

Ways forward:

Make the API json file public
Add a way to access private files from the production.js grunt task.
Something else?

zepumph commented 3 years ago

Tagging @samreid too.

zepumph commented 3 years ago

For this particular deploy, I will remove this check.

I forgot this is in perennial, so I can't do this this just for GFL. Still for now I will do this.

samreid commented 3 years ago

I'm not sure which of the proposed ways forward (make the API files public or add authentication for our process) is more promising. A third alternative which seems less promising is to have the build process check out and build the old version. This seems fragile and like it would slow things down, but it circumvents the access problems.

zepumph commented 3 years ago

@samreid and I want to use basic authentication through headers using request-promise-native (what we are using to request stuff currently).

Like this!


Index: js/grunt/phetioAPIChangeCheck.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/phetioAPIChangeCheck.js b/js/grunt/phetioAPIChangeCheck.js
--- a/js/grunt/phetioAPIChangeCheck.js  (revision cc938d2e36e24e647206074b63b50e36c7b46680)
+++ b/js/grunt/phetioAPIChangeCheck.js  (date 1614199466249)
@@ -48,7 +48,12 @@
   const latestVersionString = `${latestVersion.versionMajor}.${latestVersion.versionMinor}.${latestVersion.versionMaintenance}`;

   const latestDeployedURL= `https://phet-io.colorado.edu/sims/${repo}/${latestVersionString}/${phetioAPIFileName}`;
-  const latestDeployedVersionAPI = JSON.parse( await request( latestDeployedURL ) );
+  const latestDeployedVersionAPI = JSON.parse( await request( {
+    uri: latestDeployedURL,
+    headers:{
+      Authentication: `Basic fdjskafleaiopfejsaiofds` // base64 encoding like username:password
+    }
+  } ) );

   const builtVersionAPI = JSON.parse( grunt.file.read( builtVersionAPIFile ) );

See https://www.npmjs.com/package/request-promise#get-something-from-a-json-rest-api for doc

Steps for this issue:

Create a "machine" account or something, which all devs will need to add to their build-local, with their password".
Like: buildLocal.phetioUsernamePassword: 'username:password',
Test
Add this script back in.

High priority but not blocking sounds good to us.

zepumph commented 1 year ago

I deleted this file today. I think we would rewrite it if we ever need this code again. We have greatly changed our philosophy about API changes between versions since 2021. Closing

phetsims / perennial

phetioAPIChangeCheck requires authentication #209