nikku / wuffle

A multi-repository / multi-organization task board for GitHub issues.
https://wuffle.dev
MIT License
118 stars 23 forks source link

API Changes for Github has broken SAML oauth journeys #80

Closed joannenolan-sky closed 4 years ago

joannenolan-sky commented 4 years ago

Describe the Bug

A change has been made by Github deprecating some features of access tokens.

https://github.blog/changelog/2019-11-05-api-changes-for-oauth-token-access-to-saml-organizations/

This has broken the getReadFilter in UserAccess.js so that it will return an empty array instead of the repos as you can no longer use just an access token to get data, you must now use BasicAuth.

This means that no org issues will be displayed by Wuffle.

This is mainly see in this code in UserAccess.js

github.repos.list.endpoint.merge({
            visibility: 'private'
          }),

Steps to Reproduce

This is seen when logging into Github via Wuffle, issues are being synched correctly but no private repos will be displayed.

Expected Behavior

The repos.list call will return the list of arrays the user has access to and display the issues for the correct repositories

Environment

This is seen in all browsers on all version of the code

nikku commented 4 years ago

I cannot reproduce your issue on self hosted instances of the board that connect to public and private repositories on GitHub. So I assume this is broken / removed from GitHub enterprise or specifically configured organizations only?

Could you point me to detailed deprecation notes that state what has changed so the access listing is now broken? More specifically I cannot read anything about basic auth now being required to access the list of visible end points.

nikku commented 4 years ago

We already request user tokens via the SAML web flow, cf. https://github.com/nikku/wuffle/blob/master/packages/app/lib/apps/auth-routes/AuthRoutes.js#L52.

That token is used to query for private repositories with the users identity, cf. https://github.com/nikku/wuffle/blob/master/packages/app/lib/apps/user-access/UserAccess.js#L74.

I don't see right now what should be wrong here.

Reviewing the SAML token flow code it seems like we may attach the wron OAuth scopes though: https://github.com/nikku/wuffle/blob/master/packages/app/lib/apps/auth-routes/AuthRoutes.js#L52

The documentation states that OAuth scopes must be space separated (https://developer.github.com/apps/building-oauth-apps/authorizing-oauth-apps/#parameters) so this may be something that could be investigated.

joannenolan-sky commented 4 years ago

Hi yes I am not sure why it is not working either as the webflow for Oauth seems to be the correct one, but it looks like the scope is no longer being set for the access token so no repos are returned using /user/repos but you can get org repos using that token org/repos. I will try it with the scope spaces and see if it makes any difference-> there was no difference with spaces

nikku commented 4 years ago

Looking at the OAuth documentation for GitHub apps today the scope configuration is entirely gone: https://developer.github.com/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps/#1-request-a-users-github-identity

This means, effectively the app cannot check for repositories the user can access in the way we do it: https://github.com/nikku/wuffle/blob/master/packages/app/lib/apps/user-access/UserAccess.js#L74

Instead, we must use the app specific endpoints to check the repositories available for a particular user: https://developer.github.com/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps/#check-which-installations-resources-a-user-can-access

nikku commented 4 years ago

Please verify if https://github.com/nikku/wuffle/commit/579743a9002873a2a22f6116debfbcf335f5e7ed fixes the issue for you. It comes at a performance penalty for boards with a huge amount of organizations and repositories. Future releases may perform aggressive caching to optimize for that case, too.

joannenolan-sky commented 4 years ago

I am checking it now.

joannenolan-sky commented 4 years ago

Yes, that has fixed the issue. Thanks for sorting that. It is a little slower on initial sign in but there doesn't seem to be any way around this with the new changes(Our current setup has boards with 7 repos, 10 repos and 3 repos).

nikku commented 4 years ago

Thanks for reporting back. Happy to hear the issue is fixed.