Add support for get_app endpoint with app authentication #2471
Conversation
77393c9 to
d8993ab
Compare
The /app endpoint requires the use of a JWT to be accessed: https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#get-the-authenticated-app This change creates a new GithubIntegration and calls the `/app` endpoint through it so the `requester` used uses JWT and not a token. * Updated exiting GithubApp testGetAuthenticatedApp to use authentication * added test to validate assertion is raised when not called by an authenticated app. Prior to this, an authenticated application would fail to get access to its attribute with the following error: https://gist.github.com/chantra/f84e6981e2ab732a5e7acb2978872109 Signed-off-by: Manu Bretelle <chantr4@gmail.com>
| permissions=self.__app_auth.token_permissions, | ||
| ) | ||
|
|
||
| def get_app(self): |
There was a problem hiding this comment.
I think this method should go into GithubIntegration.
There was a problem hiding this comment.
Thanks. It is there already. This method here is just a proxy to get from the GitHub object to a GithubIntegration.
Are you fine with the GithubObject accessing directly the requester’s __app_auth to generate a new GithubIntegration? Or adding a app_auth getter so Github object can re-use it?
There was a problem hiding this comment.
I'd rather say GithubIntegration requires some rework to get the authentication, token refresh and Requester init args sorted out: #2461
|
In github/Requester.py
<#2471 (comment)>:
> @@ -381,6 +381,15 @@ def _get_installation_authorization(self):
permissions=self.__app_auth.token_permissions,
)
+ def get_app(self):
I'd rather say GithubIntegration requires some rework to get the
authentication, token refresh and Requester init args sorted out: #2461
<#2461>
thanks for sorting out the building blocks. I will rebase and adjust the
PR accordingly.
… —
Reply to this email directly, view it on GitHub
<#2471 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF77HIU7R2S4JGMORZUELW6BNM3ANCNFSM6AAAAAAWFZKRN4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Ok, attempted chantra@1154f22 There is an issue with being able to call github.get_app() through as I don't see how we can get the GithubIntegration back to call GithubIntegration.get_app(). Also, I believe, if calling using the Github's requester, then the token will be sent, not the jwt, hence the need to be able to get back to the GithubIntegration form Github. Another solution is to make |
|
@EnricoMi do you have any feedback on my previous comment? |
|
@EnricoMi just checking if you have any feedback on my previous comment? thanks |
|
I think calling the Then, together with rework #2461, the following should work: |
|
Thanks @EnricoMi . I would be happy to do those changes once your branch is merged. Please tag me when it lands so I can check it out and update this PR. |
|
@chantra auth rework is merged, go ahead and move |
|
Thanks @EnricoMi Let me close this PR and start fresh. |
add support for get_app endpoint for app authenticated applications
The /app endpoint requires the use of a JWT to be accessed: https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#get-the-authenticated-app
This change creates a new GithubIntegration and calls the
/appendpoint through it so the
requesterused uses JWT and not a token.Prior to this, an authenticated application would fail to get access to
its attribute with the following error: https://gist.github.com/chantra/f84e6981e2ab732a5e7acb2978872109