Issue 1655: Add authorize credentials list and removal on organization under SAML #1676
Conversation
github/AuthorizationOrganization.pyi
Outdated
| @property | ||
| def token_last_eight(self) -> str: ... | ||
| @property | ||
| def scopes(self) -> list: ... |
There was a problem hiding this comment.
List[] including the types, such as List[str], please.
tests/Organization1655.py
Outdated
| from . import Framework | ||
|
|
||
|
|
||
| class Organization1655(Framework.TestCase): |
There was a problem hiding this comment.
Can we have a better name, such as OrganizationAuthorization?
tests/Organization1655.py
Outdated
|
|
||
| def testGetCredentialsAuth(self): | ||
| credentials_list = self.org.get_credential_authorizations() | ||
| self.assertListEqual(credentials_list, []) |
There was a problem hiding this comment.
You also need to test delete, but I'd also prefer we return something so we can verify the returned attributes.
There was a problem hiding this comment.
I will need to wait in that case, since the only Organization I have available at the moment,with SAML SSO, is private. I can not display the credentials list in the --record. I will ask GitHub sales for a trial for Entreprise Org hosted on GitHub.
tests/Organization1655.py
Outdated
|
|
||
| ############################ Copyrights and license ############################ | ||
| # # | ||
| # Copyright 2020 Alexandre Delisle <alexodelisle @gmail.com> # |
There was a problem hiding this comment.
space in the email address.
Authorization organization, PyGithub#1676
Codecov Report
@@ Coverage Diff @@
## master #1676 +/- ##
==========================================
+ Coverage 98.67% 98.69% +0.02%
==========================================
Files 48 49 +1
Lines 2561 2609 +48
==========================================
+ Hits 2527 2575 +48
Misses 34 34
Continue to review full report at Codecov.
|
PyGithub#1676: add missing attribute…
PyGithub#1676: add missing __repr__ …
|
@s-t-e-v-e-n-k , finally got the coverage to pass. I think I got all the reviews, since there is now test for the Attributes and repr, also got an Enterprise Team so the credential list also has non redacted information. |
| def get_credential_authorizations(self): | ||
| """ | ||
| :calls: `GET /orgs/:org/credential-authorizations <https://developer.github.com/v3/orgs>`_ | ||
| :rtype: list |
There was a problem hiding this comment.
list of :class:github.AuthorizationOrganization.AuthorizationOrganization_
(I'm not certain of the formatting, but roughly that, please)
| def remove_credential_authorization( | ||
| self, | ||
| credential_id: Union[int, _NotSetType] = ..., | ||
| ) -> None: ... |
There was a problem hiding this comment.
The return type is not None.
|
|
||
| class AuthorizationOrganization(github.GithubObject.NonCompletableGithubObject): | ||
| """ | ||
| This class represents a credential id. |
There was a problem hiding this comment.
I'd like more detail here, along with a link to the documentation of the object.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Any chance this gets some traction? |
|
@AlexandreODelisle this is awesome, can we get this over the finish line? Happy to approve once review comments are resolved. |
|
Is it possible to do something with this to resume this pull request and be able to have this functionality in PyGithub? Is it correct to create a new PR with these changes to be able to work on it? cc/ @EnricoMi , @AlexandreODelisle |
|
Please feel free to go ahead with it. Personally, I am no longer using SAML SSO on a regular basis for authentication, and also another py package. I am sorry for the delay in response. |
The objective is to implement the two available methods to list and remove credential ids linked to an Organization that have been Authorize by the user to act as authentication on the Organization.
Added the two endpoints available under the Organization on the GitHub API
https://developer.github.com/v3/orgs/#list-saml-sso-authorizations-for-an-organization
https://developer.github.com/v3/orgs/#remove-a-saml-sso-authorization-for-an-organization
Also needed to create a new Object to handle the return payload of the API since the raw data differs from the Authorization Object..
Added one unit test to list the credentials under an Organization, but I do not have a Organization that is SAML protected that I can use to display in the unittest.. So I added on a personnal organization with no SAML, so an empty List is validated.
#1655