feat: validate github organization during OAuth login #6700
Conversation
WalkthroughThe changes enhance GitHub authentication by integrating organization membership verification. A new configuration field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OAuthProvider as GitHubOAuthProvider
participant GitHubAPI as OrgMembershipEndpoint
User->>OAuthProvider: Initiate OAuth login
OAuthProvider->>GitHubAPI: GET /orgs/{org}/members/{username}
alt Membership confirmed (200 OK)
GitHubAPI-->>OAuthProvider: Membership verified
OAuthProvider->>User: Authentication Success
else Membership not confirmed
GitHubAPI-->>OAuthProvider: Membership denied
OAuthProvider->>User: Raise AuthenticationException (5122)
end
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
admin/app/authentication/github/form.tsx (1)
97-109: Improve description for GitHub Organization ID field.The field is correctly added and marked as optional, but the description could be more helpful for users.
Consider updating the description to provide more context about what this field is for and how to obtain the organization ID:
description: ( <> - The organization github ID. + Optional: Enter your GitHub Organization ID to restrict login to members of this organization. + When provided, only users who belong to this organization will be allowed to sign in. + You can find your Organization ID in your GitHub organization settings or via the GitHub API. </> ),apiserver/plane/authentication/provider/oauth/github.py (1)
21-21: Remove extraneous f-string prefix.The org_membership_url doesn't contain any placeholders but is defined as an f-string.
- org_membership_url = f"https://api.github.com/orgs" + org_membership_url = "https://api.github.com/orgs"🧰 Tools
🪛 Ruff (0.8.2)
21-21: f-string without any placeholders
Remove extraneous
fprefix(F541)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
admin/app/authentication/github/form.tsx(3 hunks)apiserver/plane/authentication/adapter/error.py(1 hunks)apiserver/plane/authentication/provider/oauth/github.py(4 hunks)apiserver/plane/license/management/commands/configure_instance.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/authentication/provider/oauth/github.py
21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
30-30: Line too long (97 > 88)
(E501)
131-131: Line too long (131 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (9)
apiserver/plane/authentication/adapter/error.py (1)
39-39: New error code added for GitHub organization membership checks.This error code will be used when a GitHub user tries to authenticate but doesn't belong to the configured organization.
apiserver/plane/license/management/commands/configure_instance.py (1)
74-79: Configuration option added for GitHub organization validation.The GitHub organization ID configuration is correctly added to the configuration keys, consistent with other GitHub settings. The value is properly marked as non-encrypted.
admin/app/authentication/github/form.tsx (2)
46-46: Default value added for GitHub organization ID.This field correctly retrieves the GitHub organization ID from the existing configuration.
167-167: Reset function updated to include GitHub organization ID.The form reset is correctly handling the new field.
apiserver/plane/authentication/provider/oauth/github.py (5)
26-26: Scope defined for organization membership checks.Appropriate GitHub API scope added to access organization data.
30-43: GitHub organization ID added to provider configuration.The configuration is properly retrieved and now includes the organization ID value. The line is slightly over the recommended length limit but remains readable.
🧰 Tools
🪛 Ruff (0.8.2)
30-30: Line too long (97 > 88)
(E501)
55-58: Conditional scope expansion for organization membership checks.This implementation correctly adds the organization scope only when an organization ID is provided, following the principle of least privilege.
129-133:✅ Verification successful
GitHub organization membership verification implemented.
The method uses the GitHub API to check if a user belongs to the specified organization. This correctly implements the feature requirement.
However, there's a line length issue and we should validate if the GitHub API endpoint works as expected.
🏁 Script executed:
#!/bin/bash # Check if the GitHub API endpoint format is correct for org membership curl -s "https://docs.github.com/en/rest/orgs/members?apiVersion=2022-11-28" | grep -i "/orgs/{org}/memberships/{username}"Length of output: 131070
GitHub membership check implementation verified and approved
- The GitHub API endpoint used—
/orgs/{org}/memberships/{username}—matches the documented format as confirmed by the curl/grep output.- The method correctly returns a 200 status for valid memberships.
- There is a minor style concern regarding line length; please refactor or adjust lint settings to resolve the line length warning.
🧰 Tools
🪛 Ruff (0.8.2)
131-131: Line too long (131 > 88)
(E501)
141-147:✅ Verification successful
Organization membership validation added to authentication flow.
This implementation correctly checks if the user belongs to the specified organization before allowing authentication, raising the appropriate exception if not.
Can you verify that the GitHub API responses are properly handled? Let's check the documentation for the membership API:
🌐 Web query:
What HTTP status codes are returned by GitHub API for organization membership checks?💡 Result:
The GitHub API returns specific HTTP status codes for organization membership checks, depending on the type of membership being verified. Here's a breakdown of the key endpoints and their responses:
Organization Membership Status Codes
1. General Membership Check
Endpoint:GET /orgs/{org}/memberships/{username}
Status codes:
200 OK: User has an active membership403 Forbidden: Request lacks proper authorization404 Not Found: User has no membership record [1]2. Public Membership Check
Endpoint:GET /orgs/{org}/public_members/{username}
Status codes:
204 No Content: User is a public member404 Not Found: User isn't a public member [1]3. Membership State Transitions
When modifying memberships (PUT/PATCHendpoints), additional codes appear:
422 Unprocessable Entity: Invalid request parameters or spam detection200 OK: Successful membership update [1]
Endpoint Success Code Error Codes Standard Check GET /memberships200 403, 404 Public Check GET /public_members204 404 Modification PUT/PATCH200 403, 422 These responses help developers handle different scenarios:
- Handle 404s for missing memberships
- Use 403 to detect authorization issues
- Leverage 204's empty body for public member verification
Citations:
- 1: https://docs.github.com/rest/orgs/members
- 2: https://docs.github.com/rest/teams/members
- 3: https://docs.github.com/rest/commits/statuses
- 4: https://docs.github.com/en/enterprise-server@3.15/rest/teams/members
- 5: https://docs.github.com/en/rest/orgs/orgs
- 6: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28
- 7: https://docs.github.com/rest/orgs/orgs
- 8: https://docs.github.com/en/rest/repos/repos
- 9: https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28
- 10: https://docs.github.com/en/rest/orgs?apiVersion=2022-11-28
GitHub API membership responses are correctly handled.
The implementation correctly interprets GitHub’s HTTP status codes based on the official documentation (e.g., 200/204 for valid membership versus 403/404 for non-membership). The membership check viais_user_in_organizationappropriately raises anAuthenticationExceptionwhen the user isn’t part of the organization, so no additional changes are required.
sriramveeraghanta
changed the title
|
Hey @SamuelTR20 Your changes looks good to us, we will be testing it internally. We will merging these changes as a part of the next minor release. |
Description
A new functionality has been added to the existing OAuth2 authentication system. In addition to validating GitHub credentials, a new field for entering the Organization ID has been introduced. This field allows the system to verify whether the user belongs to the specified organization before granting access. If the user does not belong to the organization, access will be denied. If the Organization ID field is left empty, the behavior will remain the same as before, allowing access to any user with a valid GitHub account.
Type of Change
Screenshots and Media (if applicable)
Before
After
Unauthorized
Test Scenarios
Case 1: User enters valid GitHub credentials and a valid
Organization IDthey belong to.Case 2: User enters valid GitHub credentials but an
Organization IDthey do not belong to.Case 3: User enters valid GitHub credentials but does not provide an
Organization ID.Case 4: User enters invalid GitHub credentials.
Organization ID.Summary by CodeRabbit