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.

Sign up for GitHub

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

Jump to bottom

phetioAPIChangeCheck requires authentication #209

Closed
zepumph opened this issue Feb 24, 2021 · 5 comments
Closed

phetioAPIChangeCheck requires authentication #209

zepumph opened this issue Feb 24, 2021 · 5 comments
Assignees
@zepumph
Labels
dev:phet-io priority:2-high

Comments

@zepumph
Copy link
Member

zepumph commented Feb 24, 2021

phetioAPIChangeCheck was created in #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 zepumph self-assigned this Feb 24, 2021
@zepumph
Copy link
Member Author

zepumph commented Feb 24, 2021

Tagging @samreid too.

@zepumph
Copy link
Member Author

zepumph commented Feb 24, 2021

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.

zepumph added a commit that referenced this issue Feb 24, 2021
@zepumph
remove phetioAPIChangeCheck for now, #209
cc938d2
@zepumph zepumph mentioned this issue Feb 24, 2021
Closed
@samreid
Copy link
Member

samreid commented Feb 24, 2021

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
Copy link
Member Author

zepumph commented Feb 24, 2021

@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:

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

High priority but not blocking sounds good to us.

@zepumph
Copy link
Member Author

zepumph commented Mar 3, 2023

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

@zepumph zepumph closed this as completed Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zepumph zepumph

Labels
dev:phet-io priority:2-high
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@samreid @zepumph