Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor API for consistency #59

Closed
charithe opened this issue Apr 21, 2021 · 3 comments · Fixed by #60
Closed

Refactor API for consistency #59

charithe opened this issue Apr 21, 2021 · 3 comments · Fixed by #60
Labels
Milestone

Comments

@charithe
Copy link
Contributor

We have /api/check for checking a single principal and a single resource against a single action. It returns a response like the following.

{
  "requestId": "460d1429-9798-4a6f-8505-170193909003",
  "statusCode": 200,
  "statusMessage": "Allow",
  "effect": "EFFECT_ALLOW",
  "meta": {
    "matchedPolicy": "leave_request:20210210",
    "effectiveDerivedRoles": [
      "any_employee",
      "direct_manager"
    ],
    "evaluationDuration": "0.000628452s"
  }
}

Then we have the /api/check_resource_batch endpoint which checks a single principal against multiple actions and multiple resources.

{
  "requestId":  "test",
  "resourceInstances":  {
    "XX125":  {
      "actions":  {
        "approve":  "EFFECT_DENY",
        "create":  "EFFECT_DENY",
        "view:public":  "EFFECT_ALLOW"
      }
    },
    "XX150":  {
      "actions":  {
        "approve":  "EFFECT_DENY",
        "create":  "EFFECT_DENY",
        "view:public":  "EFFECT_ALLOW"
      }
    },
    "XX250":  {
      "actions":  {
        "approve":  "EFFECT_DENY",
        "create":  "EFFECT_DENY",
        "view:public":  "EFFECT_ALLOW"
      }
    },
    "YY100":  {
      "actions":  {
        "approve":  "EFFECT_ALLOW",
        "create":  "EFFECT_ALLOW",
        "view:public":  "EFFECT_ALLOW"
      }
    },
    "YY200":  {
      "actions":  {
        "approve":  "EFFECT_ALLOW",
        "create":  "EFFECT_ALLOW",
        "view:public":  "EFFECT_ALLOW"
      }
    }
  }
}

This is not ideal because we have two wildly different responses for very similar requests. The /api/check endpoint is strictly not necessary because it can be subsumed by the /api/check_resource_batch endpoint.

Proposal

  • Remove the current /api/check endpoint.
  • Rename /api/check_resource_batch to /api/check and make it the default endpoint.
  • introduce a debug parameter to the request so that we can add debug metadata (matched policy, active derived roles) to the response.
@charithe charithe added this to the MVP milestone Apr 21, 2021
@emreb
Copy link
Contributor

emreb commented Apr 21, 2021

Would the format of a single check still be the same?

@charithe
Copy link
Contributor Author

No, it would be replaced by the batch request where the batch size is 1.

@charithe
Copy link
Contributor Author

Fixed by #60

@charithe charithe linked a pull request Apr 23, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants