Skip to content

Commit

Permalink
Only sync permissions after oauth login
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidgubler
davidgubler committed Sep 4, 2023
1 parent 607cd85 commit be62809
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 161 deletions.
7 changes: 4 additions & 3 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ The Grafana Organizations Operator only cares about organizations present in Key

### Issues with Grafana

* When a new user is created in Grafana (either by logging in via Keycloak or explicitly created via API), Grafana's `auto_assign_org` "feature" automatically gives the user permission to some organization (whichever is configured). This is almost never what we want. To work around this:
* When a user is created in Grafana, Grafana's `auto_assign_org` "feature" automatically gives the user permission to some organization (whichever is configured). This is almost never what we want. To work around this:
* It would be possible to disable `auto_assign_org`, but then Grafana would create a new organization for every new user, which would be even worse.
* We can't let Grafana create users on demand when they first log in (as that would give the user permission to see the `auto_assign_org`), thus we proactively create Grafana users for all known user, thereby avoiding any situations where Grafana would create a user on demand.
* When we create a Grafana user via API we immediately list its group memberships and remove all of them. This is a bit dangerous because it has the potential for race conditions or could leave unwanted permissions behind in case operator fails between the Grafana API calls, but it's the easiest way to handle this.
* We can't create the user and assign the correct permissions before the user logs in for the first time, because upon oauth login Grafana resets all permissions.
* Therefore we just fix permissions after the user has been created by Grafana. This leaves a time gap during which the user can have permissions he shouldn't have, but there isn't much we can do against that.
* A possible improvement would be to configure Grafana such that `auto_assign_org_id` points to a completely empty org, that way the invalid permissions wouldn't matter, but this isn't something this operator can configure.
* Because the `grafana-api-golang-client` implementation is incomplete we are wrapping it in the GrafanaClient type and add some functionality.
* The Grafana API often ignores the OrgID JSON field. The only workaround for this is to set the HTTP header `x-grafana-org-id`. The GrafanaClient wrapper takes care of this.

Expand Down
2 changes: 1 addition & 1 deletion gen-dev-env.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
set -e -o pipefail
SECRET="$(kubectl --as cluster-admin -n vshn-appuio-grafana get secret grafana-organizations-operator -ojson)" || (>&2 echo "You must be logged in to 'APPUiO Cloud LPG 2' with cluster-admin permissions for this to work" && exit 1)
echo -n "" > env
for VAL in KEYCLOAK_ADMIN_GROUP_PATH KEYCLOAK_CLIENT_ID KEYCLOAK_PASSWORD KEYCLOAK_REALM KEYCLOAK_URL KEYCLOAK_USERNAME; do
for VAL in KEYCLOAK_ADMIN_GROUP_PATH KEYCLOAK_AUTO_ASSIGN_ORG_GROUP_PATH KEYCLOAK_CLIENT_ID KEYCLOAK_PASSWORD KEYCLOAK_REALM KEYCLOAK_URL KEYCLOAK_USERNAME; do
echo -n "export ${VAL}=\"" >> env
echo "${SECRET}" | jq -r ".data.${VAL}" | base64 -d >> env
echo "\"" >> env
Expand Down
43 changes: 23 additions & 20 deletions main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ import (
)

var (
GrafanaUrl string
GrafanaUsername string
GrafanaPassword string
KeycloakUrl string
KeycloakRealm string
KeycloakUsername string
KeycloakPassword string
KeycloakClientId string
KeycloakAdminGroupPath string
GrafanaUrl string
GrafanaUsername string
GrafanaPassword string
KeycloakUrl string
KeycloakRealm string
KeycloakUsername string
KeycloakPassword string
KeycloakClientId string
KeycloakAdminGroupPath string
KeycloakAutoAssignOrgGroupPath string
)

func main() {
Expand Down Expand Up @@ -51,18 +52,20 @@ func main() {
KeycloakPasswordHidden = "***hidden***"
}
KeycloakAdminGroupPath = os.Getenv("KEYCLOAK_ADMIN_GROUP_PATH")
KeycloakAutoAssignOrgGroupPath = os.Getenv("KEYCLOAK_AUTO_ASSIGN_ORG_GROUP_PATH")

klog.Infof("GRAFANA_URL: %s\n", GrafanaUrl)
klog.Infof("GRAFANA_USERNAME: %s\n", GrafanaUsername)
klog.Infof("GRAFANA_PASSWORD: %s\n", GrafanaPasswordHidden)
klog.Infof("KEYCLOAK_URL: %s\n", KeycloakUrl)
klog.Infof("KEYCLOAK_REALM: %s\n", KeycloakRealm)
klog.Infof("KEYCLOAK_USERNAME: %s\n", KeycloakUsername)
klog.Infof("KEYCLOAK_PASSWORD: %s\n", KeycloakPasswordHidden)
klog.Infof("KEYCLOAK_CLIENT_ID: %s\n", KeycloakClientId)
klog.Infof("KEYCLOAK_ADMIN_GROUP_PATH: %s\n", KeycloakAdminGroupPath)
klog.Infof("GRAFANA_URL: %s\n", GrafanaUrl)
klog.Infof("GRAFANA_USERNAME: %s\n", GrafanaUsername)
klog.Infof("GRAFANA_PASSWORD: %s\n", GrafanaPasswordHidden)
klog.Infof("KEYCLOAK_URL: %s\n", KeycloakUrl)
klog.Infof("KEYCLOAK_REALM: %s\n", KeycloakRealm)
klog.Infof("KEYCLOAK_USERNAME: %s\n", KeycloakUsername)
klog.Infof("KEYCLOAK_PASSWORD: %s\n", KeycloakPasswordHidden)
klog.Infof("KEYCLOAK_CLIENT_ID: %s\n", KeycloakClientId)
klog.Infof("KEYCLOAK_ADMIN_GROUP_PATH: %s\n", KeycloakAdminGroupPath)
klog.Infof("KEYCLOAK_AUTO_ASSIGN_ORG_GROUP_PATH: %s\n", KeycloakAutoAssignOrgGroupPath)

keycloakClient, err := controller.NewKeycloakClient(KeycloakUrl, KeycloakRealm, KeycloakUsername, KeycloakPassword, KeycloakClientId, KeycloakAdminGroupPath)
keycloakClient, err := controller.NewKeycloakClient(KeycloakUrl, KeycloakRealm, KeycloakUsername, KeycloakPassword, KeycloakClientId, KeycloakAdminGroupPath, KeycloakAutoAssignOrgGroupPath)
if err != nil {
klog.Errorf("Could not create keycloakClient client: %v\n", err)
os.Exit(1)
Expand Down Expand Up @@ -108,7 +111,7 @@ func main() {
for {
err = controller.Reconcile(ctx, keycloakClient, grafanaClient, dashboard)
select {
case <-time.After(10 * time.Second):
case <-time.After(2 * time.Second):
case <-ctx.Done():
os.Exit(0)
}
Expand Down
85 changes: 62 additions & 23 deletions pkg/grafanaClient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package controller

import (
"encoding/json"
"errors"
"fmt"
grafana "github.com/grafana/grafana-api-golang-client"
"io"
"net/http"
"net/url"
"strconv"
)

type UserOrg struct {
Expand Down Expand Up @@ -49,12 +51,12 @@ func NewGrafanaClient(baseURL string, cfg grafana.Config) (*GrafanaClient, error
}, nil
}

func (this GrafanaClient) GetUsername() string {
func (this *GrafanaClient) GetUsername() string {
return this.config.BasicAuth.Username()
}

// This method is missing in the grafana-api-golang-client, that's the reason why we're wrapping that client at all
func (this GrafanaClient) GetUserOrgs(user grafana.User) ([]UserOrg, error) {
func (this *GrafanaClient) GetUserOrgs(user grafana.User) ([]UserOrg, error) {
url := this.baseURL
url.Path = fmt.Sprintf("/api/users/%d/orgs", user.ID)
req, err := http.NewRequest("GET", url.String(), nil)
Expand Down Expand Up @@ -82,93 +84,130 @@ func (this GrafanaClient) GetUserOrgs(user grafana.User) ([]UserOrg, error) {
return userOrgs, nil
}

func (this GrafanaClient) CloseIdleConnections() {
func (this *GrafanaClient) GetAutoAssignOrgId() (int64, error) {
url := this.baseURL
url.Path = "/api/admin/settings"
req, err := http.NewRequest("GET", url.String(), nil)
if err != nil {
return 0, err
}
password, _ := this.config.BasicAuth.Password()
req.SetBasicAuth(this.config.BasicAuth.Username(), password)
r, err := this.client.Do(req)
defer r.Body.Close()
if err != nil {
return 0, err
}

body, err := io.ReadAll(r.Body)
if err != nil {
return 0, err
}

settings := make(map[string]map[string]string)
err = json.Unmarshal(body, &settings)
if err != nil {
return 0, err
}

settingsUsers, ok := settings["users"]
if ok {
settingsAutoAssignOrgID, ok := settingsUsers["auto_assign_org_id"]
if ok {
return strconv.ParseInt(settingsAutoAssignOrgID, 10, 64)
}
}

return 0, errors.New("setting users.auto_assign_org_id not found")
}

func (this *GrafanaClient) CloseIdleConnections() {
this.client.CloseIdleConnections()
}

func (this GrafanaClient) OrgUsers(orgID int64) ([]grafana.OrgUser, error) {
func (this *GrafanaClient) OrgUsers(orgID int64) ([]grafana.OrgUser, error) {
return this.grafanaClient.OrgUsers(orgID)
}

func (this GrafanaClient) UpdateOrgUser(orgID, userID int64, role string) error {
func (this *GrafanaClient) UpdateOrgUser(orgID, userID int64, role string) error {
return this.grafanaClient.UpdateOrgUser(orgID, userID, role)
}

func (this GrafanaClient) AddOrgUser(orgID int64, user, role string) error {
func (this *GrafanaClient) AddOrgUser(orgID int64, user, role string) error {
return this.grafanaClient.AddOrgUser(orgID, user, role)
}

func (this GrafanaClient) RemoveOrgUser(orgID, userID int64) error {
func (this *GrafanaClient) RemoveOrgUser(orgID, userID int64) error {
return this.grafanaClient.RemoveOrgUser(orgID, userID)
}

func (this GrafanaClient) CreateUser(user grafana.User) (int64, error) {
func (this *GrafanaClient) CreateUser(user grafana.User) (int64, error) {
return this.grafanaClient.CreateUser(user)
}

func (this GrafanaClient) Users() (users []grafana.UserSearch, err error) {
func (this *GrafanaClient) Users() (users []grafana.UserSearch, err error) {
return this.grafanaClient.Users()
}

func (this GrafanaClient) UserUpdate(u grafana.User) error {
func (this *GrafanaClient) UserUpdate(u grafana.User) error {
return this.grafanaClient.UserUpdate(u)
}

func (this GrafanaClient) DeleteUser(id int64) error {
func (this *GrafanaClient) DeleteUser(id int64) error {
return this.grafanaClient.DeleteUser(id)
}

func (this GrafanaClient) Orgs() ([]grafana.Org, error) {
func (this *GrafanaClient) Orgs() ([]grafana.Org, error) {
return this.grafanaClient.Orgs()
}

func (this GrafanaClient) UpdateOrg(id int64, name string) error {
func (this *GrafanaClient) UpdateOrg(id int64, name string) error {
return this.grafanaClient.UpdateOrg(id, name)
}

func (this GrafanaClient) NewOrg(name string) (int64, error) {
func (this *GrafanaClient) NewOrg(name string) (int64, error) {
return this.grafanaClient.NewOrg(name)
}

func (this GrafanaClient) Org(id int64) (grafana.Org, error) {
func (this *GrafanaClient) Org(id int64) (grafana.Org, error) {
return this.grafanaClient.Org(id)
}

func (this GrafanaClient) DeleteOrg(id int64) error {
func (this *GrafanaClient) DeleteOrg(id int64) error {
return this.grafanaClient.DeleteOrg(id)
}

// We don't just wrap this method, we also work around the bad orgID handling of the original library and Grafana API
func (this GrafanaClient) DataSources(org *grafana.Org) ([]*grafana.DataSource, error) {
func (this *GrafanaClient) DataSources(org *grafana.Org) ([]*grafana.DataSource, error) {
return this.grafanaClient.WithOrgID(org.ID).DataSources()
}

// Ditto
func (this GrafanaClient) UpdateDataSource(org *grafana.Org, s *grafana.DataSource) error {
func (this *GrafanaClient) UpdateDataSource(org *grafana.Org, s *grafana.DataSource) error {
return this.grafanaClient.WithOrgID(org.ID).UpdateDataSource(s)
}

// Ditto
func (this GrafanaClient) DeleteDataSource(org *grafana.Org, id int64) error {
func (this *GrafanaClient) DeleteDataSource(org *grafana.Org, id int64) error {
return this.grafanaClient.WithOrgID(org.ID).DeleteDataSource(id)
}

// Ditto
func (this GrafanaClient) NewDataSource(org *grafana.Org, s *grafana.DataSource) (int64, error) {
func (this *GrafanaClient) NewDataSource(org *grafana.Org, s *grafana.DataSource) (int64, error) {
return this.grafanaClient.WithOrgID(org.ID).NewDataSource(s)
}

// Ditto
func (this GrafanaClient) DataSource(org *grafana.Org, id int64) (*grafana.DataSource, error) {
func (this *GrafanaClient) DataSource(org *grafana.Org, id int64) (*grafana.DataSource, error) {
return this.grafanaClient.WithOrgID(org.ID).DataSource(id)
}

// Ditto
func (this GrafanaClient) Dashboards(org *grafana.Org) ([]grafana.FolderDashboardSearchResponse, error) {
func (this *GrafanaClient) Dashboards(org *grafana.Org) ([]grafana.FolderDashboardSearchResponse, error) {
return this.grafanaClient.WithOrgID(org.ID).Dashboards()
}

// Ditto
func (this GrafanaClient) NewDashboard(org *grafana.Org, dashboard grafana.Dashboard) (*grafana.DashboardSaveResponse, error) {
func (this *GrafanaClient) NewDashboard(org *grafana.Org, dashboard grafana.Dashboard) (*grafana.DashboardSaveResponse, error) {
return this.grafanaClient.WithOrgID(org.ID).NewDashboard(dashboard)
}
36 changes: 19 additions & 17 deletions pkg/keycloakClient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ import (
)

type KeycloakClient struct {
baseURL url.URL
username string
password string
clientId string
realm string
adminGroupPath string
country string
adminGroup *KeycloakGroup
client *http.Client
baseURL url.URL
username string
password string
clientId string
realm string
adminGroupPath string
autoAssignOrgGroupPath string
country string
adminGroup *KeycloakGroup
client *http.Client
}

type KeycloakUser struct {
Expand Down Expand Up @@ -90,7 +91,7 @@ func (this *KeycloakUser) GetDisplayName() string {
return this.FirstName + " " + this.LastName
}

func NewKeycloakClient(baseURL string, realm string, username string, password string, clientId string, adminGroupPath string) (*KeycloakClient, error) {
func NewKeycloakClient(baseURL string, realm string, username string, password string, clientId string, adminGroupPath string, autoAssignOrgGroupPath string) (*KeycloakClient, error) {
u, err := url.Parse(baseURL)
if err != nil {
return nil, err
Expand All @@ -103,13 +104,14 @@ func NewKeycloakClient(baseURL string, realm string, username string, password s
}

return &KeycloakClient{
baseURL: *u,
client: cli,
realm: realm,
username: username,
password: password,
clientId: clientId,
adminGroupPath: adminGroupPath,
baseURL: *u,
client: cli,
realm: realm,
username: username,
password: password,
clientId: clientId,
adminGroupPath: adminGroupPath,
autoAssignOrgGroupPath: autoAssignOrgGroupPath,
}, nil
}

Expand Down
Loading

0 comments on commit be62809

Please sign in to comment.