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

Improve auth.json.sample #20416

Merged
merged 3 commits into from
Jan 19, 2019
Merged

Improve auth.json.sample #20416

merged 3 commits into from
Jan 19, 2019

Conversation

brendanfalkowski
Copy link
Contributor

Description (*)

  1. The file auth.json.sample was indented with 3-spaces not the normal 4-spaces for JSON, which was corrected.
  2. A placeholder for the GitHub auth was added to this file since it is required and part of the docs: https://devdocs.magento.com/guides/v2.3/install-gde/prereq/dev_install.html

Manual testing scenarios (*)

None. File is non-executable and is a sample of this environment file.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @brendanfalkowski. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-engcom-team
Copy link
Contributor

Hi @okorshenko, thank you for the review.
ENGCOM-3917 has been created to process this Pull Request

@magento-engcom-team magento-engcom-team merged commit 7d0f89d into magento:2.3-develop Jan 19, 2019
@ghost
Copy link

ghost commented Jan 19, 2019

Hi @brendanfalkowski, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@magento-engcom-team
Copy link
Contributor

Hi @brendanfalkowski. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

1 similar comment
@magento-engcom-team
Copy link
Contributor

Hi @brendanfalkowski. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

@hostep
Copy link
Contributor

hostep commented Jan 19, 2019

@brendanfalkowski: I don't really agree with the Github OAuth token in this sample file.
Since a Github OAuth token is personal and isn't linked to a specific (Magento) project.

Composer has a user specific configuration file for that, usually in your home directory: ~/.composer/auth.json. Composer will then merge the user specific configuration with a project specific configuration file whenever you execute it.

I think this could turn out to be a sort of security problem when people put their personal Github OAuth token in the auth.json file and then also commit that file in a version control system and share it with other people.
Some people argue that auth.json should not be included in version control, while others argue that it should be included, it depends on how one or multiple people working on a Magento project take care of these repo.magento.com access tokens and if they want to share them or keep them personal.

In my opinion the github-oauth part should be removed again from the auth.json.sample file to avoid giving bad advise to newcomers in the composer world.

Thoughts? Other opinions?

@brendanfalkowski
Copy link
Contributor Author

@hostep Thanks for writing about this. The security implications make sense, but from my personal experience when Magento did not provide any guidance on Composer and GitHub auth that made Magento even harder to install.

For someone who rarely uses Composer (a frontend developer like me) having a complete configuration file eases the introduction and might still necessary on the server depending on how code is deployed.

I created a PR for Dev Docs a few weeks ago that expanded this part of the documentation, and asked for feedback on whether GitHub personal access tokens were actually the best method. It was the only thing I found that worked, but there was no feedback on the changes when it was approved.

I made this PR following the same idea that providing more information in context gives an inexperienced person hints at what they're missing. I don't think things like this should be exclusively in code or documentation, but both to improve awareness.

Personally I am using the "home directory" and "project directory" Composer auth.json in different projects without committing them to Git. I faced the same issue of repo.magento.com access tokens not being shared which makes installing Magento Enterprise fail without a special token.

Environments and configuration is not an area I work on, but security is more important than ease of use. If Magento decides to amend the sample file then that's fine. I would like to see the Dev Docs become more accurate and opinionated because that also removes confusion when you have to be open to multiple ways of working.

@hostep
Copy link
Contributor

hostep commented Jan 21, 2019

Thanks for the feedback, I'll have a look in the documentation to see if this can be explained better (maybe later this week).

You are correct in that you (sometimes) need Github personal access tokens (even on non-Magento projects), but the place where you should store them would be in your user configuration and not in the project configuration in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants