Fixed linter errors and fixed tests

- Added error handling for JSON encoding in ServersHandler and ServersDetailHandler.
- Updated test cases to reflect changes in handler functions and improved mock expectations.
- Refactored server initialization to include ReadHeaderTimeout for better request handling.
- Modified GitHubDeviceAuth to accept context in ValidateToken and checkOrgMembership methods.
- Improved error messages in database import functions for better debugging.
- Updated memory and mongo database list functions to accept context and improved filtering logic.
- Enhanced logging in publisher tool for better visibility during execution.
- Cleaned up code formatting and comments for better readability and maintainability.
This commit is contained in:
Avinash Sridhar 2025-05-27 18:57:59 -04:00 • committed by Toby Padilla
commit b6c141a721
2 changed files with 317 additions and 225 deletions

View file

@ -3,6 +3,7 @@ package auth
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
@ -70,14 +71,19 @@ func NewGitHubDeviceAuth(config GitHubOAuthConfig) *GitHubDeviceAuth {
// It verifies the token owner matches the repository owner or is a member of the owning organization.
// It also verifies that the token was created for the same ClientID used to set up the authentication.
// Returns true if valid, false otherwise along with an error explaining the validation failure.
func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (bool, error) {
func (g *GitHubDeviceAuth) ValidateToken(ctx context.Context, token string, requiredRepo string) (bool, error) {
// If no repo is required, we can't validate properly
if requiredRepo == "" {
return false, fmt.Errorf("repository reference is required for token validation")
}
// First, validate that the token is associated with our ClientID
tokenReq, err := http.NewRequest("GET", "https://api.github.com/applications/"+g.config.ClientID+"/token", nil)
tokenReq, err := http.NewRequestWithContext(
ctx,
http.MethodGet,
"https://api.github.com/applications/"+g.config.ClientID+"/token",
nil,
)
if err != nil {
return false, err
}
@ -97,7 +103,8 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo
}
// POST instead of GET for security reasons per GitHub API
tokenReq, err = http.NewRequest("POST", "https://api.github.com/applications/"+g.config.ClientID+"/token", io.NopCloser(bytes.NewReader(checkBody)))
tokenURL := "https://api.github.com/applications/" + g.config.ClientID + "/token"
tokenReq, err = http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, io.NopCloser(bytes.NewReader(checkBody)))
if err != nil {
return false, err
}
@ -135,7 +142,7 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo
}
// Get the authenticated user
userReq, err := http.NewRequest("GET", "https://api.github.com/user", nil)
userReq, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://api.github.com/user", nil)
if err != nil {
return false, err
}
@ -175,17 +182,20 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo
// Verify that the authenticated user matches the owner
if userInfo.Login != owner {
// Check if the user is a member of the organization
isMember, err := g.checkOrgMembership(token, userInfo.Login, owner)
isMember, err := g.checkOrgMembership(ctx, token, userInfo.Login, owner)
if err != nil {
return false, fmt.Errorf("failed to check org membership: %s", owner)
}
if !isMember {
return false, fmt.Errorf("token belongs to user %s, but repository is owned by %s and user is not a member of the organization", userInfo.Login, owner)
return false, fmt.Errorf(
"token belongs to user %s, but repository is owned by %s and user is not a member of the organization",
userInfo.Login, owner)
}
}
// If we've reached this point, the token has access the repo and the user matches the owner or is a member of the owner org
// If we've reached this point, the token has access the repo and the user matches
// the owner or is a member of the owner org
return true, nil
}
@ -210,13 +220,13 @@ func (g *GitHubDeviceAuth) ExtractGitHubRepo(repoURL string) (owner, repo string
}
// checkOrgMembership checks if a user is a member of an organization
func (g *GitHubDeviceAuth) checkOrgMembership(token, username, org string) (bool, error) {
func (g *GitHubDeviceAuth) checkOrgMembership(ctx context.Context, token, username, org string) (bool, error) {
// Create request to check if user is a member of the organization
// GitHub API endpoint: GET /orgs/{org}/members/{username}
// true if status code is 204 No Content
// false if status code is 404 Not Found
url := fmt.Sprint("https://api.github.com/orgs/", org, "/members/", username)
req, err := http.NewRequest("GET", url, nil)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return false, err
}

View file

@ -8,37 +8,40 @@ import (
"github.com/modelcontextprotocol/registry/internal/model"
)
// AuthServiceImpl implements the Service interface
type AuthServiceImpl struct {
// ServiceImpl implements the Service interface
type ServiceImpl struct {
config *config.Config
githubAuth *GitHubDeviceAuth
}
// NewAuthService creates a new authentication service
//
//nolint:ireturn // Factory function intentionally returns interface for dependency injection
func NewAuthService(cfg *config.Config) Service {
githubConfig := GitHubOAuthConfig{
ClientID: cfg.GithubClientID,
ClientSecret: cfg.GithubClientSecret,
}
return &AuthServiceImpl{
return &ServiceImpl{
config: cfg,
githubAuth: NewGitHubDeviceAuth(githubConfig),
}
}
func (s *AuthServiceImpl) StartAuthFlow(ctx context.Context, method model.AuthMethod, repoRef string) (map[string]string, string, error) {
func (s *ServiceImpl) StartAuthFlow(_ context.Context, _ model.AuthMethod,
_ string) (map[string]string, string, error) {
// return not implemented error
return nil, "", fmt.Errorf("not implemented")
}
func (s *AuthServiceImpl) CheckAuthStatus(ctx context.Context, statusToken string) (string, error) {
func (s *ServiceImpl) CheckAuthStatus(_ context.Context, _ string) (string, error) {
// return not implemented error
return "", fmt.Errorf("not implemented")
}
// ValidateAuth validates authentication credentials
func (s *AuthServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentication) (bool, error) {
func (s *ServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentication) (bool, error) {
// If authentication is required but not provided
if auth.Method == "" || auth.Method == model.AuthMethodNone {
return false, ErrAuthRequired
@ -47,7 +50,9 @@ func (s *AuthServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentic
switch auth.Method {
case model.AuthMethodGitHub:
// Extract repo reference from the repository URL if it's not provided
return s.githubAuth.ValidateToken(auth.Token, auth.RepoRef)
return s.githubAuth.ValidateToken(ctx, auth.Token, auth.RepoRef)
case model.AuthMethodNone:
return false, ErrAuthRequired
default:
return false, ErrUnsupportedAuthMethod
}