Convert role and API management to REST API

[billkalter]

Github Issue

50

What Are We Doing Here?

This unfortunately sizable PR addresses a single purpose: convert role and API key management to a publicly-accessible RESTful front end. As much as possible this was broken into smaller, logical commits. The general flow the PR is:

• First several commits:
  • Small fixes or modifications. These are typically minor things that, on their own, likely wouldn't warrant a PR, but each one blocks a bit of what's necessary to convert to a RESTful API.
• "Added user access control API and client"
  • Defines the public-facing user access control (UAC) API and client implementaiton.
• "Refactored API key and role management as public resources"
  • Building on the previous commit, this is where the server backend implements the resources defined by the UAC client. This and the previous commit represent the bulk of this PR.
• "Updated documentation"
  • Adds and updates MD files to define the new API. This may actually be a good starting point to reference what, from a client perspective, has changed with this PR.
• Remaining commits:
  • The remaining commits fall into one of two categories:
    1. Additional features not core to role and API key management but which are desirable. These were always intended to be part of the implementation, but to keep the core changes separate they were done in separate commits. For example, the ability to query whether a particular role or key has permission to perform X is useful but not critical, so it that functionality is added here.
    2. Very small updates found after the rest of the work was done. Not really large enough to warrant fixing and rebasing all of the previous commits.

How to Test and Verify

1. Check out this PR
2. Go through the API key management README
3. Create roles and API keys to verify all works as expected. Much of this is already documented in the README so it would be redundant to repeat here.

Risk

High. The actual changes aren't particularly complex, but this PR pulls out the old role and API key management system and replaces it with a new one, so any errors may negatively affect the ability to manage these.

Having said that, the backend for roles and API keys is unchanged, so if there were a breaking issue with the interface the system would still work with the existing set of roles and API keys, but until it were fixed there'd be no way to alter that set.

Level

High

Required Testing

Manual

Code Review Checklist

• Tests are included. If not, make sure you leave us a line or two for the reason.

• Pulled down the PR and performed verification of at least being able to
  build and run.

• Well documented, including updates to any necessary markdown files. When
  we inevitably come back to this code it will only take hours to figure out, not
  days.

• Consistent/Clear/Thoughtful? We are better with this code. We also aren't
  a victim of rampaging consistency, and should be using this course of action.
  We don't have coding standards out yet for this project, so please make sure to address any feedback regarding STYLE so the codebase remains consistent.

• PR has a valid summary, and a good description.

[bv-jenkins]

This PR was tested by custom branch build on jenkins

[fahdsiddiqui]

What is the reasoning behind not throwing a 403 ? Is it a security measure by design?

[billkalter]

It's because there is no overarching permission such as "user has permission to view roles". Instead there are permissions to filter which roles a user can read, such as role|read|your_team|*. So functionally the service iterates over matching roles and filters out those for which the user doesn't have read permission. As an optimization Emo could check whether any of the user's permissions give her permission to view any possible matching role and return immediately if not (e.g.; pre-check whether the user has any permission starting withrole|read). Even if this were implemented I'd think it would still return an empty list, otherwise the response would be very confusing. Just because the caller doesn't have permission to view any existing roles today doesn't mean a new role won't be created tomorrow for which she does have read access. So returning 403 today and a list with the one role she has read permission for tomorrow is inconsistent; only the data changed, not her permissions.

[fahdsiddiqui]

Just looked at "Added user access control API and client" commit. Looks great 👍 , just one question.

[bv-jenkins]

This PR was tested by custom branch build on jenkins

[bv-jenkins]

This PR was tested by custom branch build on jenkins

[sujithvaddi]

@billkalter 👍

• also, verified all the endpoints after the latest commit.