Skip to content

Share more code between DNS and HTTP auth flows#552

Merged
domdomegg merged 7 commits intomodelcontextprotocol:mainfrom
joelverhagen:joelverhagen/refactor
Sep 29, 2025
Merged

Share more code between DNS and HTTP auth flows#552
domdomegg merged 7 commits intomodelcontextprotocol:mainfrom
joelverhagen:joelverhagen/refactor

Conversation

@joelverhagen
Copy link
Contributor

Motivation and Context

Currently the dns and http based auth flows share code well in the publisher tool. However there is a lot of duplication in the auth endpoint. This on it's own is not a problem but it does make enhancements to the proof record (the v=MCPv1; ... string) harder to keep in sync.

This PR adds a common.go file to share logic between the two.

This supports work I am prototyping for #482.

How Has This Been Tested?

Added new test cases for the permissions. Ran them before and after the refactoring to ensure no behavior change.
Ran existing UTs.
Ran the integration tests.
Manually exercised the flows locally.

Breaking Changes

None.

Types of changes

  • 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

  • I have read the MCP Documentation
  • 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

@joelverhagen joelverhagen changed the title Share more code between DNS and HTTP flows Share more code between DNS and HTTP auth flows Sep 25, 2025
# Conflicts:
#	internal/api/handlers/v0/auth/http_test.go
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor suggestions but generally looking good!

Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I really like this! Very clean now :)

@domdomegg domdomegg merged commit df018fc into modelcontextprotocol:main Sep 29, 2025
3 checks passed
@joelverhagen joelverhagen deleted the joelverhagen/refactor branch September 30, 2025 15:39
slimslenderslacks pushed a commit to slimslenderslacks/registry that referenced this pull request Dec 18, 2025
…#552)

<!-- Provide a brief summary of your changes -->

## Motivation and Context

Currently the `dns` and `http` based auth flows share code well in the
publisher tool. However there is a lot of duplication in the auth
endpoint. This on it's own is not a problem but it does make
enhancements to the proof record (the `v=MCPv1; ...` string) harder to
keep in sync.

This PR adds a `common.go` file to share logic between the two.

This supports work I am prototyping for
modelcontextprotocol#482.

## How Has This Been Tested?

Added new test cases for the permissions. Ran them before and after the
refactoring to ensure no behavior change.
Ran existing UTs.
Ran the integration tests.
Manually exercised the flows locally.

## Breaking Changes

None.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] 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
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants