OCA / interface-github

Tools to interact with github from Odoo
GNU Affero General Public License v3.0
46 stars 77 forks source link

[12.0][FIX] github_connector: Add credentials to clone a private repository. #58

Closed imanie383 closed 4 years ago

imanie383 commented 4 years ago

When you try to clone a private repository, the process is interrupted by the request for credentials like in the following video : https://youtu.be/7tDVTRsqWas

pedrobaeza commented 4 years ago

This should be done through a token, not with credentials. Why do you need to add this?

pedrobaeza commented 4 years ago

@CarlosRoca13 can you check this? Specifically, if it's flawing in the same vulnerability than previously.

yajo commented 4 years ago

The need is obvious by the video: to clone private github repos. It doesn't make much sense that the token is global, as different repos might need different tokens. But well... that's how it is designed, and we don't need to change that until it's really needed. You should explain in the README which scopes does the token need to work fine. Also you should explain that the git code should be found in a secure storage where only the Odoo process has read and write capabilities, or secrets might get exposed. Since the login/password is stored in the config file, and that file needs more or less the same access level, this is not exposing a new vulnerability if it is configured properly, but that's why you should add those warnings to the README. Also you should recommend using tokens instead of login+password, although I think that's already done elsewhere.

imanie383 commented 4 years ago

The need is obvious by the video: to clone private github repos. It doesn't make much sense that the token is global, as different repos might need different tokens. But well... that's how it is designed, and we don't need to change that until it's really needed. You should explain in the README which scopes does the token need to work fine. Also you should explain that the git code should be found in a secure storage where only the Odoo process has read and write capabilities, or secrets might get exposed. Since the login/password is stored in the config file, and that file needs more or less the same access level, this is not exposing a new vulnerability if it is configured properly, but that's why you should add those warnings to the README. Also you should recommend using tokens instead of login+password, although I think that's already done elsewhere.

Done @Yajo

hugho-ad commented 4 years ago

@Yajo

could you review please?

yajo commented 4 years ago

Yes, good idea.

yajo commented 4 years ago

/ocabot merge minor

OCA-git-bot commented 4 years ago

On my way to merge this fine PR! Prepared branch 12.0-ocabot-merge-pr-58-by-Yajo-bump-minor, awaiting test results.

OCA-git-bot commented 4 years ago

Congratulations, your PR was merged at 773012e4be6204d0dc34d90126bc608b1df3d35b. Thanks a lot for contributing to OCA. ❤️

pedrobaeza commented 4 years ago

Please forward-port it to v13