fix: check authz for evaluation endpoints

[markphelps]

Reported in Discord: https://discord.com/channels/960634591000014878/1281742374275649569

A user reported that with authz enabled evaluation for client side SDKs did not work. This fixes that and adds more testing

Also disables authz checking for ofrep server endpoints

[codecov]

Codecov Report

Attention: Patch coverage is 87.50000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 64.66%. Comparing base (c4a0f96) to head (59773d1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/server/middleware/grpc/middleware.go	82.85%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3438      +/-   ##
==========================================
+ Coverage   64.63%   64.66%   +0.03%     
==========================================
  Files         174      174              
  Lines       13916    13918       +2     
==========================================
+ Hits         8994     9000       +6     
+ Misses       4231     4227       -4     
  Partials      691      691              

Flag	Coverage Δ
unittests	64.66% <87.50%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[GeorgeMac]

I think we need to update the authz engine to support requesting at least read flag in the requested namespace here. Or go as far as the authz engine supporting request more than one resource to do flag and segment.
by blanket disabling it you let anyone with authn but no authz read from any namespace.

[GeorgeMac]

I made a PR on this PR ☝️

[erka]

@markphelps is it any reason why ofrep and evaluation endpoints should not have authz?

[markphelps]

@markphelps is it any reason why ofrep and evaluation endpoints should not have authz?

I guess I could undo that change for now? although we skipped evaluation server-side authz on purpose it seems when we initially implemented it. I think it gets harder for bulk eval as idk if we wanna check each flag?

[erka]

as idk if we wanna check each flag?

What is it? I don't see any restrictions for a specific flag during CRUD.

We could have another action like evaluate for flag or namespace resource. It may be helpful for those who use one Flipt instance for different environments with different namespaces.

[GeorgeMac]

We could have another action like evaluate for flag or namespace resource. It may be helpful for those who use one Flipt instance for different environments with different namespaces.

My gut leans on adding a new action / verb as @erka suggests here, specifically for evaluations (eval + ofrep, but not necessarily snapshot data).
Long term, I think a specific verb/action makes sense. It makes it clearer what the intent is and allows e.g. audit log consumers to ignore evaluations (if they wish), which might be high-volume output. Adding in read flag and segment on evaluation kind of bulks up the audit log stream potentially quite a bit.

I think maybe read flag / segment might still make sense for the eval snapshot data endpoint though.
The caller is not specifically evaluating, they're reading all flags and segments in a namespace.
Since it is designed to be fetched less frequently that actual evaluations, it doesn't have the same impact on audit log volume. So less concerned about that in this instance.

[erka]

@markphelps @GeorgeMac Let's move this discussion to another issue or PR. I believe it's the best to resolve the original issue right now.

[markphelps]

@erka @GeorgeMac are we good to merge this to unblock the user in Discord? I agree we should prob discuss evaluation server side authz in another issue

[GeorgeMac]

Yes @markphelps 👍