NotificationsYou must be signed in to change notification settings
Fork928
Star10.1k

chore: break down dbauthz.System into smaller roles#6218

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

johnstcn merged 4 commits intomainfromcj/authz-system-breakup

Feb 15, 2023

Merged

chore: break down dbauthz.System into smaller roles#6218

johnstcn merged 4 commits intomainfromcj/authz-system-breakup

Feb 15, 2023

Conversation

Copy link

Member

johnstcn commentedFeb 15, 2023•
edited
Loading

rbac: export rbac.Permissions
dbauthz: move GetDeploymentDAUs, GetTemplateDAUs, GetTemplateAverageBuildTime from querier.go to system.go and removes auth checks
dbauthz: remove AsSystem(), add invididual roles for metrics cache, autostart, provisionerd, add restricted system role for everything else

Fixes#6158

chore: break down dbauthz.System into smaller roles

603ec8e

- rbac: export rbac.Permissions- dbauthz: move GetDeploymentDAUs, GetTemplateDAUs,  GetTemplateAverageBuildTime from querier.go to system.go  and removes auth checks- dbauthz: remove AsSystem(), add invididual roles for  metrics cache, autostart, provisionerd, add restricted  system role for everything else

johnstcn added the release/experimentalThese changes are feature-flagged, they may change or be removed in future releases label

Feb 15, 2023

johnstcn requested review fromspikecurtis andEmyrk

February 15, 2023 13:53

johnstcn self-assigned this

Feb 15, 2023

Emyrk approved these changes

Feb 15, 2023

View reviewed changes

Copy link

Member

Emyrk left a comment

Choose a reason for hiding this comment

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

On the right path

coderd/database/dbauthz/dbauthz.go


		// AsSystemRestricted returns a context with an actor that has permissions
		// required for various system operations e.g. login, logout.
		func AsSystemRestricted(ctx context.Context) context.Context {

Copy link

Member

EmyrkFeb 15, 2023

Choose a reason for hiding this comment

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

Do we plan on keeping this around? Or is it a catch all for the remaining stuff for now?

Copy link

MemberAuthor

johnstcnFeb 15, 2023

Choose a reason for hiding this comment

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

This is a catch-all for the remaining stuff. It's mostly used for HTTP middleware.
I've pared down all the perms except read, which can still be pared down to the bare minimum if need be.

If we need to break it down further in future, we can do so. I think this is fine for now though.