[jira] [Created] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions

["Alexander Kolbasov (JIRA)" <ji...@apache.org>]

Alexander Kolbasov created SENTRY-1546:
------------------------------------------

             Summary: Generic Policy provides bad error messages for Sentry exceptions
                 Key: SENTRY-1546
                 URL: https://issues.apache.org/jira/browse/SENTRY-1546
             Project: Sentry
          Issue Type: Bug
    Affects Versions: 1.8.0
            Reporter: Alexander Kolbasov
            Priority: Minor


I discovered that when you attempt to create a role that already exists the error message you get back from Thrift i just 'Role: foo' which is very confusing.

The reason is that the SentryStore throws 
{code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
and the generic policy processor passes the message as is:

{code}
  public TCreateSentryRoleResponse create_sentry_role(
      final TCreateSentryRoleRequest request) throws TException {
    Response<Void> respose = requestHandle(new RequestHandler<Void>() {
      @Override
      public Response<Void> handle() throws Exception {
        validateClientVersion(request.getProtocol_version());
        authorize(request.getRequestorUserName(),
            getRequestorGroups(conf, request.getRequestorUserName()));
        store.createRole(request.getComponent(), request.getRoleName(),
                request.getRequestorUserName());
        return new Response<Void>(Status.OK());
      }
    });
...
{code}

The similar thing is happening for other requests and other Sentry-specific exceptions.

The legacy policy processor does decorate the error a bit:

{code}
  public TCreateSentryRoleResponse create_sentry_role(
    TCreateSentryRoleRequest request) throws TException {
    final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
    TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
    try {
      validateClientVersion(request.getProtocol_version());
      authorize(request.getRequestorUserName(),
          getRequestorGroups(request.getRequestorUserName()));
      sentryStore.createSentryRole(request.getRoleName());
      response.setStatus(Status.OK());
      notificationHandlerInvoker.create_sentry_role(request, response);
    } catch (SentryAlreadyExistsException e) {
      String msg = "Role: " + request + " already exists.";
      LOGGER.error(msg, e);
      response.setStatus(Status.AlreadyExists(msg, e));
    } catch (SentryAccessDeniedException e) {
      LOGGER.error(e.getMessage(), e);
      response.setStatus(Status.AccessDenied(e.getMessage(), e));
    } catch (SentryThriftAPIMismatchException e) {
      LOGGER.error(e.getMessage(), e);
      response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
    } catch (Exception e) {
      String msg = "Unknown error for request: " + request + ", message: " + e.getMessage();
      LOGGER.error(msg, e);
      response.setStatus(Status.RuntimeError(msg, e));
    } finally {
...
{code}

I think that it is better to just put the right message in the exception itself and do not decorate it later.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)