From b2845cd65fa618ed51a203fc1bd7a0871e675462 Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 16 Mar 2022 10:15:25 -0400 Subject: [PATCH] Refactor authz skip for device auth, add org_logo_url for frontend (#4619) --- server/authz/authz.go | 16 ++++++------ server/contexts/authz/authz.go | 33 +++++++++++++++++++++++++ server/service/appconfig.go | 7 ++++-- server/service/devices.go | 27 +++++++++++++++++--- server/service/endpoint_middleware.go | 10 ++++++++ server/service/hosts.go | 9 ++++--- server/service/integration_core_test.go | 12 ++++++--- 7 files changed, 92 insertions(+), 22 deletions(-) diff --git a/server/authz/authz.go b/server/authz/authz.go index dc569229ba..98cb7909e6 100644 --- a/server/authz/authz.go +++ b/server/authz/authz.go @@ -70,16 +70,14 @@ func (a *Authorizer) SkipAuthorization(ctx context.Context) { } } -// IsAlreadyAuthorized returns true if the context has already authorized. This -// may be useful to skip authorization checks in the Service layer when e.g. -// the current request has already been authorized due to its different -// authentication method, such as the device authentication token which allows -// reading the corresponding host's information even if there is no user -// associated with the request (so typical user-based authorization checks -// would fail). -func (a *Authorizer) IsAlreadyAuthorized(ctx context.Context) bool { +// IsAuthenticatedWith returns true if the request has been authenticated with +// the specified authentication method, false otherwise. This is useful to avoid +// calling Authorize if the request is authenticated with a method that doesn't +// support granular authorizations - provided it is ok to grant access to the +// protected data. +func (a *Authorizer) IsAuthenticatedWith(ctx context.Context, method authz_ctx.AuthenticationMethod) bool { if authctx, ok := authz_ctx.FromContext(ctx); ok { - return authctx.Checked() + return authctx.AuthnMethod() == method } return false } diff --git a/server/contexts/authz/authz.go b/server/contexts/authz/authz.go index 83f04d20b0..375d138da7 100644 --- a/server/contexts/authz/authz.go +++ b/server/contexts/authz/authz.go @@ -22,12 +22,33 @@ func FromContext(ctx context.Context) (*AuthorizationContext, bool) { return v, ok } +// AuthenticationMethod identifies the method used to authenticate. +type AuthenticationMethod int + +// List of supported authentication methods. +const ( + // AuthnUserToken is when authentication is done via a user's API token, + // obtained via user/password login (or fleetctl for API-only users). + // This authentication mode supports granular authorization. + AuthnUserToken AuthenticationMethod = iota + // AuthnHostToken is when authentication is done via the osquery host + // authentication token. This authentication mode does not support granular + // authorization. + AuthnHostToken + // AuthnDeviceToken is when authentication is done via the orbit identifier, + // which only allows limited access to the device's own host information. + // This authentication mode does not support granular authorization. + AuthnDeviceToken +) + // AuthorizationContext contains the context information used for the // authorization check. type AuthorizationContext struct { l sync.Mutex // checked indicates whether a call was made to check authorization for the request. checked bool + // store the authentication method, as some methods cannot have granular authorizations. + authnMethod AuthenticationMethod } func (a *AuthorizationContext) Checked() bool { @@ -41,3 +62,15 @@ func (a *AuthorizationContext) SetChecked() { defer a.l.Unlock() a.checked = true } + +func (a *AuthorizationContext) AuthnMethod() AuthenticationMethod { + a.l.Lock() + defer a.l.Unlock() + return a.authnMethod +} + +func (a *AuthorizationContext) SetAuthnMethod(method AuthenticationMethod) { + a.l.Lock() + defer a.l.Unlock() + a.authnMethod = method +} diff --git a/server/service/appconfig.go b/server/service/appconfig.go index 947ecf2efa..6c75309fff 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -12,6 +12,7 @@ import ( "net" "net/url" + authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" @@ -101,8 +102,10 @@ func getAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet.Se } func (svc *Service) AppConfig(ctx context.Context) (*fleet.AppConfig, error) { - if err := svc.authz.Authorize(ctx, &fleet.AppConfig{}, fleet.ActionRead); err != nil { - return nil, err + if !svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceToken) { + if err := svc.authz.Authorize(ctx, &fleet.AppConfig{}, fleet.ActionRead); err != nil { + return nil, err + } } return svc.ds.AppConfig(ctx) diff --git a/server/service/devices.go b/server/service/devices.go index 29a4055f62..12e3cf99f4 100644 --- a/server/service/devices.go +++ b/server/service/devices.go @@ -20,25 +20,44 @@ func (r *getDeviceHostRequest) deviceAuthToken() string { return r.Token } +type getDeviceHostResponse struct { + Host *HostDetailResponse `json:"host"` + OrgLogoURL string `json:"org_logo_url"` + Err error `json:"error,omitempty"` +} + +func (r getDeviceHostResponse) error() error { return r.Err } + func getDeviceHostEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (interface{}, error) { host, ok := hostctx.FromContext(ctx) if !ok { err := ctxerr.Wrap(ctx, fleet.NewAuthRequiredError("internal error: missing host from request context")) - return getHostResponse{Err: err}, nil + return getDeviceHostResponse{Err: err}, nil } // must still load the full host details, as it returns more information hostDetails, err := svc.GetHost(ctx, host.ID) if err != nil { - return getHostResponse{Err: err}, nil + return getDeviceHostResponse{Err: err}, nil } resp, err := hostDetailResponseForHost(ctx, svc, hostDetails) if err != nil { - return getHostResponse{Err: err}, nil + return getDeviceHostResponse{Err: err}, nil } - return getHostResponse{Host: resp}, nil + // the org logo URL config is required by the frontend to render the page; + // we need to be careful with what we return from AppConfig in the response + // as this is a weakly authenticated endpoint (with the device auth token). + ac, err := svc.AppConfig(ctx) + if err != nil { + return getDeviceHostResponse{Err: err}, nil + } + + return getDeviceHostResponse{ + Host: resp, + OrgLogoURL: ac.OrgInfo.OrgLogoURL, + }, nil } // AuthenticateDevice returns the host identified by the device authentication diff --git a/server/service/endpoint_middleware.go b/server/service/endpoint_middleware.go index 6d6de8e33b..c51dbf36c7 100644 --- a/server/service/endpoint_middleware.go +++ b/server/service/endpoint_middleware.go @@ -11,6 +11,7 @@ import ( "github.com/go-kit/kit/log/level" kithttp "github.com/go-kit/kit/transport/http" + authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz" hostctx "github.com/fleetdm/fleet/v4/server/contexts/host" "github.com/fleetdm/fleet/v4/server/contexts/token" "github.com/fleetdm/fleet/v4/server/contexts/viewer" @@ -56,6 +57,9 @@ func authenticatedDevice(svc fleet.Service, logger log.Logger, next endpoint.End ctx = hostctx.NewContext(ctx, host) instrumentHostLogger(ctx) + if ac, ok := authz_ctx.FromContext(ctx); ok { + ac.SetAuthnMethod(authz_ctx.AuthnDeviceToken) + } resp, err := next(ctx, request) if err != nil { @@ -100,6 +104,9 @@ func authenticatedHost(svc fleet.Service, logger log.Logger, next endpoint.Endpo ctx = hostctx.NewContext(ctx, host) instrumentHostLogger(ctx) + if ac, ok := authz_ctx.FromContext(ctx); ok { + ac.SetAuthnMethod(authz_ctx.AuthnHostToken) + } resp, err := next(ctx, request) if err != nil { @@ -154,6 +161,9 @@ func authenticatedUser(svc fleet.Service, next endpoint.Endpoint) endpoint.Endpo } ctx = viewer.NewContext(ctx, *v) + if ac, ok := authz_ctx.FromContext(ctx); ok { + ac.SetAuthnMethod(authz_ctx.AuthnUserToken) + } return next(ctx, request) } diff --git a/server/service/hosts.go b/server/service/hosts.go index be413ca997..dc179883e2 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -7,6 +7,7 @@ import ( "time" "github.com/fleetdm/fleet/v4/server/contexts/authz" + authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/logging" "github.com/fleetdm/fleet/v4/server/contexts/viewer" @@ -270,7 +271,7 @@ func getHostEndpoint(ctx context.Context, request interface{}, svc fleet.Service } func (svc *Service) GetHost(ctx context.Context, id uint) (*fleet.HostDetail, error) { - alreadyAuthd := svc.authz.IsAlreadyAuthorized(ctx) + alreadyAuthd := svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceToken) if !alreadyAuthd { // First ensure the user has access to list hosts, then check the specific // host once team_id is loaded. @@ -555,7 +556,7 @@ func refetchHostEndpoint(ctx context.Context, request interface{}, svc fleet.Ser } func (svc *Service) RefetchHost(ctx context.Context, id uint) error { - if !svc.authz.IsAlreadyAuthorized(ctx) { + if !svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceToken) { if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return err } @@ -671,7 +672,7 @@ func listHostDeviceMappingEndpoint(ctx context.Context, request interface{}, svc } func (svc *Service) ListHostDeviceMapping(ctx context.Context, id uint) ([]*fleet.HostDeviceMapping, error) { - if !svc.authz.IsAlreadyAuthorized(ctx) { + if !svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceToken) { if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return nil, err } @@ -715,7 +716,7 @@ func getMacadminsDataEndpoint(ctx context.Context, request interface{}, svc flee } func (svc *Service) MacadminsData(ctx context.Context, id uint) (*fleet.MacadminsData, error) { - if !svc.authz.IsAlreadyAuthorized(ctx) { + if !svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceToken) { if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return nil, err } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 9a346b023a..a968253f93 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -3512,6 +3512,11 @@ func (s *integrationTestSuite) TestDeviceAuthenticatedEndpoints() { t := s.T() hosts := s.createHosts(t) + ac, err := s.ds.AppConfig(context.Background()) + require.NoError(t, err) + ac.OrgInfo.OrgLogoURL = "http://example.com/logo" + err = s.ds.SaveAppConfig(context.Background(), ac) + require.NoError(t, err) // create some mappings and MDM/Munki data s.ds.ReplaceHostDeviceMapping(context.Background(), hosts[0].ID, []*fleet.HostDeviceMapping{ @@ -3537,16 +3542,17 @@ func (s *integrationTestSuite) TestDeviceAuthenticatedEndpoints() { res.Body.Close() // get host with valid token - var getHostResp getHostResponse + var getHostResp getDeviceHostResponse res = s.DoRawNoAuth("GET", "/api/v1/fleet/device/"+token, nil, http.StatusOK) json.NewDecoder(res.Body).Decode(&getHostResp) res.Body.Close() require.Equal(t, hosts[0].ID, getHostResp.Host.ID) require.False(t, getHostResp.Host.RefetchRequested) + require.Equal(t, "http://example.com/logo", getHostResp.OrgLogoURL) hostDevResp := getHostResp.Host // make request for same host on the host details API endpoint, responses should match - getHostResp = getHostResponse{} + getHostResp = getDeviceHostResponse{} s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/hosts/%d", hosts[0].ID), nil, http.StatusOK, &getHostResp) require.Equal(t, hostDevResp, getHostResp.Host) @@ -3555,7 +3561,7 @@ func (s *integrationTestSuite) TestDeviceAuthenticatedEndpoints() { res.Body.Close() // host should have that flag turned to true - getHostResp = getHostResponse{} + getHostResp = getDeviceHostResponse{} res = s.DoRawNoAuth("GET", "/api/v1/fleet/device/"+token, nil, http.StatusOK) json.NewDecoder(res.Body).Decode(&getHostResp) res.Body.Close()