search

imsun / gitment

A comment system based on GitHub Issues.
https://imsun.github.io/gitment/
MIT License
4.06k stars 350 forks source link

Update with some new freatures #25

Closed aimingoo closed 6 years ago

aimingoo commented 7 years ago
sinkcup commented 7 years ago

@imsun review的怎么样了？

aimingoo commented 7 years ago

@sinkcup and @imsun 相关的说明在这里：https://aimingoo.github.io/1-1725.html

sinkcup commented 7 years ago

@aimingoo 新建一个项目吧（比如gitment-aimingoo），别fork了，@imsun 不在，没法合并。

imsun commented 7 years ago

我倒不是不在，但是这个 pr 和我有一些理念上的冲突。本来想找个时间统一说下，不过最近太忙老是忘——其实还是太懒。那我先简单说下：

  1. No client_secret。这是和我冲突最大的一点。@aimingoo 的实现方式是通过 gateway 代理了 OAuth 请求，然后将 client_secret 和允许接收的域名放在了自己的 server 上，也就是自己必须要跑个 server 才能实现这一点。而我建这个项目的初衷就是希望使用者不必关心任何后端问题，只通过前端代码就能体验项目的完整功能。此外 client_secret 目前来看还没有隐藏的必要，即使暴露也是足够安全的。下面是我以前的解释：

    一方面 client ID 和 client secret 并不能独立用来访问 API 。在所有 API 中只有 https://developer.github.com/v3/oauth_authorizations/#delete-an-authorization 里最后三个 API 可以凭借 client ID 和 client secret 访问，但这三个 API 同时需要 access token ，因此在本项目中只有用户本身有能力使用这三个 API ，而且不会造成任何不良影响。 另一方面，按照 OAuth 的标准（ https://tools.ietf.org/html/rfc6749#section-4.1 ），只要有 client ID 和 callback URI 就已经可以保证认证过程安全，别人无法通过 client secret 来影响认证过程的安全性，也无法用来获取用户的 token 。 另外在开源前，我也找过几个朋友对这个项目进行了测试（包括整天黑站的那种），目前并没有发现安全问题。如果哪位发现了安全问题，请在项目内提 issue ，非常感谢。

  2. Force redirect protocol to support HTTPS/HTTP Github pages site。这个我去看了 @aimingoo 的博客。之前的确没考虑过这个问题，但是仔细想想是没有这个问题的。因为之前就可以在 Gitment option 里添加 redirect_uri，与 @aimingoo 实现的强制跳转效果是一致的。
  3. 本地化问题。这个是考虑到 GitHub 本身就是纯英文，为了和 GitHub 保持一致，就没有再进行本地化。之前我在博客里给的建议是有本地化需求的可以通过自定义主题来做。不过看了 @aimingoo 的博客之后我对这个持保留态度，因为我不清楚有多少人有向非 GitHub 用户展示评论的需求。
  4. 其他小问题。主要是这个 pr 把删除文档的 commit 一起提交上来了，编程风格也有一些冲突（比如句末加分号）。

总之非常感谢 @aimingoo 对项目的关注与支持，也非常抱歉拖了那么久。上述有什么理解不对的地方请指出。

综上目前这个 pr 是不能合的。如果有其他类似需求建议做成自定义主题，或者单开一个项目。

aimingoo commented 7 years ago

各位，谢谢关注这个issue~

说说我的看法。先讲第二个问题，是个小问题，就是‘Force redirect protocol to support HTTPS/HTTP Github pages site’。这个事情可能之前大家没注意到，而正好我的github.io站点碰巧遇上了，才有这个问题出现。

现在，如果你访问http://aimingoo.github.io，或者其它某个http开始的github.io网址，你会直接重定向到https。这其实只是现在github缺省为你做的配置项（而且不能修改了），这是在2016.06.15之后申请的github pages才会有的状况。如果是在之前申请的，那么缺省并不会打开‘Enforce HTTPS’选项，因此访问http://与https://会是两个网址，而不会自动跳转。

关于这项配置的变化，Github的说明在这里：https://help.github.com/articles/securing-your-github-pages-site-with-https/

由于不会自动跳转，所以如果按gitment原来的代码，那就是从url上直接取callback，因此就会导致http/https总有一个不能用的情况——你在git application后台配置的redirect callback只能是二者之一，而网页上从url取的地址是两种都有可能。

所以对于这种情况，才加上了force_redirect_protocol这个选项，它只需要跟后台配置的redirect callback采用一致的http/https协议即可。这样一来，就只从当前的url上来取地址部分，而协议部分可以与后台一致。

再补充一下，我正好遇到这个问题，就是因为我的github pages仓库其实是早在去年上半年就申请过了，只是一直没有放内容。我开始使用的时候，也就正好是http/https两种协议都可以用，而‘Enforce HTTPS’这个配置项在github仓库里正好就没打开。所以糊里糊涂地就碰上了这个问题。而后来，由于我的仓库出了点问题，我又删除了重建，所以现在这个aimingoo.github.io已经是新的了，这个问题也就不能重现了。但我确信，如果用户的github pages是2016.06.15之前申请的，那么问题就一样还有。

好了。上面这个问题简单，但是解释起来很麻烦，呵呵。

下面来说imsun提到的关键问题，也就是No client_secret。

首先，我确实了解imsun这样处理的原因，也确实花了不少时间来思考和假设client_secret暴露之后的后果。在这个问题上，我跟imsun的结论是一样的：我也想不出在callback url限制了回调地址的情况下，暴露client_secret有什么毛病——看起来是安全的。

关于这一点细节，我在博客里并没有写（https://aimingoo.github.io/1-1722.html），我只是强调：这存在安全风险。并且，事实上当我把包含了client_secret的博客页面放在github.io上面时，也确实收到了github的邮件警告：

Hello aimingoo,

We noticed that a valid OAuth access token of yours was committed to a public GitHub repository. Publicly disclosing a valid access token would allow other people to interact with GitHub on your behalf, potentially altering data, your contact information, and billing data. ...

但是我虽然跟imsun关于“暴露了client_secret能不能带来攻击”这个问题上结论一样，但我仍然选择“尽可能去隐藏client_secret”。这是因为对风险的认识不同，我认为“风险”之所以是“风险”就在于还没有发生。如果我们找得到办法，或者想得出某种可能性发生，那它就不是“风险”了，而是“错误”。我对风险的态度是阻止——尽管我也想不出它怎么可能发生，但既然是风险，那我就要尽力阻止，而非容忍。

“暴露client_secret”是一望即知的风险，而不是已经存在错误。

这是第一。第二，则是我与imsun在“接口”上的认识不同。我认为接口就是规则，接口上要求隐藏client_secret，那就应该遵循，这是接口的意义。而不是根据对接口内部的实现的猜测，或者探测，甚至于实现代码的分析去假设我们如何使用该接口。接口不变的是约定本身，而不是其具体的实现。所以，我认为在/login/oauth/access_token这个接口上就只应该传入code和client_id，通过其它某种方式传入或暴露client_secret，就是错的。这不仅仅是风险问题，而是怎么对待“接口”这个约束条件的问题。

所以我才会花了不少力气去申请了一个支持ssl的网站，然后写了intersect这个项目去代理api请求的原因。这些事情做完了，我才发现我其实最终还是搞了个新的主页空间出来，已经背离了原来“仅仅在gihub.io上部署静态pages”的初衷。但我还是认为我是对的，因为接口如此。我可以觉得我选错了github api这个接口；但既然选了这个接口，那就按接口的要求来实现，才是对的。

然而，如果真是如此，使用gitment是不是就一定要一个proxy/gateway呢？是不是就非得像我一样去申请一个https并部署intersect这样的网关尼？其实不然。

如果： 1、开一个接口https://gh-oauth.imsun.net/access_token，调用时使用{client_domain, client_id, code}作为参数； 2、这个client_domain与client_secret是1vs1的关系，而不是直接取当前href的全部（亦即是说可以是redirect_uri，或跟github后台配置的callback一致）； 3、开在一个服务在https://gh-oauth.imsun.net/access_register上，走一次oAuth流程，让使用者一次性地将一对{client_domain, client_secret}提交到https://gh-oauth.imsun.net后台登记； 4、接口https://gh-oauth.imsun.net/access_token将使用{client_domain, client_id}在后台查找到client_secret，并将{client_secret, client_id, code}转发到github以取得access_token，返回。

简单地说，用{client_domain, client_id}替代{client_secret, client_id}，但要开发者在gh-oauth.imsun.net做一次登记。

有两点说明：

这个过程看起来不爽，但只是要求使用gitment的开发者多一次登记（要经过github oAuth之后才好），其它使用过程是一致的。

使用上述过程，其实是在登记过程中信任imsun，或gh-oauth.imsun.net一次即可；而把client_secret放在github pages上，我得信任所有人。

@sinkcup 如果确实不能合并，那么我回头就再开一个好了。谢谢

aimingoo commented 7 years ago

@sinkcup 如你所愿。"A mint on Gitment" at aimingoo/gitmint

另外，你所说的organization的问题 #27 ，在 https://gmirror.org/ 和 https://openwrt.io/about/ 均未能重现，所以……能否给我一个可重现的地址？

sinkcup commented 7 years ago

@aimingoo 几天不见，我的 gmirror.org 已经倒闭了，因为发现没什么人用，就关掉了。

这件事恰恰说明了：个人网站提供服务是不靠谱的，随时会倒闭，而完全依靠github api的服务是靠谱的。所以我觉得@imsun 的选择是对的。如果做一个proxy服务隐藏secret，随着用户增多，服务器费用越来越高，谁来出？夜间服务故障时，谁来修复？域名过期了、服务器没有续费了，大家就不能发评论了，多尴尬！别忘了这个项目的初衷就是多说不盈利倒闭了……

OAuth协议定义了 Implicit Grant 模式，无需secret，用于 client（js、android、ios），因为client无法安全保存secret。而 github 不支持 Implicit Grant 模式：

Note: GitHub's OAuth implementation supports the standard authorization code grant type. You should implement the web application flow described below to obtain an authorization code and then exchange it for a token. (The implicit grant type is not supported.)

所以我觉得做comment服务有两个原则：

  1. 直接调用知名平台，不依赖第三方，避免倒闭不能用
  2. 不要泄漏secret
  3. 当2和1违背时，优先1，github 这么多年都不支持 Implicit Grant 模式，看来泄漏没什么风险，大家也自测过了。

@aimingoo @imsun 目前的确不支持github org项目发评论，我在 openwrt.io 上试了，不能用，又移除了。参考：https://github.com/openwrtio/portal/commit/d847afda9ddb176ddadf8ea4905358a016e957cc

tiexo commented 7 years ago

唔，我觉得当前最重要的是，解决浏览器兼容适配的问题（包括移动端） 如果连评论框都不显示，加再多的功能也没用... 各位高手不妨在这方面贡献一下

aimingoo commented 7 years ago

@sinkcup

organization的问题已经Ok了。在这里 https://github.com/imsun/gitment/pull/87 ，或参考：organization下的仓库无法使用gitment #53

sinkcup commented 6 years ago

@aimingoo @imsun 既然不合适，请关闭此pull request

aimingoo commented 6 years ago

好吧。关闭什么的我最擅长了