-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Comments
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. |
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. |
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 |
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 |
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. |
I am checking it now. |
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). |
Thanks for reporting back. Happy to hear the issue is fixed. |
* cache user access by login * invalidate user access based on received webhook events Related to #80
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
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
The text was updated successfully, but these errors were encountered: