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

Correct ngrok credential lengths #250

Merged
merged 2 commits into from Jun 2, 2023

Conversation

williamhpark
Copy link
Contributor

@williamhpark williamhpark commented Apr 21, 2023

Resolves: #249

The lengths of the ngrok credentials specified in ngrok/credentials.go seem to be incorrect. The correct lengths should be:

  • Authtoken: 49
  • API key: 49

You can create the secrets using these links:

The incorrect length values don't seem to affect any functionality at the moment, but it's important to record the correct values for future features, e.g. any case where validating extracted credentials is needed.

@williamhpark williamhpark self-assigned this Apr 21, 2023
@williamhpark williamhpark marked this pull request as ready for review April 21, 2023 19:26
@williamhpark williamhpark force-pushed the wpark/249-ngrok-credential-length branch from 8dd5818 to 06f1c34 Compare April 21, 2023 19:30
Copy link
Contributor

@arunsathiya arunsathiya left a comment

Choose a reason for hiding this comment

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

Great spot, @williamhpark! I am not sure why I had entered 43 and 48. 49 does seem to be correct length (confirmed both by checking my old authtoken and API key, and by generating new ones on the ngrok website)

Could we regenerate the new example secrets with make ngrok/example-secrets and add them to these files?

  • plugins/ngrok/credentials_test.go
  • plugins/ngrok/test-fixtures/config.yml

@arunsathiya
Copy link
Contributor

I am not seeing anything on the changelog either that suggests that this is a new change.

@AndyTitu AndyTitu added the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Apr 25, 2023
@williamhpark williamhpark force-pushed the wpark/249-ngrok-credential-length branch from d6514d1 to 8a125ae Compare April 26, 2023 00:38
@williamhpark williamhpark force-pushed the wpark/249-ngrok-credential-length branch from 8a125ae to 2fbd1e8 Compare April 26, 2023 00:39
@williamhpark
Copy link
Contributor Author

williamhpark commented Apr 26, 2023

Great spot, @williamhpark! I am not sure why I had entered 43 and 48. 49 does seem to be correct length (confirmed both by checking my old authtoken and API key, and by generating new ones on the ngrok website)

Could we regenerate the new example secrets with make ngrok/example-secrets and add them to these files?

  • plugins/ngrok/credentials_test.go
  • plugins/ngrok/test-fixtures/config.yml

@arunsathiya Ah thanks for catching that, done!

Copy link
Contributor

@accraw accraw left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

@accraw accraw added waiting-on-sec-review and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels Apr 26, 2023
@SimonBarendse SimonBarendse merged commit bb20d67 into main Jun 2, 2023
4 checks passed
@SimonBarendse SimonBarendse deleted the wpark/249-ngrok-credential-length branch June 2, 2023 07:42
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.

Correct the ngrok credential length fields
5 participants