public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

IAM: Add the implementation of the Update User API (#112054)

Browse Source 
* wip

* Add validate, wire mutate, add tests

* Address copilot's feedback

* Address feedback
This commit is contained in:
Misi
2025-10-13 10:00:18 +02:00
committed by GitHub
parent 85174e3313
commit bd1dbb68ba
21 changed files with 820 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/iam/user/mutate.go
+5 -1
View File
@@ -5,9 +5,11 @@ import (
"strings"
iamv0alpha1 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"golang.org/x/text/cases"
"golang.org/x/text/language"
)
func MutateOnCreate(ctx context.Context, obj *iamv0alpha1.User) error {
func MutateOnCreateAndUpdate(ctx context.Context, obj *iamv0alpha1.User) error {
obj.Spec.Email = strings.ToLower(obj.Spec.Email)
obj.Spec.Login = strings.ToLower(obj.Spec.Login)
@@ -15,5 +17,7 @@ func MutateOnCreate(ctx context.Context, obj *iamv0alpha1.User) error {
obj.Spec.Login = obj.Spec.Email
}
obj.Spec.Role = cases.Title(language.Und).String(obj.Spec.Role)
return nil
}
pkg/registry/apis/iam/user/mutate_test.go
+63 -1
View File
@@ -68,10 +68,72 @@ func TestMutateOnCreate_LoginEmail(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := MutateOnCreate(context.Background(), tc.inputUser)
err := MutateOnCreateAndUpdate(context.Background(), tc.inputUser)
require.NoError(t, err)
require.Equal(t, tc.expectedLogin, tc.inputUser.Spec.Login)
require.Equal(t, tc.expectedEmail, tc.inputUser.Spec.Email)
})
}
}
func TestMutateOnCreate_Role(t *testing.T) {
testCases := []struct {
name string
inputUser *iamv0alpha1.User
expectedRole string
}{
{
name: "role is lowercase",
inputUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Role: "admin",
},
},
expectedRole: "Admin",
},
{
name: "role is uppercase",
inputUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Role: "ADMIN",
},
},
expectedRole: "Admin",
},
{
name: "role is mixed case",
inputUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Role: "aDmIn",
},
},
expectedRole: "Admin",
},
{
name: "role is already title case",
inputUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Role: "Admin",
},
},
expectedRole: "Admin",
},
{
name: "role is empty",
inputUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Role: "",
},
},
expectedRole: "",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := MutateOnCreateAndUpdate(context.Background(), tc.inputUser)
require.NoError(t, err)
require.Equal(t, tc.expectedRole, tc.inputUser.Spec.Role)
})
}
}
pkg/registry/apis/iam/user/store.go
+48 -1
View File
@@ -48,7 +48,54 @@ type LegacyStore struct {
// Update implements rest.Updater.
func (s *LegacyStore) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
return nil, false, apierrors.NewMethodNotSupported(resource.GroupResource(), "update")
if !s.enableAuthnMutation {
return nil, false, apierrors.NewMethodNotSupported(resource.GroupResource(), "update")
}
ns, err := request.NamespaceInfoFrom(ctx, true)
if err != nil {
return nil, false, err
}
oldObj, err := s.Get(ctx, name, nil)
if err != nil {
return nil, false, err
}
newObj, err := objInfo.UpdatedObject(ctx, oldObj)
if err != nil {
return nil, false, err
}
if updateValidation != nil {
if err := updateValidation(ctx, newObj, oldObj); err != nil {
return nil, false, err
}
}
userObj, ok := newObj.(*iamv0alpha1.User)
if !ok {
return nil, false, fmt.Errorf("expected User object, got %T", newObj)
}
updateCmd := legacy.UpdateUserCommand{
UID: name,
Login: userObj.Spec.Login,
Email: userObj.Spec.Email,
Name: userObj.Spec.Name,
IsAdmin: userObj.Spec.GrafanaAdmin,
IsDisabled: userObj.Spec.Disabled,
EmailVerified: userObj.Spec.EmailVerified,
Role: userObj.Spec.Role,
}
result, err := s.store.UpdateUser(ctx, ns, updateCmd)
if err != nil {
return nil, false, err
}
iamUser := toUserItem(&result.User, ns.Value)
return &iamUser, false, nil
}
// DeleteCollection implements rest.CollectionDeleter.
pkg/registry/apis/iam/user/validate.go
+71
View File
@@ -6,6 +6,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"github.com/grafana/authlib/types"
iamv0alpha1 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
)
@@ -27,5 +28,75 @@ func ValidateOnCreate(ctx context.Context, obj *iamv0alpha1.User) error {
return apierrors.NewBadRequest("user must have either login or email")
}
err = validateRole(obj)
if err != nil {
return err
}
return nil
}
func validateRole(obj *iamv0alpha1.User) error {
if obj.Spec.Role == "" {
return apierrors.NewBadRequest("role is required")
}
if !identity.RoleType(obj.Spec.Role).IsValid() {
return apierrors.NewBadRequest(fmt.Sprintf("invalid role '%s'", obj.Spec.Role))
}
return nil
}
func ValidateOnUpdate(ctx context.Context, oldObj, newObj *iamv0alpha1.User) error {
requester, err := identity.GetRequester(ctx)
if err != nil {
return apierrors.NewUnauthorized("no identity found")
}
isGrafanaAdmin := requester.GetIsGrafanaAdmin()
isServiceUser := requester.IsIdentityType(types.TypeAccessPolicy)
if !isGrafanaAdmin {
if newObj.Spec.Disabled != oldObj.Spec.Disabled {
return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(),
newObj.Name,
fmt.Errorf("only grafana admins can disable or enable a user"))
}
if newObj.Spec.GrafanaAdmin != oldObj.Spec.GrafanaAdmin {
return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(),
newObj.Name,
fmt.Errorf("only grafana admins can change grafana admin status"))
}
}
if !newObj.Spec.Provisioned && oldObj.Spec.Provisioned {
return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(),
newObj.Name,
fmt.Errorf("provisioned user cannot be un-provisioned"))
}
if !isServiceUser {
if newObj.Spec.Provisioned && !oldObj.Spec.Provisioned {
return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(),
newObj.Name,
fmt.Errorf("only service users can provision a user"))
}
if newObj.Spec.EmailVerified && !oldObj.Spec.EmailVerified {
return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(),
newObj.Name,
fmt.Errorf("only service users can verify email"))
}
}
if newObj.Spec.Login == "" && newObj.Spec.Email == "" {
return apierrors.NewBadRequest("user must have either login or email")
}
err = validateRole(newObj)
if err != nil {
return err
}
return nil
}
pkg/registry/apis/iam/user/validate_test.go
+310 -1
View File
@@ -24,6 +24,7 @@ func TestValidateOnCreate(t *testing.T) {
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Login: "testuser",
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
@@ -38,6 +39,7 @@ func TestValidateOnCreate(t *testing.T) {
Spec: iamv0alpha1.UserSpec{
Login: "newadmin",
GrafanaAdmin: true,
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
@@ -52,6 +54,7 @@ func TestValidateOnCreate(t *testing.T) {
Spec: iamv0alpha1.UserSpec{
Login: "newadmin",
GrafanaAdmin: true,
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
@@ -64,7 +67,9 @@ func TestValidateOnCreate(t *testing.T) {
{
name: "user with empty login and email",
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{},
Spec: iamv0alpha1.UserSpec{
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
@@ -78,6 +83,7 @@ func TestValidateOnCreate(t *testing.T) {
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Login: "testuser",
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
@@ -91,6 +97,7 @@ func TestValidateOnCreate(t *testing.T) {
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Email: "test@test.com",
Role: "Viewer",
},
},
requester: &identity.StaticRequester{
@@ -99,6 +106,49 @@ func TestValidateOnCreate(t *testing.T) {
},
expectError: false,
},
{
name: "user with empty role",
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Login: "testuser",
},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: false,
},
expectError: true,
errorContains: "role is required",
},
{
name: "user with invalid role",
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Login: "testuser",
Role: "InvalidRole",
},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: false,
},
expectError: true,
errorContains: "invalid role 'InvalidRole'",
},
{
name: "user with valid role",
user: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{
Login: "testuser",
Role: "Admin",
},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
}
for _, tt := range tests {
@@ -121,3 +171,262 @@ func TestValidateOnCreate(t *testing.T) {
})
}
}
func TestValidateOnUpdate(t *testing.T) {
tests := []struct {
name string
oldUser *iamv0alpha1.User
newUser *iamv0alpha1.User
requester *identity.StaticRequester
expectError bool
errorContains string
}{
{
name: "un-provisioning a provisioned user",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: true, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: false, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: true,
errorContains: "provisioned user cannot be un-provisioned",
},
{
name: "non-service user provisions a user",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: true,
errorContains: "only service users can provision a user",
},
{
name: "service user provisions a user",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Provisioned: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeAccessPolicy,
},
expectError: false,
},
{
name: "no changes",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
},
expectError: false,
},
{
name: "update with empty login and email",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "", Email: "", Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
},
expectError: true,
errorContains: "user must have either login or email",
},
{
name: "update with only login",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Email: "test@test.com", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "", Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
{
name: "update with only email",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "", Email: "test@test.com", Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
{
name: "service user verifies email",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", EmailVerified: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", EmailVerified: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeAccessPolicy,
},
expectError: false,
},
{
name: "non-service user verifies email",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", EmailVerified: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", EmailVerified: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
},
expectError: true,
errorContains: "only service users can verify email",
},
{
name: "grafana admin disables user",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Disabled: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Disabled: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
{
name: "non-admin disables user",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Disabled: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Disabled: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: false,
},
expectError: true,
errorContains: "only grafana admins can disable or enable a user",
},
{
name: "grafana admin grants admin",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", GrafanaAdmin: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", GrafanaAdmin: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
{
name: "non-admin grants admin",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", GrafanaAdmin: false, Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", GrafanaAdmin: true, Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: false,
},
expectError: true,
errorContains: "only grafana admins can change grafana admin status",
},
{
name: "update to empty role",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: ""},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: true,
errorContains: "role is required",
},
{
name: "update to invalid role",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "InvalidRole"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: true,
errorContains: "invalid role 'InvalidRole'",
},
{
name: "update to valid role",
oldUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Editor"},
},
newUser: &iamv0alpha1.User{
Spec: iamv0alpha1.UserSpec{Login: "testuser", Role: "Viewer"},
},
requester: &identity.StaticRequester{
Type: types.TypeUser,
IsGrafanaAdmin: true,
},
expectError: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := identity.WithRequester(
context.Background(),
tt.requester,
)
err := ValidateOnUpdate(ctx, tt.oldUser, tt.newUser)
if tt.expectError {
require.Error(t, err)
if tt.errorContains != "" {
require.Contains(t, err.Error(), tt.errorContains)
}
} else {
require.NoError(t, err)
}
})
}
}