Skip to content

Commit

Permalink
cloud: fix(auth): preserve the status code while returning error (#7832
Browse files Browse the repository at this point in the history
…) (#7834)

* fix(auth): preserve the status code while returning error (#7832)

Clients like dgo rely on GRPC status code for operations in scenarios like relogin if JWT expired. We must preserve the status code while returning and not just wrap it.

(cherry picked from commit 1a92da4)

* nit

(cherry picked from commit ce63209)
  • Loading branch information
NamanJain8 authored and ahsanbarkati committed May 27, 2021
1 parent f172024 commit eca6585
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 11 deletions.
12 changes: 9 additions & 3 deletions edgraph/access_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -1077,27 +1077,33 @@ func authorizeSchemaQuery(ctx context.Context, er *query.ExecutionResult) error
// AuthGuardianOfTheGalaxy authorizes the operations for the users who belong to the guardians
// group in the galaxy namespace. This authorization is used for admin usages like creation and
// deletion of a namespace, resetting passwords across namespaces etc.
// NOTE: The caller should not wrap the error returned. If needed, propagate the GRPC error code.
func AuthGuardianOfTheGalaxy(ctx context.Context) error {
if !x.WorkerConfig.AclEnabled {
return nil
}
ns, err := x.ExtractJWTNamespace(ctx)
if err != nil {
return errors.Wrap(err, "Authorize guradian of the galaxy, extracting jwt token, error:")
return status.Error(codes.Unauthenticated,
"AuthGuardianOfTheGalaxy: extracting jwt token, error: "+err.Error())
}
if ns != 0 {
return errors.New("Only guardian of galaxy is allowed to do this operation")
return status.Error(
codes.PermissionDenied, "Only guardian of galaxy is allowed to do this operation")
}
// AuthorizeGuardians will extract (user, []groups) from the JWT claims and will check if
// any of the group to which the user belongs is "guardians" or not.
if err := AuthorizeGuardians(ctx); err != nil {
return errors.Wrap(err, "AuthGuardianOfTheGalaxy, failed to authorize guardians")
s := status.Convert(err)
return status.Error(
s.Code(), "AuthGuardianOfTheGalaxy: failed to authorize guardians. "+s.Message())
}
glog.V(3).Info("Successfully authorised guardian of the galaxy")
return nil
}

// AuthorizeGuardians authorizes the operation for users which belong to Guardians group.
// NOTE: The caller should not wrap the error returned. If needed, propagate the GRPC error code.
func AuthorizeGuardians(ctx context.Context) error {
if len(worker.Config.HmacSecret) == 0 {
// the user has not turned on the acl feature
Expand Down
15 changes: 10 additions & 5 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ func parseSchemaFromAlterOperation(ctx context.Context, op *api.Operation) (*sch
// Only the guardian of the galaxy can do a galaxy wide query/mutation. This operation is
// needed by live loader.
if err := AuthGuardianOfTheGalaxy(ctx); err != nil {
return nil, errors.Wrap(err, "Non guardian of galaxy user cannot bypass namespaces.")
s := status.Convert(err)
return nil, status.Error(s.Code(),
"Non guardian of galaxy user cannot bypass namespaces. "+s.Message())
}
var err error
namespace, err = strconv.ParseUint(x.GetForceNamespace(ctx), 0, 64)
Expand Down Expand Up @@ -383,8 +385,9 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
return empty, errors.New("Drop all operation is not permitted.")
}
if err := AuthGuardianOfTheGalaxy(ctx); err != nil {
return empty, errors.Wrapf(err, "Drop all can only be called by the guardian of the"+
" galaxy")
s := status.Convert(err)
return empty, status.Error(s.Code(),
"Drop all can only be called by the guardian of the galaxy. "+s.Message())
}
if len(op.DropValue) > 0 {
return empty, errors.Errorf("If DropOp is set to ALL, DropValue must be empty")
Expand Down Expand Up @@ -1214,11 +1217,13 @@ func (s *Server) doQuery(ctx context.Context, req *Request) (
ostats.Record(ctx, x.NumMutations.M(1))
}

if x.IsGalaxyOperation(ctx) {
if req.doAuth == NeedAuthorize && x.IsGalaxyOperation(ctx) {
// Only the guardian of the galaxy can do a galaxy wide query/mutation. This operation is
// needed by live loader.
if err := AuthGuardianOfTheGalaxy(ctx); err != nil {
return nil, errors.Wrap(err, "Non guardian of galaxy user cannot bypass namespaces.")
s := status.Convert(err)
return nil, status.Error(s.Code(),
"Non guardian of galaxy user cannot bypass namespaces. "+s.Message())
}
}

Expand Down
7 changes: 4 additions & 3 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2600,9 +2600,10 @@ func assertNonGuardianFailure(t *testing.T, queryName string, respIsNull bool,
resp := makeRequestAndRefreshTokenIfNecessary(t, token, params)

require.Len(t, resp.Errors, 1)
require.Contains(t, resp.Errors[0].Message,
fmt.Sprintf("rpc error: code = PermissionDenied desc = Only guardians are allowed access."+
" User '%s' is not a member of guardians group.", userid))
require.Contains(t, resp.Errors[0].Message, "rpc error: code = PermissionDenied")
require.Contains(t, resp.Errors[0].Message, fmt.Sprintf(
"Only guardians are allowed access. User '%s' is not a member of guardians group.",
userid))
if len(resp.Data) != 0 {
queryVal := "null"
if !respIsNull {
Expand Down

0 comments on commit eca6585

Please sign in to comment.