refactor(server/authz): make policy and data external dependencies #3108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: George MacRorie <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
GeorgeMac
force-pushed
the
gm/authz-refactor-engine
branch
from
705d229 to
5959be4
Compare
Signed-off-by: George MacRorie <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
markphelps
reviewed
May 23, 2024
config/flipt.schema.cue
Outdated
| @@ -115,7 +115,15 @@ import "strings" | |||
| } | |||
|
|
|||
| #authorization: { | |||
| required?: bool | *false | |||
| #authorizationSource: { | |||
| backend: "filesystem" | |||
There was a problem hiding this comment.
What about 'local' to match storage for flag state?
There was a problem hiding this comment.
We can do that, do we want to change the recent addition to the storage.git.backend to also be local?
I was making this consistent with that.
There was a problem hiding this comment.
Since that is not out yet, we can make that change there too.
internal/server/authz/engine.go
Outdated
| return | ||
| case <-ticker.C: | ||
| if err := e.update(ctx, storage.ReplaceOp); err != nil { | ||
| e.logger.Error("updating policy and data", zap.Error(err)) |
There was a problem hiding this comment.
Just to double check, we want to update both policy and data on the same cadence?
There was a problem hiding this comment.
I started with that and then paired it back to be simpler, but if we ultimately want to managed those cadences separately, probably best to slip that in now so the config doesn't get weird 👍
There was a problem hiding this comment.
yeah i mean i guess it doesnt matter too much since its a noop of the file didn't change, other than checking the modtime. im wondering which file would realistically change more often? likely the data, but i think i agree that simple is better from a config aspect so im down to keep it as is. just wanted to double check
There was a problem hiding this comment.
I was already well down the make this work hole. Just spent a large portion of the day back in the complexities of our configuration 😂 All done now though.
…d data Signed-off-by: George MacRorie <[email protected]>
GeorgeMac
force-pushed
the
gm/authz-refactor-engine
branch
from
b78dfb3 to
0ed210a
Compare
Signed-off-by: George MacRorie <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
kodiakhq bot
added a commit
that referenced
this pull request
May 30, 2024
* feat(wip): authz/rbac 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]> * chore: start adding role attribute path/jmes * chore: mod tidy * Authz OIDC tests (#3098) * 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]> * chore: refactor request models to include scope Signed-off-by: Mark Phelps <[email protected]> * chore: fix engine_test * chore: make scope optional and use subject if not provided * chore: fix executor_test Signed-off-by: Mark Phelps <[email protected]> * chore: fix log sink test Signed-off-by: Mark Phelps <[email protected]> * chore: consolidate some auth metadata to make creating policies simpler (#3106) * refactor(server/authz): make policy and data external dependencies (#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]> * refactor(authz): pass entire request and authentication to IsAllowed (#3126) Signed-off-by: George MacRorie <[email protected]> * chore: set raw claims if they exist in authz metadata (#3125) * chore: go mod tidy Signed-off-by: Mark Phelps <[email protected]> * chore: set raw claims if they exist in authz metadata Signed-off-by: Mark Phelps <[email protected]> * chore: fix authn oidc server test Signed-off-by: Mark Phelps <[email protected]> * chore: skip authz on auth public server Signed-off-by: Mark Phelps <[email protected]> * chore: log for debugging Signed-off-by: Mark Phelps <[email protected]> --------- Signed-off-by: Mark Phelps <[email protected]> * fix: Authz fixes (#3132) * chore: go mod tidy Signed-off-by: Mark Phelps <[email protected]> * fix: authz endpoint skip for getauthself/deleteauthself Signed-off-by: Mark Phelps <[email protected]> * chore: rm claims unmarshal for now * chore: make authorization experimental Signed-off-by: Mark Phelps <[email protected]> * chore: add request methods to auth requests Signed-off-by: Mark Phelps <[email protected]> * chore: add schema * chore: set package name to flipt.authz.v1 * chore: fix telemetry test Signed-off-by: Mark Phelps <[email protected]> --------- Signed-off-by: Mark Phelps <[email protected]> * chore: rename poll duration to poll interval Signed-off-by: Mark Phelps <[email protected]> * chore: mod/work sync Signed-off-by: Mark Phelps <[email protected]> * chore: fix config test Signed-off-by: Mark Phelps <[email protected]> * chore: rm unused supports authz config; fmt cache config --------- Signed-off-by: Mark Phelps <[email protected]> Signed-off-by: George MacRorie <[email protected]> Co-authored-by: George <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes FLI-1028
This refactors the engine to source policy and optionally data from a configurable source.
Currently, this is simply a path on the filesystem which is periodically read.
The filesystem source uses mod time to avoid causing the policy engine to need to be recompiled or for the data to have to be updated when it does not change.
p.s. I renamed scope to resource because I think scope is often the tuple of subject/resource and action/verb (e.g.
flag:read).