<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> The following PR eliminates duplicate struct definitions by migrating the publisher tool to use canonical types from pkg/model, creating a single source of truth across the codebase. The publisher tool previously had its own copies of structs that were duplicated, ie. ServerJSON, Package, Repository, VersionDetail, EnvironmentVariable, RuntimeArgument and a few others which was not optimal **Changes in the PR:** - Refactored the structs to single canonical types located at pkg/model - Refactor the implementation to use/build canonical structures directly - Remove all duplicate definitions (5 structs eliminated) - Publisher now uses model.ServerJSON, model.Package, model.Repository, etc. - Renamed some of the structs to better reflect their use case, i.e. ServerJSON instead of ServerDetail - Fixes an issue where the Remote property was missing from the previous publisher copy of the Server **Benefits:** - Single source of truth - Schema changes in one place - Type safety - Compiler catches mismatches - Enhanced validation - Field constraints and bson tags - Zero breaking changes - Same CLI interface and JSON output - Future-proof - New canonical fields automatically available Supersedes: #217 and #240 ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> Locally ## Breaking Changes <!-- Will users need to update their code or configurations? --> No ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> --------- Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
216 lines
6.8 KiB
Go
216 lines
6.8 KiB
Go
package auth_test
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/golang-jwt/jwt/v5"
|
|
"github.com/modelcontextprotocol/registry/internal/api/handlers/v0/auth"
|
|
internalauth "github.com/modelcontextprotocol/registry/internal/auth"
|
|
"github.com/modelcontextprotocol/registry/internal/config"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// Mock OIDC validator for testing
|
|
type MockOIDCValidator struct {
|
|
validateFunc func(ctx context.Context, token string, audience string) (*auth.GitHubOIDCClaims, error)
|
|
}
|
|
|
|
func (m *MockOIDCValidator) ValidateToken(ctx context.Context, token string, audience string) (*auth.GitHubOIDCClaims, error) {
|
|
return m.validateFunc(ctx, token, audience)
|
|
}
|
|
|
|
func TestGitHubOIDCHandler_ExchangeToken(t *testing.T) {
|
|
cfg := &config.Config{
|
|
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", // 32 bytes hex
|
|
}
|
|
|
|
handler := auth.NewGitHubOIDCHandler(cfg)
|
|
|
|
tests := []struct {
|
|
name string
|
|
mockValidator *MockOIDCValidator
|
|
expectError bool
|
|
expectedSubject string
|
|
expectedPerms int
|
|
}{
|
|
{
|
|
name: "successful token exchange",
|
|
mockValidator: &MockOIDCValidator{
|
|
validateFunc: func(_ context.Context, _ string, _ string) (*auth.GitHubOIDCClaims, error) {
|
|
return &auth.GitHubOIDCClaims{
|
|
RegisteredClaims: jwt.RegisteredClaims{
|
|
Subject: "repo:octo-org/octo-repo:environment:prod",
|
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
|
Audience: jwt.ClaimStrings{"mcp-registry"},
|
|
},
|
|
RepositoryOwner: "octo-org",
|
|
}, nil
|
|
},
|
|
},
|
|
expectError: false,
|
|
expectedSubject: "repo:octo-org/octo-repo:environment:prod",
|
|
expectedPerms: 1,
|
|
},
|
|
{
|
|
name: "validation failure",
|
|
mockValidator: &MockOIDCValidator{
|
|
validateFunc: func(_ context.Context, _ string, _ string) (*auth.GitHubOIDCClaims, error) {
|
|
return nil, fmt.Errorf("token validation failed")
|
|
},
|
|
},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "invalid repository owner name",
|
|
mockValidator: &MockOIDCValidator{
|
|
validateFunc: func(_ context.Context, _ string, _ string) (*auth.GitHubOIDCClaims, error) {
|
|
return &auth.GitHubOIDCClaims{
|
|
RegisteredClaims: jwt.RegisteredClaims{
|
|
Subject: "repo:invalid@name/octo-repo:environment:prod",
|
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
|
Audience: jwt.ClaimStrings{"mcp-registry"},
|
|
},
|
|
RepositoryOwner: "invalid@name", // invalid character
|
|
}, nil
|
|
},
|
|
},
|
|
expectError: false, // Handler should succeed but return empty permissions
|
|
expectedSubject: "repo:invalid@name/octo-repo:environment:prod",
|
|
expectedPerms: 0, // No permissions due to invalid name
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
handler.SetValidator(tt.mockValidator)
|
|
|
|
response, err := handler.ExchangeToken(context.Background(), "test-oidc-token")
|
|
|
|
if tt.expectError {
|
|
assert.Error(t, err)
|
|
assert.Nil(t, response)
|
|
} else {
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, response)
|
|
assert.NotEmpty(t, response.RegistryToken)
|
|
assert.Greater(t, response.ExpiresAt, 0)
|
|
|
|
// Validate the generated JWT token
|
|
jwtManager := internalauth.NewJWTManager(cfg)
|
|
claims, err := jwtManager.ValidateToken(context.Background(), response.RegistryToken)
|
|
require.NoError(t, err)
|
|
|
|
assert.Equal(t, internalauth.MethodGitHubOIDC, claims.AuthMethod)
|
|
assert.Equal(t, tt.expectedSubject, claims.AuthMethodSubject)
|
|
assert.Len(t, claims.Permissions, tt.expectedPerms)
|
|
|
|
if tt.expectedPerms > 0 {
|
|
assert.Equal(t, internalauth.PermissionActionPublish, claims.Permissions[0].Action)
|
|
assert.True(t, strings.HasPrefix(claims.Permissions[0].ResourcePattern, "io.github."))
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestBuildPermissionsFromOIDC(t *testing.T) {
|
|
cfg := &config.Config{
|
|
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
|
}
|
|
handler := auth.NewGitHubOIDCHandler(cfg)
|
|
|
|
tests := []struct {
|
|
name string
|
|
claims *auth.GitHubOIDCClaims
|
|
expectedPerms []internalauth.Permission
|
|
}{
|
|
{
|
|
name: "valid repository owner",
|
|
claims: &auth.GitHubOIDCClaims{
|
|
RegisteredClaims: jwt.RegisteredClaims{
|
|
Subject: "repo:octo-org/octo-repo:environment:prod",
|
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
|
Audience: jwt.ClaimStrings{"mcp-registry"},
|
|
},
|
|
RepositoryOwner: "octo-org",
|
|
},
|
|
expectedPerms: []internalauth.Permission{
|
|
{
|
|
Action: internalauth.PermissionActionPublish,
|
|
ResourcePattern: "io.github.octo-org/*",
|
|
},
|
|
},
|
|
},
|
|
{
|
|
name: "invalid repository owner name",
|
|
claims: &auth.GitHubOIDCClaims{
|
|
RegisteredClaims: jwt.RegisteredClaims{
|
|
Subject: "repo:invalid@name/octo-repo:environment:prod",
|
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
|
Audience: jwt.ClaimStrings{"mcp-registry"},
|
|
},
|
|
RepositoryOwner: "invalid@name", // contains invalid character
|
|
},
|
|
expectedPerms: nil, // No permissions for invalid names
|
|
},
|
|
{
|
|
name: "user repository",
|
|
claims: &auth.GitHubOIDCClaims{
|
|
RegisteredClaims: jwt.RegisteredClaims{
|
|
Subject: "repo:username/octo-repo:environment:prod",
|
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
|
Audience: jwt.ClaimStrings{"mcp-registry"},
|
|
},
|
|
RepositoryOwner: "username",
|
|
},
|
|
expectedPerms: []internalauth.Permission{
|
|
{
|
|
Action: internalauth.PermissionActionPublish,
|
|
ResourcePattern: "io.github.username/*",
|
|
},
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Access the method through reflection since it's not exported
|
|
// For testing purposes, we'll create a simple test by calling ExchangeToken
|
|
// and validating the resulting permissions
|
|
mockValidator := &MockOIDCValidator{
|
|
validateFunc: func(_ context.Context, _ string, _ string) (*auth.GitHubOIDCClaims, error) {
|
|
return tt.claims, nil
|
|
},
|
|
}
|
|
handler.SetValidator(mockValidator)
|
|
|
|
response, err := handler.ExchangeToken(context.Background(), "test-token")
|
|
|
|
if tt.expectedPerms == nil {
|
|
// For invalid names, we expect empty permissions but successful token generation
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, response)
|
|
|
|
// Validate the JWT to check permissions
|
|
jwtManager := internalauth.NewJWTManager(cfg)
|
|
claims, err := jwtManager.ValidateToken(context.Background(), response.RegistryToken)
|
|
require.NoError(t, err)
|
|
assert.Len(t, claims.Permissions, 0)
|
|
} else {
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, response)
|
|
|
|
// Validate the JWT to check permissions
|
|
jwtManager := internalauth.NewJWTManager(cfg)
|
|
claims, err := jwtManager.ValidateToken(context.Background(), response.RegistryToken)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, tt.expectedPerms, claims.Permissions)
|
|
}
|
|
})
|
|
}
|
|
}
|