Skip to content

Auth checks return the wrong http status code when aclErrorStatus is set to 403 #1812

Description

@simoami

When aclErrorStatus is set to 403, it fails one use case, when the accessToken is supplied but invalid.

Consider the following use cases:

  1. attempt to access a protected resource without creds (missing token): 401(a auth/login failure)
  2. attempt to access a protected resource with invalid creds (fake token): 401 (a auth/login failure)
  3. attempt to access a protected resource with expired creds (old token): 401 (a auth/login failure)
  4. attempt to access a protected resource with valid creds (valid token) that requires an elevated permission (higher role) : 403 (access denied)

Out of these 4 use cases, # 2 and # 3 fail to return 401 and instead return 403. This is because in application.js, the logic that returns 401 does not check for a valid token (i.e. if the token has a valid userId), leaving further ACL failures to default to aclErrorStatus, which is set to 403. See annotated code below:

application.js (line 343)

    var errStatusCode = modelSettings.aclErrorStatus || app.get('aclErrorStatus') || 401;

    /////// HANDLE 401 USE CASES
    if (!req.accessToken) { // <- this lets a fake or expired token get away from a 401
      errStatusCode = 401;
    }
    ////// LAST CHANCE FOR 401 USE CASES ENDS HERE

    ////// EVERYTHING BELOW USES errStatusCode
    if (Model.checkAccess) {
      ...
    }

Suggested fix:

    var errStatusCode = modelSettings.aclErrorStatus || app.get('aclErrorStatus') || 401;

    if (!req.accessToken || !req.accessToken.userId) { // Checks both the presence and validity of a token
      errStatusCode = 401;
    }
    if (Model.checkAccess) {
      ...
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions