RBAC: "Access denied" on read only dashboards after moving to Grafana 11

noamApps commented 2 months ago

What happened?

We are using Grafana 11.2 in a multi-org manner. authentication is done using JWT + grafana session cookie.

After upgrading to grafana 11 - we started to witness to odd behaviors that initially didn't seem related to us but might very well be connected.

-The main problem: users with Role Editor, cannot access dashboards that have view/edit permissions for editors unless they have explict permissions for their user email, getting either "Access denied to this dashboard" or "You'll need additional permissions to perform this action. Permissions needed: dashboards:read" - its seems random.

another phenomena we witnessed: a user browsing from multiple browsers gets its org switched all of a sudden

The first problem (the more serious one) doesn't occur for organizations that existed pre grafana 11 upgrade (seems to happen to new organizations only)

I tried to track down the issue, I wasn't able to fully identify it, but two things popped up :

The Get Active org flow that seems to change the active org stat in the backend side
There is A DualWriting mechanism that seems involved that seems might explain why the Access Denied issue happens in newer organizations but not in old ones.

There is a good chance the problem only happens for users that are in two organizations or more , Im working on reproducing it.

UPDATE

this happens only on organizations created after we upgraded to Grafana 11 (not sure if started in 11.0 or 11.2)

The access denied error is intermittent, a working api call to a dashboard and forbidden one can occur only a second apart

What did you expect to happen?

Role based access to dashboards should work.

Did this work before?

Yes

How do we reproduce it?

We still weren't able to fully reproduce, but In general:

Add a user to two different organizations
Add a dashboard the have permissions only for Roles (not to any specific user) in both orgs - maybe worth doing so using the server admin to keep the affected users clean.
try to access one of them - Access denied issue should pop up

Is the bug inside a dashboard panel?

No response

Environment (with versions)?

Grafana: 11.2 OS: Browser:

Grafana platform?

Kubernetes

Datasource(s)?

No response

krrish-sehgal commented 1 month ago

I would like to work on this , Please assign me this issue under hacktoberfest 2024. Thanks.

noamApps commented 1 month ago

Hi @krrish-sehgal , not sure how do I assign some one (looks

After looking into this a bit more (activating traces) I found two code pieces that looks suspicious

In this code the usr Object is being modified after being committed to the cache in s.userService.GetSignedInUser

in pkg/services/authn/authnimpl/sync/user_sync.go line 113

    usr, err := s.userService.GetSignedInUser(ctx, &user.GetSignedInUserQuery{
        UserID: userID,
        OrgID:  r.OrgID,
    })
    if err != nil {
        if errors.Is(err, user.ErrUserNotFound) {
            return errFetchingSignedInUserNotFound.Errorf("%w", err)
        }
        return errFetchingSignedInUser.Errorf("failed to resolve user: %w", err)
    }

    if id.ClientParams.AllowGlobalOrg && id.OrgID == authn.GlobalOrgID {
        usr.Teams = nil
        usr.OrgName = ""
        usr.OrgRole = org.RoleNone
        usr.OrgID = authn.GlobalOrgID
    }

       syncSignedInUserToIdentity(usr, id)

and in the same file , line 175

    goCtx := context.WithoutCancel(ctx)
    go func(userID int64) {
        defer func() {
            if err := recover(); err != nil {
                s.log.Error("Panic during user last seen sync", "err", err)
            }
        }()

        if err := s.userService.UpdateLastSeenAt(goCtx,
            &user.UpdateUserLastSeenAtCommand{UserID: userID, OrgID: r.OrgID}); err != nil &&
            !errors.Is(err, user.ErrLastSeenUpToDate) {
            s.log.Error("Failed to update last_seen_at", "err", err, "userId", userID)
        }
    }(userID)

    return nil

it appears that #88496 made a change to the context in which this routine run, it looks right however its the only goroutine in the user sync operations , and the bug I experience looks a lot like a race:

the 200,200,403,403... seems completely random
it got much better after I added resources to the db + grafana, however still happening
looks like it happens more in traffic hours

Also this are snippets that changed around 11.1/11.2 which is where I started experience the bug

grafana / grafana