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

Remove secrets #188

Merged
merged 2 commits into from
Apr 23, 2019
Merged

Remove secrets #188

merged 2 commits into from
Apr 23, 2019

Conversation

jameslzhu
Copy link
Member

@jameslzhu jameslzhu commented Apr 19, 2019

Implements #175.
Removes the recaptcha site_key, secret_key from plaintext, loads from the secrets.yml.
I'm not sure if there are any other secrets, a quick grep for "key" doesn't pull up anything.

Of note, this causes user registration to fail for developers without access to the recaptcha secret keys; I'm not sure if there's a good way around this, or if we can disable recaptcha validation in dev environments.

I've judged this to be an issue not worth fixing, because developers will be making admin users from the command line anyways, and probably rarely from the web interface. (Changes to the recaptcha will only be focused on the new site, anyways.)

@jameslzhu jameslzhu requested a review from jvperrin April 19, 2019 02:24
@jameslzhu jameslzhu mentioned this pull request Apr 19, 2019
2 tasks
Copy link
Member

@jvperrin jvperrin left a comment

Choose a reason for hiding this comment

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

Sounds good to me, I think we need to either purge the secret from history or reissue it though so that it's not actually sensitive any more.

@jvperrin
Copy link
Member

Ah, I see you mentioned that in #175 already, and opted not to rewrite history, which sounds great to me if the secret can be rotated effectively. I also can't find any other credentials, but I haven't done a very comprehensive search either, just looked through the initializers and grep'd for a few keywords.

@jameslzhu jameslzhu merged commit e959fbe into compserv:migrate Apr 23, 2019
@jameslzhu jameslzhu deleted the secrets branch April 23, 2019 03:33
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.

None yet

2 participants