Skip to content

Commit

Permalink
authorize: allow access to /.pomerium/webauthn when policy denies acc…
Browse files Browse the repository at this point in the history
…ess (#4016)

authorize: allow access to /.pomerium/webauthn when policy denies access (#4015)

Co-authored-by: Caleb Doxsey <[email protected]>
  • Loading branch information
backport-actions-token[bot] and calebdoxsey committed Feb 27, 2023
1 parent 1af749e commit 7afa9d4
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 11 deletions.
14 changes: 10 additions & 4 deletions authorize/check_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,20 @@ func (a *Authorize) requireWebAuthnResponse(
opts := a.currentOptions.Load()
state := a.state.Load()

if !a.shouldRedirect(in) {
return a.deniedResponse(ctx, in, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), nil)
}

// always assume https scheme
checkRequestURL := getCheckRequestURL(in)
checkRequestURL.Scheme = "https"

// If we're already on a webauthn route, return OK.
// https://github.com/pomerium/pomerium-console/issues/3210
if checkRequestURL.Path == urlutil.WebAuthnURLPath || checkRequestURL.Path == urlutil.DeviceEnrolledPath {
return a.okResponse(result.Headers), nil
}

if !a.shouldRedirect(in) {
return a.deniedResponse(ctx, in, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), nil)
}

q := url.Values{}
if deviceType, ok := result.Allow.AdditionalData["device_type"].(string); ok {
q.Set(urlutil.QueryDeviceType, deviceType)
Expand Down
30 changes: 30 additions & 0 deletions authorize/check_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,36 @@ func TestAuthorize_handleResult(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 302, int(res.GetDeniedResponse().GetStatus().GetCode()))
})
t.Run("device-unauthenticated", func(t *testing.T) {
res, err := a.handleResult(context.Background(),
&envoy_service_auth_v3.CheckRequest{},
&evaluator.Request{},
&evaluator.Result{
Allow: evaluator.NewRuleResult(false, criteria.ReasonDeviceUnauthenticated),
})
assert.NoError(t, err)
assert.Equal(t, 302, int(res.GetDeniedResponse().GetStatus().GetCode()))

t.Run("webauthn path", func(t *testing.T) {
res, err := a.handleResult(context.Background(),
&envoy_service_auth_v3.CheckRequest{
Attributes: &envoy_service_auth_v3.AttributeContext{
Request: &envoy_service_auth_v3.AttributeContext_Request{
Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{
Path: "/.pomerium/webauthn",
},
},
},
},
&evaluator.Request{},
&evaluator.Result{
Allow: evaluator.NewRuleResult(true, criteria.ReasonPomeriumRoute),
Deny: evaluator.NewRuleResult(false, criteria.ReasonDeviceUnauthenticated),
})
assert.NoError(t, err)
assert.NotNil(t, res.GetOkResponse())
})
})
}

func TestAuthorize_okResponse(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion config/envoyconfig/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (b *Builder) buildPomeriumHTTPRoutes(options *config.Options, host string)
routes = append(routes,
// enable ext_authz
b.buildControlPlanePathRoute("/.pomerium/jwt", true),
b.buildControlPlanePathRoute("/.pomerium/webauthn", true),
b.buildControlPlanePathRoute(urlutil.WebAuthnURLPath, true),
// disable ext_authz and passthrough to proxy handlers
b.buildControlPlanePathRoute("/ping", false),
b.buildControlPlanePathRoute("/healthz", false),
Expand Down
7 changes: 4 additions & 3 deletions config/envoyconfig/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/pomerium/pomerium/config"
"github.com/pomerium/pomerium/config/envoyconfig/filemgr"
"github.com/pomerium/pomerium/internal/testutil"
"github.com/pomerium/pomerium/internal/urlutil"
)

func policyNameFunc() func(*config.Policy) string {
Expand Down Expand Up @@ -88,7 +89,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) {

testutil.AssertProtoJSONEqual(t, `[
`+routeString("path", "/.pomerium/jwt", true)+`,
`+routeString("path", "/.pomerium/webauthn", true)+`,
`+routeString("path", urlutil.WebAuthnURLPath, true)+`,
`+routeString("path", "/ping", false)+`,
`+routeString("path", "/healthz", false)+`,
`+routeString("path", "/.pomerium", false)+`,
Expand Down Expand Up @@ -127,7 +128,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) {

testutil.AssertProtoJSONEqual(t, `[
`+routeString("path", "/.pomerium/jwt", true)+`,
`+routeString("path", "/.pomerium/webauthn", true)+`,
`+routeString("path", urlutil.WebAuthnURLPath, true)+`,
`+routeString("path", "/ping", false)+`,
`+routeString("path", "/healthz", false)+`,
`+routeString("path", "/.pomerium", false)+`,
Expand Down Expand Up @@ -155,7 +156,7 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) {

testutil.AssertProtoJSONEqual(t, `[
`+routeString("path", "/.pomerium/jwt", true)+`,
`+routeString("path", "/.pomerium/webauthn", true)+`,
`+routeString("path", urlutil.WebAuthnURLPath, true)+`,
`+routeString("path", "/ping", false)+`,
`+routeString("path", "/healthz", false)+`,
`+routeString("path", "/.pomerium", false)+`,
Expand Down
10 changes: 8 additions & 2 deletions internal/urlutil/known.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,21 @@ func SignOutURL(r *http.Request, authenticateURL *url.URL, key []byte) string {
return NewSignedURL(key, u).Sign().String()
}

// Device paths
const (
WebAuthnURLPath = "/.pomerium/webauthn"
DeviceEnrolledPath = "/.pomerium/device-enrolled"
)

// WebAuthnURL returns the /.pomerium/webauthn URL.
func WebAuthnURL(r *http.Request, authenticateURL *url.URL, key []byte, values url.Values) string {
u := authenticateURL.ResolveReference(&url.URL{
Path: "/.pomerium/webauthn",
Path: WebAuthnURLPath,
RawQuery: buildURLValues(values, url.Values{
QueryDeviceType: {DefaultDeviceType},
QueryEnrollmentToken: nil,
QueryRedirectURI: {authenticateURL.ResolveReference(&url.URL{
Path: "/.pomerium/device-enrolled",
Path: DeviceEnrolledPath,
}).String()},
}).Encode(),
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/policy/criteria/pomerium_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package criteria
import (
"github.com/open-policy-agent/opa/ast"

"github.com/pomerium/pomerium/internal/urlutil"
"github.com/pomerium/pomerium/pkg/policy/generator"
"github.com/pomerium/pomerium/pkg/policy/parser"
"github.com/pomerium/pomerium/pkg/policy/rules"
Expand Down Expand Up @@ -34,7 +35,7 @@ func (c pomeriumRoutesCriterion) GenerateRule(_ string, _ parser.Value) (*ast.Ru
r2.Body = ast.Body{
ast.MustParseExpr(`contains(input.http.url, "/.pomerium/")`),
ast.MustParseExpr(`not contains(input.http.url, "/.pomerium/jwt")`),
ast.MustParseExpr(`not contains(input.http.url, "/.pomerium/webauthn")`),
ast.MustParseExpr(`not contains(input.http.url, "` + urlutil.WebAuthnURLPath + `")`),
}
r1.Else = r2

Expand Down

0 comments on commit 7afa9d4

Please sign in to comment.