Use default NGROK_AUTH_TOKEN env variable

[khryniewicz]

Description This PR modifies the PyngrokConfig class in pyngrok/conf.py to use an environment variable for the auth_token default value. This change is helps with ngrok now requires an AUTH_TOKEN, and without it, an error is thrown pyngrok.exception.PyngrokNgrokError: The ngrok process errored on start: authentication failed: Usage of ngrok requires a verified account and authtoken..

Issues

• None

Type of Change

• [x] New feature (non-breaking change which adds functionality)

Testing Done Manual testing was done to ensure that the application works as expected with the new changes.

Checklist

• [x] My code follows the PEP 8 style guidelines for Python
• [x] I have performed a self-review of my own code
• [x] I have commented my code in particularly hard-to-understand areas
• [ ] If applicable, I have made corresponding changes to the documentation
• [ ] I have added tests that prove my change is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes
• [x] My changes generate no new warnings

[alexdlaird]

1) a default value of None would need to be given (you can do os.environ.get("NGROK_AUTHTOKEN", None) to set a default. But 2) I don't think this is necessary, as the auth token can already be set several different ways, and os.environ["NGROK_AUTHTOKEN"] could be used just as easily there.

https://pyngrok.readthedocs.io/en/latest/index.html#setting-the-authtoken

[khryniewicz]

The method already returns None as default

>>> os.environ.get("DOESNT_EXIST")
>>> os.environ.get("DOESNT_EXIST") == None
True
>>> os.environ.get("DOESNT_EXIST", None) == None
True

I guess you add the None param for readability if it's not clear already.

The benefit of this change is that if the auth token is already in the .env under the standard name it doesn't have to be explicitly set in the code at all.

Some other repos setting up the default key / token like that:

• OpenAI
• Twilio
• Official ngrok

[alexdlaird]

Oh okay, I do like that approach then. I can make a similar change in the java-ngrok library. Can you take a look at why none of the tests pass with this change though? This would also need new tests of its own to be merged, for instance I think there are already tests around setting the auth_token in the config (because it was allowed to be None before)—those can probably stay, they're fine as a regression, but let's have a test around this that checks it gets set from the env.

Also, throughout the test code elsewhere we use the env var NGROK_AUTHTOKEN, could we use that instead for the var name (no second underscore)?

And thanks for the None default remind, totally forgot .get() already does that for us. Great!

[alexdlaird]

Here is the same change in the java-ngrok library, let's make the functionality (and documentation, unit test(s)) identical between the two libraries, if you could update this PR accordingly. Thanks!

https://github.com/alexdlaird/java-ngrok/pull/85/files

[alexdlaird]

This functionality can be found in 7.0.4.

https://github.com/alexdlaird/pyngrok/releases/tag/7.0.4

[khryniewicz]

Sorry for the late reply, and thank you for merging.

The tests aren't failing because of this change, but because of changes in ngrok itself. They now require authentication to use ngrok at all. You can see the error messages in the pipeline:

ERROR:  authentication failed: Usage of ngrok requires a verified account and authtoken.
ERROR:  
ERROR:  Sign up for an account: https://dashboard.ngrok.com/signup
ERROR:  Install your authtoken: https://dashboard.ngrok.com/get-started/your-authtoken
ERROR:  
ERROR:  ERR_NGROK_4018
ERROR:  

I'm sure the same error will appear if you rerun the workflow that was passing before this change.

One way to resolve this would be to create an account specifically for this repo, get the authtoken, and add it as a secret to be used in GitHub Actions. With these change it will not require other changes to code to set the token in tests.

[alexdlaird]

Yes, I realized that after I commented, they seemed to have changed that on or around December 20th. That's why I went and merged it, the tests work fine against main.

I already have an ngrok auth token as a secret, that's why the build pipeline otherwise works. Trouble is, GitHub doesn't shared secrets for Actions to Pull Requests (they used to), as they determined it's too for someone to create a fork, open a pull request, and feed off secrets to a third party with malicious code. If you use pull_request_target the secrets will be used, but I haven't done that due to this security concern, and I haven't come up with a better approach as of yet. As it stands, the pipeline still does validate the entire suite on the develop branch, so that's good enough for now. Until I better isolate the tests / just run a subset of the tests that don't require the auth token for PRs, this will continue to be an issue, but not top of mind for me right now as I have workarounds.