Skip to content
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

fix(core): Updates dpop check for connect #1760

Merged
merged 3 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions service/internal/auth/authn.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@
}

type receiverInfo struct {
// The URI of the request
u string
// The HTTP method of the request
m string
// Acceptable URIs of the request
u []string
// Allowed HTTP methods of the request
m []string
}

func normalizeURL(o string, u *url.URL) string {
Expand All @@ -209,6 +209,8 @@
return
}

dp := r.Header.Values("Dpop")

// Verify the token
header := r.Header["Authorization"]
if len(header) < 1 {
Expand All @@ -225,10 +227,11 @@
}
}
accessTok, ctxWithJWK, err := a.checkToken(r.Context(), header, receiverInfo{
u: normalizeURL(origin, r.URL),
m: r.Method,
}, r.Header["Dpop"])
u: []string{normalizeURL(origin, r.URL)},
m: []string{r.Method},
}, dp)
if err != nil {
slog.WarnContext(r.Context(), "unauthenticated", "error", err, "dpop", dp, "authorization", header)
http.Error(w, "unauthenticated", http.StatusUnauthorized)
return
}
Expand Down Expand Up @@ -270,6 +273,32 @@
ctx context.Context,
req connect.AnyRequest,
) (connect.AnyResponse, error) {
ri := receiverInfo{
u: []string{req.Spec().Procedure},
m: []string{http.MethodPost},
}

paths := req.Header()["Pattern"]
if len(paths) == 0 {
paths = allowedPublicEndpoints[:]
}
for _, m := range []string{"Origin", "Grpcgateway-Origin", "Grpcgateway-Referer"} {
dmihalcik-virtru marked this conversation as resolved.
Show resolved Hide resolved
origins := req.Header().Values(m)
if len(origins) == 0 {
continue
}
for _, o := range origins {
if strings.HasSuffix(o, ":443") {
o = "https://" + strings.TrimPrefix(strings.TrimSuffix(o, ":443"), "https://")
} else {
o = strings.TrimSuffix(o, ":80")
}
for _, u := range paths {
ri.u = append(ri.u, normalizeURL(o, &url.URL{Path: u}))
}
}
}

// Interceptor Logic
// Allow health checks and other public routes to pass through
if slices.ContainsFunc(a.publicRoutes, a.isPublicRoute(req.Spec().Procedure)) { //nolint:contextcheck // There is no way to pass a context here
Expand All @@ -289,10 +318,7 @@
token, newCtx, err := a.checkToken(
ctx,
header,
receiverInfo{
u: req.Spec().Procedure,
m: http.MethodPost,
},
ri,
req.Header()["Dpop"],
)
if err != nil {
Expand Down Expand Up @@ -515,7 +541,7 @@
return nil, fmt.Errorf("`htm` claim missing in DPoP JWT")
}

if htm != dpopInfo.m {
if !slices.Contains(dpopInfo.m, htm.(string)) {

Check failure on line 544 in service/internal/auth/authn.go

View workflow job for this annotation

GitHub Actions / go (service)

type assertion must be checked (forcetypeassert)
return nil, fmt.Errorf("incorrect `htm` claim in DPoP JWT; received [%v], but should match [%v]", htm, dpopInfo.m)
}

Expand All @@ -524,7 +550,7 @@
return nil, fmt.Errorf("`htu` claim missing in DPoP JWT")
}

if htu != dpopInfo.u {
if !slices.Contains(dpopInfo.u, htu.(string)) {

Check failure on line 553 in service/internal/auth/authn.go

View workflow job for this annotation

GitHub Actions / go (service)

type assertion must be checked (forcetypeassert)
return nil, fmt.Errorf("incorrect `htu` claim in DPoP JWT; received [%v], but should match [%v]", htu, dpopInfo.u)
}

Expand Down
4 changes: 2 additions & 2 deletions service/internal/auth/authn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,8 @@ func (s *AuthSuite) TestInvalid_DPoP_Cases() {
context.Background(),
[]string{fmt.Sprintf("DPoP %s", string(testCase.accessToken))},
receiverInfo{
u: "/a/path",
m: http.MethodPost,
u: []string{"/a/path"},
m: []string{http.MethodPost},
},
[]string{dpopToken},
)
Expand Down
26 changes: 25 additions & 1 deletion service/internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"net"
"net/http"
"net/http/pprof"
"net/textproto"
"regexp"
"strings"
"time"
Expand All @@ -28,6 +29,7 @@
"golang.org/x/net/http2/h2c"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/metadata"
)

const (
Expand Down Expand Up @@ -165,7 +167,29 @@
}

// GRPC Gateway Mux
grpcGatewayMux := runtime.NewServeMux()
grpcGatewayMux := runtime.NewServeMux(
runtime.WithIncomingHeaderMatcher(
func(key string) (string, bool) {
if k, ok := runtime.DefaultHeaderMatcher(key); ok {
return k, true
}
if textproto.CanonicalMIMEHeaderKey(key) == "Dpop" {
return "Dpop", true
}
return "", false
},
),
runtime.WithMetadata(func(ctx context.Context, r *http.Request) metadata.MD {

Check failure on line 182 in service/internal/server/server.go

View workflow job for this annotation

GitHub Actions / go (service)

unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it as _ (revive)
md := make(map[string]string)
if method, ok := runtime.RPCMethod(ctx); ok {
md["method"] = method // /grpc.gateway.examples.internal.proto.examplepb.LoginService/Login
}
if pattern, ok := runtime.HTTPPathPattern(ctx); ok {
md["pattern"] = pattern // /v1/example/login
}
return metadata.New(md)
}),
)

// Create http server
httpServer, err := newHTTPServer(config, connectRPC.Mux, grpcGatewayMux, authN, logger)
Expand Down
Loading