From 2ac77cff74d504b4bd1530739071655a93fffff2 Mon Sep 17 00:00:00 2001 From: Mike Cohen Date: Mon, 8 Jul 2024 13:00:39 +1000 Subject: [PATCH] Bugfix: Report access denied to org errors. (#3596) Previously the org was just automatically switched to another valid org when a user had no access to that org. This caused a problem when the user was an org admin so could switch to any org, but didnt have permission in the new org. This caused the app to switch to another org but the org selector still had the old org shown. This resulted in a confusing UI when the org selector was showing an org but the application was actually communicating with a different org. This change propagates the access denied error to ensure the user is aware they can not connect to the org they wanted rather than automatically switch to another org. Also fixed a GUI bug where fractional timestamps were not properly shown. --- api/authenticators/basic.go | 7 +++++-- api/authenticators/orgs.go | 15 +++++++++++++-- api/download.go | 2 -- api/static.go | 4 ++-- .../src/components/artifacts/reporting.jsx | 4 ++-- gui/velociraptor/src/components/utils/time.jsx | 18 ++++++++++++++++-- services/orgs.go | 2 +- services/orgs/orgs.go | 6 +++--- utils/errors.go | 1 + vql/functions/humanize.go | 2 +- 10 files changed, 44 insertions(+), 17 deletions(-) diff --git a/api/authenticators/basic.go b/api/authenticators/basic.go index 4480720d51c..cf3bab449b7 100644 --- a/api/authenticators/basic.go +++ b/api/authenticators/basic.go @@ -110,12 +110,15 @@ func (self *BasicAuthenticator) AuthenticateUserHandler( err = CheckOrgAccess(self.config_obj, r, user_record) if err != nil { services.LogAudit(r.Context(), - self.config_obj, user_record.Name, "Unauthorized username", + self.config_obj, user_record.Name, "User Unauthorized for Org", ordereddict.NewDict(). + Set("err", err.Error()). Set("remote", r.RemoteAddr). Set("status", http.StatusUnauthorized)) - http.Error(w, "authorization failed", http.StatusUnauthorized) + // Return status forbidden because we dont want the user + // to reauthenticate + http.Error(w, err.Error(), http.StatusForbidden) return } diff --git a/api/authenticators/orgs.go b/api/authenticators/orgs.go index 0b249e85e40..504e8497ac3 100644 --- a/api/authenticators/orgs.go +++ b/api/authenticators/orgs.go @@ -2,6 +2,7 @@ package authenticators import ( "errors" + "fmt" "net/http" "net/url" @@ -9,6 +10,7 @@ import ( api_proto "www.velocidex.com/golang/velociraptor/api/proto" config_proto "www.velocidex.com/golang/velociraptor/config/proto" "www.velocidex.com/golang/velociraptor/services" + "www.velocidex.com/golang/velociraptor/utils" ) func GetOrgIdFromRequest(r *http.Request) string { @@ -44,7 +46,7 @@ func GetOrgIdFromRequest(r *http.Request) string { // requested. If they do not have access to the org they requested we // switch them to any org in which they have at least read // access. This behaviour ensures that when a user's access is removed -// from an org the GUI immediately switched to the next available org. +// from an org the GUI immediately switches to the next available org. func CheckOrgAccess( config_obj *config_proto.Config, r *http.Request, @@ -56,6 +58,14 @@ func CheckOrgAccess( return nil } + // For the root org or an unknown org we switch to another org, + // otherwise we need to give the user a more specific error that + // they are not authorized for this org. + if !utils.IsRootOrg(org_id) && + !errors.Is(err, services.OrgNotFoundError) { + return err + } + ctx := r.Context() // Does the user already have a preferred org they want to be in? @@ -111,7 +121,8 @@ func _checkOrgAccess(r *http.Request, org_id string, user_record *api_proto.Velo } if !perm || user_record.Locked { - return errors.New("Unauthorized username") + return fmt.Errorf("User %v accessing %v: %w", + user_record.Name, org_id, utils.NoAccessToOrgError) } return nil diff --git a/api/download.go b/api/download.go index a4c1a982524..64f0747eb8a 100644 --- a/api/download.go +++ b/api/download.go @@ -128,8 +128,6 @@ func vfsFileDownloadHandler() http.Handler { org_id = utils.NormalizedOrgId(org_id) - utils.Debug(org_id) - org_manager, err := services.GetOrgManager() if err != nil { returnError(w, 404, err.Error()) diff --git a/api/static.go b/api/static.go index 2efaaa095d8..5abc3338a53 100644 --- a/api/static.go +++ b/api/static.go @@ -93,14 +93,14 @@ func (self *CachedFilesystem) Open(name string) (http.File, error) { // We do not support gz files at all - it is either brotli or // uncompressed. if strings.HasSuffix(name, ".gz") { - return nil, services.NotFoundError + return nil, services.OrgNotFoundError } fd, err := self.FileSystem.Open(name) if err != nil { // If there is not brotli file, it is just not there. if strings.HasSuffix(name, ".br") { - return nil, services.NotFoundError + return nil, services.OrgNotFoundError } // Check if a compressed .br file exists diff --git a/gui/velociraptor/src/components/artifacts/reporting.jsx b/gui/velociraptor/src/components/artifacts/reporting.jsx index ee101f88dfa..2adf763a262 100644 --- a/gui/velociraptor/src/components/artifacts/reporting.jsx +++ b/gui/velociraptor/src/components/artifacts/reporting.jsx @@ -121,9 +121,9 @@ export default class VeloReportViewer extends React.Component { } this.setState(new_state); }).catch((err) => { - let response = err.response && err.response.data; + let response = err.response && (err.response.data || err.response.message); if (response) { - let templ = "
" + response.message + "
"; + let templ = "
" + response + "
"; this.setState({"template": templ,loading: false}); } else { this.setState({"template": "Error " + err.message, loading: false}); diff --git a/gui/velociraptor/src/components/utils/time.jsx b/gui/velociraptor/src/components/utils/time.jsx index 5ca12141837..654d873e72a 100644 --- a/gui/velociraptor/src/components/utils/time.jsx +++ b/gui/velociraptor/src/components/utils/time.jsx @@ -103,12 +103,26 @@ class VeloTimestamp extends Component { } let timezone = this.context.traits.timezone || "UTC"; - var when = moment(ts); + let when = moment(ts); + let when_tz = moment.tz(when, timezone); + let formatted_ts = when_tz.format("YYYY-MM-DDTHH:mm:ss"); + + let fractional_part = when_tz.format(".SSS"); + if (fractional_part === ".000") { + fractional_part = ""; + } + formatted_ts += fractional_part; + if (when_tz.isUtc()) { + formatted_ts += "Z"; + } else { + formatted_ts += when_tz.format("Z"); + } + return renderToolTip(props, ts)}>
- {moment.tz(when, timezone).format()} + {formatted_ts}
; }; diff --git a/services/orgs.go b/services/orgs.go index 79d3d8797c0..6d1503819b9 100644 --- a/services/orgs.go +++ b/services/orgs.go @@ -19,7 +19,7 @@ var ( mu sync.Mutex org_manager OrgManager - NotFoundError = errors.New("Org not found") + OrgNotFoundError = errors.New("Org not found") ) // Currently the org manager is the only binary wide global - all diff --git a/services/orgs/orgs.go b/services/orgs/orgs.go index def2e39b44b..430558d9be0 100644 --- a/services/orgs/orgs.go +++ b/services/orgs/orgs.go @@ -99,7 +99,7 @@ func (self *OrgManager) GetOrgConfig(org_id string) (*config_proto.Config, error result, pres := self.orgs[org_id] if !pres { - return nil, services.NotFoundError + return nil, services.OrgNotFoundError } return result.config_obj, nil } @@ -112,7 +112,7 @@ func (self *OrgManager) GetOrg(org_id string) (*api_proto.OrgRecord, error) { result, pres := self.orgs[org_id] if !pres { - return nil, services.NotFoundError + return nil, services.OrgNotFoundError } return result.record, nil } @@ -129,7 +129,7 @@ func (self *OrgManager) OrgIdByNonce(nonce string) (string, error) { result, pres := self.org_id_by_nonce[nonce] if !pres { - return "", services.NotFoundError + return "", services.OrgNotFoundError } return result, nil } diff --git a/utils/errors.go b/utils/errors.go index 630d3d31b73..65eb3e4ea87 100644 --- a/utils/errors.go +++ b/utils/errors.go @@ -19,6 +19,7 @@ var ( NotFoundError = Wrap(os.ErrNotExist, "NotFoundError") InvalidArgError = errors.New("InvalidArgError") IOError = errors.New("IOError") + NoAccessToOrgError = errors.New("No access to org") ) // This is a custom error type that wraps an inner error but does not diff --git a/vql/functions/humanize.go b/vql/functions/humanize.go index e692dcfccb3..29b12d103c2 100644 --- a/vql/functions/humanize.go +++ b/vql/functions/humanize.go @@ -61,7 +61,7 @@ func (self *HumanizeFunction) Call(ctx context.Context, return humanize.Bytes(uint64(arg.Bytes)) } - return humanize.IBytes(uint64(arg.Bytes)) + return humanize.IBytes(uint64(arg.IBytes)) } func (self HumanizeFunction) Info(scope vfilter.Scope, type_map *vfilter.TypeMap) *vfilter.FunctionInfo {