Access Control: Clear user's permission cache after resource creation #59101

IevaVasiljeva · 2022-11-22T11:53:58Z

What is this feature?

Automatically reload user's permission cache after they create a folder, dashboard, team or a data source.

Why do we need this feature?

This would allow the creator to immediately access the created resource, which is needed by provisioning (Terraform etc), frontend etc.

Who is this feature for?

All users.

Which issue(s) does this PR fix?:

Related to grafana/terraform-provider-grafana#521 and grafana/terraform-provider-grafana#665

Special notes for your reviewer:

Another approach would be to call a separate endpoint to reload the permission cache (this is what we currently do from the frontend by calling /api/access-control/user/actions?reloadcache=true). However, I feel like automatically reloading the cache from the API handlers is a simpler solution and covers all the use cases (ie, other potential provisioning tools, custom scripts etc).

grafanabot · 2022-11-22T12:05:17Z

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/42817

gamab

LGTM

grafanabot · 2022-11-22T14:54:58Z

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/42866

kalleep · 2022-11-22T15:55:35Z

pkg/api/dashboard.go

+		// Clear permission cache for the user who's created the dashboard, so that new permissions are fetched for their next call
+		// Required for cases when caller wants to immediately interact with the newly created object
+		if err := hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser); err != nil {
+			return response.Error(500, "Failed to clear permission cache after dashboard creation", err)


I dont think we should return a 500 here (some for the other endpoints). The only error we could get is that the signed in user is not something we can have in the cache and then there is nothing to clear anyway.

Hmm, makes sense. Do you think we should just log a warning and continue on? Or just continue on without a warning?

kalleep · 2022-11-22T15:57:37Z

pkg/services/accesscontrol/accesscontrol.go

@@ -26,6 +26,8 @@ type Service interface {
 	registry.ProvidesUsageStats
 	// GetUserPermissions returns user permissions with only action and scope fields set.
 	GetUserPermissions(ctx context.Context, user *user.SignedInUser, options Options) ([]Permission, error)
+	// ClearUserPermissionCache removes the permission cache entry for the given user
+	ClearUserPermissionCache(user *user.SignedInUser) error


Regarding my other comment, do we really need to return something here?

If the user can be cache we delete the associated key otherwise we just ignore it because it is not in the cache anyway

Yeah, that makes sense. I'll remove the returned error.

grafanabot · 2022-11-23T17:48:14Z

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/43166

linoman · 2022-11-24T14:22:29Z

pkg/api/dashboard.go

@@ -470,14 +470,20 @@ func (hs *HTTPServer) postDashboard(c *models.ReqContext, cmd models.SaveDashboa
 		}

 		if liveerr != nil {
-			hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", err)
+			hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveerr)


Feel free to ignore this comment.

I really don't like this variable name, since it reminds me of the human organ liver rather than and error from live.

Suggested change

hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveerr)

hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveErr)

Haha, that's pretty funny :) I'll leave it as it is for now, as I didn't introduce this error variable and don't want to add too many changes to this PR. But I'll keep it in mind for later.

grafanabot · 2022-11-24T14:39:25Z

Hello @IevaVasiljeva!
Backport pull requests need to be either:

Pull requests which address bugs,
Urgent fixes which need product approval, in order to get merged,
Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

grafanabot · 2022-11-24T14:41:05Z

The backport to v9.0.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.0.x origin/v9.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.0.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.0.x

Then, create a pull request where the base branch is v9.0.x and the compare/head branch is backport-59101-to-v9.0.x.

grafanabot · 2022-11-24T14:41:08Z

The backport to v9.1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.1.x origin/v9.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.1.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.1.x

Then, create a pull request where the base branch is v9.1.x and the compare/head branch is backport-59101-to-v9.1.x.

grafanabot · 2022-11-24T14:41:11Z

The backport to v9.2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.2.x origin/v9.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.2.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.2.x

Then, create a pull request where the base branch is v9.2.x and the compare/head branch is backport-59101-to-v9.2.x.

…#59101) * refresh user's permission cache after resource creation * clear the cache instead of reloading the permissions * don't error if can't clear cache * fix tests * fix tests again (cherry picked from commit a8bae3f)

grafanabot · 2022-11-24T14:42:21Z

The backport to v9.0.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.0.x origin/v9.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.0.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.0.x

Then, create a pull request where the base branch is v9.0.x and the compare/head branch is backport-59101-to-v9.0.x.

grafanabot · 2022-11-24T14:42:24Z

The backport to v9.1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.1.x origin/v9.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.1.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.1.x

Then, create a pull request where the base branch is v9.1.x and the compare/head branch is backport-59101-to-v9.1.x.

grafanabot · 2022-11-24T14:42:27Z

The backport to v9.2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.2.x origin/v9.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.2.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.2.x

Then, create a pull request where the base branch is v9.2.x and the compare/head branch is backport-59101-to-v9.2.x.

grafanabot · 2022-11-24T14:42:30Z

The backport to v9.3.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.3.x origin/v9.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.3.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.3.x

Then, create a pull request where the base branch is v9.3.x and the compare/head branch is backport-59101-to-v9.3.x.

grafanabot · 2022-11-24T14:43:37Z

The backport to v9.2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.2.x origin/v9.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.2.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.2.x

Then, create a pull request where the base branch is v9.2.x and the compare/head branch is backport-59101-to-v9.2.x.

grafanabot · 2022-11-24T14:43:40Z

The backport to v9.3.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-59101-to-v9.3.x origin/v9.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a8bae3f0b0cfc376a1d9698c364aa2e8abe7b71e
# Push it to GitHub
git push --set-upstream origin backport-59101-to-v9.3.x
git switch main
# Remove the local backport branch
git branch -D backport-59101-to-v9.3.x

Then, create a pull request where the base branch is v9.3.x and the compare/head branch is backport-59101-to-v9.3.x.

… creation (#59307) Access Control: Clear user's permission cache after resource creation (#59101) * refresh user's permission cache after resource creation * clear the cache instead of reloading the permissions * don't error if can't clear cache * fix tests * fix tests again (cherry picked from commit a8bae3f) Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

… creation (grafana#59307) Access Control: Clear user's permission cache after resource creation (grafana#59101) * refresh user's permission cache after resource creation * clear the cache instead of reloading the permissions * don't error if can't clear cache * fix tests * fix tests again (cherry picked from commit a8bae3f) Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

refresh user's permission cache after resource creation

99a3608

IevaVasiljeva requested review from gamab, Jguer and kalleep November 22, 2022 11:53

grafanabot added area/backend enterprise-failed labels Nov 22, 2022

gamab approved these changes Nov 22, 2022

View reviewed changes

IevaVasiljeva marked this pull request as ready for review November 22, 2022 12:15

IevaVasiljeva requested a review from a team as a code owner November 22, 2022 12:15

IevaVasiljeva requested review from mildwonkey, idafurjes and yangkb09 and removed request for a team November 22, 2022 12:15

clear the cache instead of reloading the permissions

eeb2799

IevaVasiljeva requested a review from a team as a code owner November 22, 2022 14:54

grafanabot removed the enterprise-failed label Nov 22, 2022

grafanabot added enterprise-failed enterprise-ok labels Nov 22, 2022

IevaVasiljeva added add to changelog backport v9.0.x backport v9.1.x Bot will automatically open backport PR backport v9.2.x Mark PR for automatic backport to v9.2.x backport v9.3.x labels Nov 22, 2022

IevaVasiljeva added this to the 9.3.0 milestone Nov 22, 2022

IevaVasiljeva changed the title ~~Access Control: Reload user's permission cache after resource creation~~ Access Control: Clear user's permission cache after resource creation Nov 22, 2022

kalleep reviewed Nov 22, 2022

View reviewed changes

don't error if can't clear cache

a736f45

grafanabot removed the enterprise-failed label Nov 22, 2022

gamab approved these changes Nov 24, 2022

View reviewed changes

linoman approved these changes Nov 24, 2022

View reviewed changes

IevaVasiljeva merged commit a8bae3f into main Nov 24, 2022

IevaVasiljeva deleted the reload-cache-after-resource-creation branch November 24, 2022 14:38

IevaVasiljeva added the type/bug label Nov 24, 2022

grafanabot added the backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. label Nov 24, 2022

grafanabot mentioned this pull request Nov 24, 2022

Access Control: Clear user's permission cache after resource creation #59307

Merged

IevaVasiljeva removed backport v9.0.x backport v9.1.x Bot will automatically open backport PR backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. labels Nov 24, 2022

grafanabot added the backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. label Nov 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Access Control: Clear user's permission cache after resource creation #59101

Access Control: Clear user's permission cache after resource creation #59101

IevaVasiljeva commented Nov 22, 2022

grafanabot commented Nov 22, 2022

gamab left a comment

grafanabot commented Nov 22, 2022

kalleep Nov 22, 2022

IevaVasiljeva Nov 22, 2022

kalleep Nov 22, 2022

IevaVasiljeva Nov 22, 2022

grafanabot commented Nov 23, 2022

linoman Nov 24, 2022

IevaVasiljeva Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

	hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveerr)
	hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveErr)

Access Control: Clear user's permission cache after resource creation #59101

Access Control: Clear user's permission cache after resource creation #59101

Conversation

IevaVasiljeva commented Nov 22, 2022

grafanabot commented Nov 22, 2022

gamab left a comment

Choose a reason for hiding this comment

grafanabot commented Nov 22, 2022

kalleep Nov 22, 2022

Choose a reason for hiding this comment

IevaVasiljeva Nov 22, 2022

Choose a reason for hiding this comment

kalleep Nov 22, 2022

Choose a reason for hiding this comment

IevaVasiljeva Nov 22, 2022

Choose a reason for hiding this comment

grafanabot commented Nov 23, 2022

linoman Nov 24, 2022

Choose a reason for hiding this comment

IevaVasiljeva Nov 24, 2022

Choose a reason for hiding this comment

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022

grafanabot commented Nov 24, 2022