-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace authn.Request for *http.Request #9
Open
emcfarlane
wants to merge
4
commits into
connectrpc:main
Choose a base branch
from
emcfarlane:ed/makeRequest
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,6 @@ package authn | |
|
||
import ( | ||
"context" | ||
"crypto/tls" | ||
"fmt" | ||
"net/http" | ||
"strings" | ||
|
@@ -38,7 +37,7 @@ const infoKey key = iota | |
// the information is automatically attached to the context using [SetInfo]. | ||
// | ||
// Implementations must be safe to call concurrently. | ||
type AuthFunc func(ctx context.Context, req Request) (any, error) | ||
type AuthFunc func(ctx context.Context, req *http.Request) (any, error) | ||
|
||
// SetInfo attaches authentication information to the context. It's often | ||
// useful in tests. | ||
|
@@ -71,79 +70,40 @@ func Errorf(template string, args ...any) *connect.Error { | |
return connect.NewError(connect.CodeUnauthenticated, fmt.Errorf(template, args...)) | ||
} | ||
|
||
// Request describes a single RPC invocation. | ||
type Request struct { | ||
request *http.Request | ||
} | ||
|
||
// BasicAuth returns the username and password provided in the request's | ||
// Authorization header, if any. | ||
func (r Request) BasicAuth() (username string, password string, ok bool) { | ||
return r.request.BasicAuth() | ||
} | ||
|
||
// Cookies parses and returns the HTTP cookies sent with the request, if any. | ||
func (r Request) Cookies() []*http.Cookie { | ||
return r.request.Cookies() | ||
} | ||
|
||
// Cookie returns the named cookie provided in the request or | ||
// [http.ErrNoCookie] if not found. If multiple cookies match the given name, | ||
// only one cookie will be returned. | ||
func (r Request) Cookie(name string) (*http.Cookie, error) { | ||
return r.request.Cookie(name) | ||
// InferProtocol returns the inferred RPC protocol. It is one of | ||
// [connect.ProtocolConnect], [connect.ProtocolGRPC], or [connect.ProtocolGRPCWeb]. | ||
func InferProtocol(request *http.Request) string { | ||
ct := request.Header.Get("Content-Type") | ||
switch { | ||
case strings.HasPrefix(ct, "application/grpc-web"): | ||
return connect.ProtocolGRPCWeb | ||
case strings.HasPrefix(ct, "application/grpc"): | ||
return connect.ProtocolGRPC | ||
default: | ||
return connect.ProtocolConnect | ||
} | ||
} | ||
|
||
// Procedure returns the RPC procedure name, in the form "/service/method". If | ||
// the request path does not contain a procedure name, the entire path is | ||
// returned. | ||
func (r Request) Procedure() string { | ||
path := strings.TrimSuffix(r.request.URL.Path, "/") | ||
// InferProcedure returns the inferred RPC procedure. It is of the form | ||
// "/service/method". If the request path does not contain a procedure name, the | ||
// entire path is returned. | ||
func InferProcedure(request *http.Request) string { | ||
path := strings.TrimSuffix(request.URL.Path, "/") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we do this? Do connect RPC servers actually accept an invalid trailing slash like this? Pretty sure gRPC servers are usually strict and do not allow this. |
||
ultimate := strings.LastIndex(path, "/") | ||
if ultimate < 0 { | ||
return r.request.URL.Path | ||
return request.URL.Path | ||
} | ||
penultimate := strings.LastIndex(path[:ultimate], "/") | ||
if penultimate < 0 { | ||
return r.request.URL.Path | ||
return request.URL.Path | ||
} | ||
procedure := path[penultimate:] | ||
if len(procedure) < 4 { // two slashes + service + method | ||
return r.request.URL.Path | ||
return request.URL.Path | ||
} | ||
return procedure | ||
} | ||
|
||
// ClientAddr returns the client address, in IP:port format. | ||
func (r Request) ClientAddr() string { | ||
return r.request.RemoteAddr | ||
} | ||
|
||
// Protocol returns the RPC protocol. It is one of [connect.ProtocolConnect], | ||
// [connect.ProtocolGRPC], or [connect.ProtocolGRPCWeb]. | ||
func (r Request) Protocol() string { | ||
ct := r.request.Header.Get("Content-Type") | ||
switch { | ||
case strings.HasPrefix(ct, "application/grpc-web"): | ||
return connect.ProtocolGRPCWeb | ||
case strings.HasPrefix(ct, "application/grpc"): | ||
return connect.ProtocolGRPC | ||
default: | ||
return connect.ProtocolConnect | ||
} | ||
} | ||
|
||
// Header returns the HTTP request headers. | ||
func (r Request) Header() http.Header { | ||
return r.request.Header | ||
} | ||
|
||
// TLS returns the TLS connection state, if any. It may be nil if the connection | ||
// is not using TLS. | ||
func (r Request) TLS() *tls.ConnectionState { | ||
return r.request.TLS | ||
} | ||
|
||
// Middleware is server-side HTTP middleware that authenticates RPC requests. | ||
// In addition to rejecting unauthenticated requests, it can optionally attach | ||
// arbitrary information about the authenticated identity to the context. | ||
|
@@ -175,7 +135,7 @@ func NewMiddleware(auth AuthFunc, opts ...connect.HandlerOption) *Middleware { | |
func (m *Middleware) Wrap(handler http.Handler) http.Handler { | ||
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { | ||
ctx := request.Context() | ||
info, err := m.auth(ctx, Request{request: request}) | ||
info, err := m.auth(ctx, request) | ||
if err != nil { | ||
_ = m.errW.Write(writer, request, err) | ||
return | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no value in returning "unknown" (or empty string, etc) when the request doesn't look like any of these? Since this is middleware, it seems highly likely it could be used with a mux that has both connect and non-connect routes, so I think we do need better classification here.