flagr icon indicating copy to clipboard operation
flagr copied to clipboard

[WIP] AuthZ in flagr

Open wildefires opened this issue 4 years ago • 6 comments

Note: This is still a WIP, putting up the PR to solicit early feedback

Description

This PR is, when complete, meant to introduce AuthZ to flagr. It does not implement AuthN

Motivation and Context

Allows users to lock down flags from unauthorized users making changes. While changes within an organization are almost certainly well-intentioned, not everyone understands all the wide-reaching effects a change to a configuration can have. This allows flag owners to have more control over who can change it.

I've introduced the concept of authorized users and groups. Non-authorized users will be able to view, but not change fields

How Has This Been Tested?

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ ] My code follows the code style of this project.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.

wildefires avatar Apr 14 '20 03:04 wildefires

This is based on a relatively old version of master (sometime early last week, before @dcramer 's additions) thus the merge conflict.

@zhouzhuojie curious of your thoughts on approach for this

wildefires avatar Apr 14 '20 03:04 wildefires

image

This is the card that is added just under the base information

And an example of a snapshot where one of the fields changes...

image

wildefires avatar Apr 14 '20 03:04 wildefires

Codecov Report

Merging #356 into master will decrease coverage by 0.21%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #356      +/-   ##
==========================================
- Coverage   81.75%   81.54%   -0.22%     
==========================================
  Files          26       26              
  Lines        1540     1544       +4     
==========================================
  Hits         1259     1259              
- Misses        211      213       +2     
- Partials       70       72       +2     
Impacted Files Coverage Δ
pkg/entity/flag.go 94.28% <ø> (ø)
pkg/handler/crud.go 88.66% <0.00%> (-1.02%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9eac7e7...51684f5. Read the comment docs.

codecov-io avatar Apr 14 '20 03:04 codecov-io

On our internal fork we plan to use an internal auth mechanism that we have for this. What kind of "backend" makes sense for the "default" out of the box solution?

I was thinking defining a yaml file of groups and having the username / groups passed in via the Header. Not sure how to make that happen with the UI, however

wildefires avatar Apr 14 '20 17:04 wildefires

Thanks, what do you think of https://casbin.org/? I'm thinking if we can generalize the authz so that it works for many use cases, and the authz policy itself can be decoupled as a config setting.

zhouzhuojie avatar Apr 14 '20 20:04 zhouzhuojie

Would be great if we can provide some kind of interface for this so we can swap to use other implementations.

pacoguzman avatar Apr 21 '20 10:04 pacoguzman

Stale pull request message

github-actions[bot] avatar Aug 26 '22 21:08 github-actions[bot]