Skip to content

Commit

Permalink
Bugfix: Report access denied to org errors. (Velocidex#3596)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
scudette authored Jul 8, 2024
1 parent bf3bd32 commit 2ac77cf
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 17 deletions.
7 changes: 5 additions & 2 deletions api/authenticators/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
15 changes: 13 additions & 2 deletions api/authenticators/orgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ package authenticators

import (
"errors"
"fmt"
"net/http"
"net/url"

"www.velocidex.com/golang/velociraptor/acls"
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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions api/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 2 additions & 2 deletions api/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions gui/velociraptor/src/components/artifacts/reporting.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<div class='no-content'>" + response.message + "</div>";
let templ = "<div class='no-content'>" + response + "</div>";
this.setState({"template": templ,loading: false});
} else {
this.setState({"template": "Error " + err.message, loading: false});
Expand Down
18 changes: 16 additions & 2 deletions gui/velociraptor/src/components/utils/time.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <OverlayTrigger
delay={{show: 250, hide: 400}}
overlay={(props)=>renderToolTip(props, ts)}>
<div className="timestamp">
{moment.tz(when, timezone).format()}
{formatted_ts}
</div>
</OverlayTrigger>;
};
Expand Down
2 changes: 1 addition & 1 deletion services/orgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions services/orgs/orgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions utils/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion vql/functions/humanize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 2ac77cf

Please sign in to comment.