Skip to content

remove API_TOKEN_SALT from the env table #805

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
Merged
merged 3 commits into from
Apr 25, 2022
Merged

remove API_TOKEN_SALT from the env table #805

merged 3 commits into from
Apr 25, 2022

Conversation

@stb13579
Copy link
Contributor

What does it do?

  • Adds APP_KEYS to the environmental variable table
  • fixes a minor spelling error

Why is it needed?

Describe the issue you are solving.

Related issue(s)/PR(s)

#770

@vercel
Copy link

vercel bot commented Mar 18, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/strapijs/documentation/GnxgneLTbSzWB81J3LwbDH8UYPRj
✅ Preview: https://documentation-git-dev-env-update-strapijs.vercel.app

@stb13579 stb13579 requested review from petersg83 and pwizla March 18, 2022 11:03
@stb13579 stb13579 marked this pull request as ready for review March 18, 2022 11:04
@stb13579 stb13579 added source: Dev Docs PRs/issues targeting the Developer Docs internal PRs created by the Strapi core team target: v4 Documentation PRs/issues targeting content from docs.strapi.io (main branch). size: small If the PR includes only 1 file with <10 lines of text or a small code fix labels Mar 18, 2022
petersg83
petersg83 suggested changes Mar 18, 2022
View reviewed changes
docs/developer-docs/latest/setup-deployment-guides/configurations/optional/environment.md Outdated
| `ENV_PATH` | Path to the file that contains your environment variables | `String` | `'./.env'` |
| `STRAPI_PLUGIN_I18N_INIT_LOCALE_CODE` <br/><br/>_Optional_ | Initialization locale for the app, if the [Internationalization (i18n) plugin](/developer-docs/latest/plugins/i18n.md) is installed and enabled on Content-Types (see [Configuration of i18n in production environments](/developer-docs/latest/plugins/i18n.md#configuration-of-the-default-locale)) | `String` | `'en'` |
| `API_TOKEN_SALT`<br/><br/>_Optional_ | Salt to use to generate [API tokens](/developer-docs/latest/setup-deployment-guides/configurations/optional/api-tokens.md) | `String` | - |
| `APP_KEYS` | static keys used by the [session middleware](/developer-docs/latest/setup-deployment-guides/configurations/required/middlewares.md#internal-middlewares-configuration-reference) | `String` | `undefined`
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we should not put it as a config variable. And once strapi/strapi#12287 is merged, we should also remove the line 24 (API_TOKEN_SALT).
Are you available to discuss about it, it would be easier on face-to-face :)

@stb13579 stb13579 closed this Mar 18, 2022
@stb13579 stb13579 reopened this Mar 18, 2022
@stb13579 stb13579 added the flag: don't merge This PR should not be merged at the moment label Mar 18, 2022
@stb13579
Copy link
Contributor Author

Need to remove the API SALT Token from the table and not add APP_KEYS either. Will push a new commit next week.

@vercel
Copy link

vercel bot commented Apr 25, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
documentation ✅ Ready (Inspect) Visit Preview Apr 25, 2022 at 8:12AM (UTC)

petersg83
petersg83 approved these changes Apr 25, 2022
View reviewed changes
Copy link
Contributor

@petersg83 petersg83 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@stb13579 stb13579 changed the title Added APP_KEYS to env table remove API_TOKEN_SALT from the env table Apr 25, 2022
@stb13579 stb13579 removed the flag: don't merge This PR should not be merged at the moment label Apr 25, 2022
@stb13579 stb13579 merged commit f43441d into main Apr 25, 2022
@stb13579 stb13579 deleted the dev/env-update branch April 25, 2022 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pwizla pwizla Awaiting requested review from pwizla pwizla is a code owner

1 more reviewer

@petersg83 petersg83 petersg83 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

internal PRs created by the Strapi core team size: small If the PR includes only 1 file with <10 lines of text or a small code fix source: Dev Docs PRs/issues targeting the Developer Docs target: v4 Documentation PRs/issues targeting content from docs.strapi.io (main branch).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stb13579 @petersg83