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

API Changes for Github has broken SAML oauth journeys #80

Closed
joannenolan-sky opened this issue Nov 14, 2019 · 8 comments
Closed

API Changes for Github has broken SAML oauth journeys #80

joannenolan-sky opened this issue Nov 14, 2019 · 8 comments
Labels
bug Something isn't working

Comments

@joannenolan-sky
Copy link
Contributor

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

@joannenolan-sky joannenolan-sky changed the title API Changes for Github has broken some oauth journeys API Changes for Github has broken SAML oauth journeys Nov 14, 2019
@nikku
Copy link
Owner

nikku commented Nov 14, 2019

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
Copy link
Owner

nikku commented Nov 14, 2019

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
Copy link
Contributor Author

joannenolan-sky commented Nov 15, 2019

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
Copy link
Owner

nikku commented Nov 16, 2019

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 nikku closed this as completed in 579743a Nov 16, 2019
@nikku
Copy link
Owner

nikku commented Nov 16, 2019

Please verify if 579743a 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.

@nikku nikku added the bug Something isn't working label Nov 16, 2019
@joannenolan-sky
Copy link
Contributor Author

I am checking it now.

@joannenolan-sky
Copy link
Contributor Author

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
Copy link
Owner

nikku commented Nov 18, 2019

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

nikku added a commit that referenced this issue Nov 20, 2019
* cache user access by login
* invalidate user access based on received webhook events

Related to #80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants