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

Don't add a dummy API key to every new Rails app #28524

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

claudiob
Copy link
Member

Every new Rails app is currently generated with Rails.application.secrets[:api_key] set to 123.

This comes from a line in config/secrets.yml that, in my opinion, should be left commented out to only serve as a syntax example, rather than being actually set in every Rails app.

Additionally, we might want to give a better example than 123, since in the same file we are suggesting to:

Make sure the secret is at least 30 characters and all random,
no regular words or you'll be exposed to dictionary attacks.

The result of this commit is that config/secrets.yml will include something like:

# Shared secrets are available across all environments.

# shared:
#   api_key: f56930851993982510d5bd9236f4108f6fe7c15448f1c6923a51872e0dbae1a24d274b318abb6518b540dfb51079c61640885f607467e5ed1053849be7587d61

rather than:

# Shared secrets are available across all environments.

shared:
  api_key: 123

shared:
api_key: 123
# shared:
# api_key: <%= app_secret %>
Copy link
Member

Choose a reason for hiding this comment

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

I'd use a human readable example here since that Make sure the secret is at least 30 characters and all random comment is about the app secret not about a shared api key.

Every new Rails app is currently generated with
`Rails.application.secrets[:api_key]` set to `123`.

This comes from a line in `config/secrets.yml` that, in my opinion,
should be left commented out to only serve as a syntax example, rather
than being actually set in every Rails app.

Additionally, we might want to give a better example than `123`, since
in the same file we are suggesting to

> Make sure the secret is at least 30 characters and all random,
> no regular words or you'll be exposed to dictionary attacks.

The result of this commit is that `config/secrets.yml` will include something like:

```yaml
 # Shared secrets are available across all environments.

 # shared:
 #   api_key: f56930851993982510d5bd9236f4108f6fe7c15448f1c6923a51872e0dbae1a24d274b318abb6518b540dfb51079c61640885f607467e5ed1053849be7587d61
```

rather than this:

```yaml
 # Shared secrets are available across all environments.

 shared:
   api_key: 123
```
@claudiob
Copy link
Member Author

@rafaelfranca That makes sense. Let me know if you like this better, or I can just leave as 123.

@rafaelfranca rafaelfranca merged commit ce2bfd8 into rails:master Mar 22, 2017
rafaelfranca added a commit that referenced this pull request Mar 22, 2017
Don't add a dummy API key to every new Rails app
@rafaelfranca
Copy link
Member

Backported in 1f1c30d

@claudiob claudiob deleted the comment-api-key branch March 23, 2017 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants