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

feat(draft): Authz #3008

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

feat(draft): Authz #3008

wants to merge 15 commits into from

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Apr 21, 2024

Ive gone around the houses with this for several days, so decided its best to open this up for feedback before I go too much further and start adding tests, etc. There's also several open questions that I could use some help addressing.

The goal of this PR is to start down the path of adding authorization (authz) to Flipt.

This has been a heavily requested feature recently, ex: #2756

While this initial functionality will not support namespace-based authz, it lays the foundation for it by implementing global authz.

The idea basic idea is to add Role Based Access Control (RBAC) support to Flipt using Open Policy Agent for both the 'language' used to define the authz policies as well as the enforcement library.

I think that using OPA in this way continues with Flipt's ethos of:

  1. Being self-contained/not requiring external services to function (ie not requiring our users to run an authorization service like Authz/SpiceDB, Topaz, etc)
  2. Being extremely configurable/composable. (described below)
  3. Sane defaults (the default policy defined 3 roles 'admin', 'editor' and 'viewer' with what I think are pretty sane rules)

What/How

This will allow users to enable authz and assign users one of the 3 default global roles which will be mapped to a user via the authentication system of choice (more in the next section).

Each API request to Flipt that is to be authorized (management API only) is annotated with both a subject and action such as flag:create, similar to our already existing audit events.

The authz policy defines which roles can perform which actions on which subjects and is enforced via a middleware similar to how our authentication works.

Our existing audit events were changed to use these new request types as well, since they map 1-1 to audit events that we already publish.

Configurability

The goal is to eventually allow operators to define their own policy rules via JSON (or YAML) that can override our default policy rules.

This is not part of this PR, but it is the reason I chose to load the policy rules using rego/OPA's data API.
We will validate that the operator's policy data conforms to our expected schema using JSON schema (part of this PR).

The rego rules should not need to be adjusted or overridden (ideally) in this manner since the rules are abstracted via this data API.

In the future we can support loading these rules from:

  • filesystem
  • git
  • oci
  • objectstores

Mapping Authn to Authz

A key aspect of this functionality that is currently not implemented is the mapping of a user to a role.
AFAIK, most OIDC providers have the notion of roles and allow enriching the tokens they create with custom attributes such as roles.

OIDC Providers:

We'll need a mechanism to then map the roles in the OIDC token at authn time to the token that we create in Flipt and then get stored in the token metadata (here in this PR)

This is one of the major open questions I have with this approach, how to best represent this mapping, perhaps in our existing config file?

Future

Namespaced Authz and Custom Roles

If we can figure out how to do ☝🏻 , then I think the next step would be to add support for namespace based authz, so that users could have different permissions depending on the namespace of the data they are accessing. This is ultimately what is being asked for in #2756

I think for this we could implement it one of two ways:

  1. Add support for mapping namespaces to roles (default and custom) in the UI. This would likely mean we could only support it for the database backends. This however would ensure that there is strong consitency between namespaces and their auth policies
  2. Add support for mapping namespaces to roles in our config file format. This would allow us to support authz with namespace roles for all of our backends, but would be not as easy to setup, and it would be more difficult to support consistency between namespaces and their role mappings since a user could map a role to a namespace that doesn't exist via typo which could introduce a security hole. Maybe we could fix this with validation when loading the namespace mapping?

Open Questions

  • Can most OIDC providers support writing user roles to a property in the token they create for authn?
  • What about non-OIDC user session auth like GitHub? Would we leverage team membership?
  • How can we support user's configuring custom authz policies? I think this is mostly solved by defining the 'schema' required to integrate with our OPA data API, but we still need to do the plumbing to be able to load policies from other locations (like filesystem, oci, git, objectstore, etc)
  • How do we enable configuring the mapping of the authn role to the token metadata that flipt creates?
  • How can we extend this to ultimately support namespace authz? Does this require UI / database or would we do this using a config file/format?

Anyways, would love any and all feedback!

cc @piclemx @erka @GeorgeMac

if action != "" {
event = audit.NewEvent(audit.FlagType, action, actor, audit.NewFlag(r))
}
event = audit.NewEvent(request, actor, audit.NewFlag(r))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can likely clean this up a bit by having audit.NewEvent also know how to wrap the resp r with the appropriate type maybe? so we don't have to have this switch statement

{
"version": "0.1.0",
"roles": [
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to rethink this policy schema to support namespaces @GeorgeMac

feat: impl authz middleware

feat: impl authz middleware

chore: fix panic and bad redux selector

chore: fmt ui

chore: refactor

chore: fix build, change to single role, default role

chore: fix build, change to single role, default role

chore: rm unneeded files

feat: configurable roles/policies

chore: config schema and tests

chore: mv back events to audit package

chore: reset ui folder

chore: revert ui back to main

chore: policy schema, visibility of errors

chore: add policy schema test

chore: rebase on main

Signed-off-by: Mark Phelps <[email protected]>
markphelps and others added 12 commits May 20, 2024 19:09
* chore: fix tests, add role attribute path / role mapping to oidc server tests

Signed-off-by: Mark Phelps <[email protected]>

* chore: authz middleware tests

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix audit tests

Signed-off-by: Mark Phelps <[email protected]>

* chore: proto regen

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix marshal audit events behaviour

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix failing test

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
chore: refactor request models to include scope
…3108)

* refactor(server/authz): rename scope to resource

Signed-off-by: George MacRorie <[email protected]>

* feat(config/authz): add policy and data source configuration

Signed-off-by: George MacRorie <[email protected]>

* refactor(server/authz): make policy and data external dependencies

Signed-off-by: George MacRorie <[email protected]>

* refactor(cmd/grpc): integrate new authz Engine changes

Signed-off-by: George MacRorie <[email protected]>

* fix(server/authz): ensure error is captured in return

Signed-off-by: George MacRorie <[email protected]>

* fix(config): allow policy and data sources to be empty

Signed-off-by: George MacRorie <[email protected]>

* refactor(server/authz): support separate poll durations for policy and data

Signed-off-by: George MacRorie <[email protected]>

* fix(config): validate non zero poll duration for authz sources

Signed-off-by: George MacRorie <[email protected]>

* fix(cmd/grpc): calls to authz engine with changes to polling

Signed-off-by: George MacRorie <[email protected]>

---------

Signed-off-by: George MacRorie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants