Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #2552 from influxdata/fix/issue#2540 

Only promote user to super admin in user org store
pull/10616/head
Nathan Haugo 2017-12-14 11:43:33 -08:00 committed by GitHub
parent 0e1bcfc9c6 f86004cda3
commit f6f3d24a70
6 changed files with 169 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
organizations/users.go
View File

@ -142,6 +142,22 @@ func (s *UsersStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.U
// and the user that was found in the underlying store
usr.Roles = append(roles, u.Roles...)
// u.SuperAdmin == true is logically equivalent to u.SuperAdmin, however
// it is more clear on a conceptual level to check equality
//
// TODO(desa): this should go away with https://github.com/influxdata/chronograf/issues/2207
// I do not like checking super admin here. The organization users store should only be
// concerned about organizations.
//
// This allows users to be promoted to SuperAdmin on Add if they had not previously had the status.
// The reason for this is that we reuse Users and it is possible that one may try to "create" a user,
// who has super admin status, without super admin status on the request. In this case, a user should
// still retain their previous status and not be demoted. If we had done usr.SupderAdmin = u.SuperAdmin
// we could possibly demote a users super admin status.
if u.SuperAdmin == true {
usr.SuperAdmin = true
}
// Update the user in the underlying store
if err := s.store.Update(ctx, usr); err != nil {
return nil, err

56
organizations/users_test.go
View File

@ -313,11 +313,64 @@ func TestUsersStore_Add(t *testing.T) {
Name: "docbrown",
Provider: "github",
Scheme: "oauth2",
Roles: []chronograf.Role{
{
Organization: "1336",
Name: "admin",
},
},
},
},
{
name: "Add non-new user with Role. Stored user is not super admin. Provided user is super admin",
fields: fields{
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, u *chronograf.User) (*chronograf.User, error) {
return u, nil
},
UpdateF: func(ctx context.Context, u *chronograf.User) error {
return nil
},
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
ID: 1234,
Name: "docbrown",
Provider: "github",
Scheme: "oauth2",
SuperAdmin: false,
Roles: []chronograf.Role{
{
Organization: "1337",
Name: "editor",
},
},
}, nil
},
},
},
args: args{
ctx: context.Background(),
u: &chronograf.User{
ID: 1234,
Name: "docbrown",
Provider: "github",
Scheme: "oauth2",
SuperAdmin: true,
Roles: []chronograf.Role{
{
Organization: "1336",
Name: "admin",
},
},
},
orgID: "1336",
},
want: &chronograf.User{
Name: "docbrown",
Provider: "github",
Scheme: "oauth2",
SuperAdmin: true,
Roles: []chronograf.Role{
{
Organization: "1336",
Name: "admin",
@ -503,6 +556,9 @@ func TestUsersStore_Add(t *testing.T) {
if got == nil && tt.want == nil {
continue
}
if diff := cmp.Diff(got, tt.want, userCmpOptions...); diff != "" {
t.Errorf("%q. UsersStore.Add():\n-got/+want\ndiff %s", tt.name, diff)
}
}
}

11
server/users.go
View File

@ -157,6 +157,13 @@ func (s *Service) NewUser(w http.ResponseWriter, r *http.Request) {
}
ctx := r.Context()
cfg, err := s.Store.Config(ctx).Get(ctx)
if err != nil {
Error(w, http.StatusInternalServerError, err.Error(), s.Logger)
return
}
user := &chronograf.User{
Name: req.Name,
Provider: req.Provider,
@ -164,6 +171,10 @@ func (s *Service) NewUser(w http.ResponseWriter, r *http.Request) {
Roles: req.Roles,
}
if cfg.Auth.SuperAdminNewUsers {
req.SuperAdmin = true
}
if err := setSuperAdmin(ctx, req, user); err != nil {
Error(w, http.StatusUnauthorized, err.Error(), s.Logger)
return

79
server/users_test.go
View File

@ -113,6 +113,7 @@ func TestService_UserID(t *testing.T) {
func TestService_NewUser(t *testing.T) {
type fields struct {
UsersStore chronograf.UsersStore
ConfigStore chronograf.ConfigStore
Logger chronograf.Logger
}
type args struct {
@ -146,6 +147,13 @@ func TestService_NewUser(t *testing.T) {
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
return &chronograf.User{
@ -189,6 +197,13 @@ func TestService_NewUser(t *testing.T) {
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
return &chronograf.User{
@ -241,6 +256,13 @@ func TestService_NewUser(t *testing.T) {
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
return &chronograf.User{
@ -291,6 +313,13 @@ func TestService_NewUser(t *testing.T) {
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
return &chronograf.User{
@ -332,6 +361,13 @@ func TestService_NewUser(t *testing.T) {
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
return &chronograf.User{
@ -349,6 +385,48 @@ func TestService_NewUser(t *testing.T) {
wantContentType: "application/json",
wantBody: `{"id":"1338","superAdmin":true,"name":"bob","provider":"github","scheme":"oauth2","roles":[],"links":{"self":"/chronograf/v1/users/1338"}}`,
},
{
name: "Create a new User with SuperAdminNewUsers: true in ConfigStore",
args: args{
w: httptest.NewRecorder(),
r: httptest.NewRequest(
"POST",
"http://any.url",
nil,
),
user: &userRequest{
Name: "bob",
Provider: "github",
Scheme: "oauth2",
},
userKeyUser: &chronograf.User{
ID: 0,
Name: "coolUser",
Provider: "github",
Scheme: "oauth2",
SuperAdmin: true,
},
},
fields: fields{
Logger: log.New(log.DebugLevel),
ConfigStore: &mocks.ConfigStore{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: true,
},
},
},
UsersStore: &mocks.UsersStore{
AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) {
user.ID = 1338
return user, nil
},
},
},
wantStatus: http.StatusCreated,
wantContentType: "application/json",
wantBody: `{"id":"1338","superAdmin":true,"name":"bob","provider":"github","scheme":"oauth2","roles":[],"links":{"self":"/chronograf/v1/users/1338"}}`,
},
}
for _, tt := range tests {
@ -356,6 +434,7 @@ func TestService_NewUser(t *testing.T) {
s := &Service{
Store: &mocks.Store{
UsersStore: tt.fields.UsersStore,
ConfigStore: tt.fields.ConfigStore,
},
Logger: tt.fields.Logger,
}

14
ui/src/admin/components/chronograf/UsersTableRowNew.js
View File

@ -3,7 +3,6 @@ import React, {Component, PropTypes} from 'react'
import Authorized, {SUPERADMIN_ROLE} from 'src/auth/Authorized'
import Dropdown from 'shared/components/Dropdown'
import SlideToggle from 'shared/components/SlideToggle'
import {USERS_TABLE} from 'src/admin/constants/chronografTableSizing'
import {USER_ROLES} from 'src/admin/constants/chronografAdmin'
@ -17,7 +16,6 @@ class UsersTableRowNew extends Component {
provider: '',
scheme: 'oauth2',
role: this.props.organization.defaultRole,
superAdmin: false,
}
}
@ -54,10 +52,6 @@ class UsersTableRowNew extends Component {
this.setState({role: newRole.text})
}
handleSelectSuperAdmin = superAdmin => {
this.setState({superAdmin})
}
handleKeyDown = e => {
const {name, provider} = this.state
const preventCreate = !name || !provider
@ -86,7 +80,7 @@ class UsersTableRowNew extends Component {
colActions,
} = USERS_TABLE
const {onBlur} = this.props
const {name, provider, scheme, role, superAdmin} = this.state
const {name, provider, scheme, role} = this.state
const dropdownRolesItems = USER_ROLES.map(r => ({...r, text: r.name}))
const preventCreate = !name || !provider
@ -116,11 +110,7 @@ class UsersTableRowNew extends Component {
</td>
<Authorized requiredRole={SUPERADMIN_ROLE}>
<td style={{width: colSuperAdmin}} className="text-center">
<SlideToggle
active={superAdmin}
size="xs"
onToggle={this.handleSelectSuperAdmin}
/>
&mdash;
</td>
</Authorized>
<td style={{width: colProvider}}>

5
ui/src/style/pages/users.scss
View File

@ -104,11 +104,6 @@ table.table.chronograf-admin-table tbody tr.chronograf-admin-table--user td div.
/* Styles for new user row */
table.table.chronograf-admin-table tbody tr.chronograf-admin-table--new-user {
background-color: $g4-onyx;
> td {
padding-top: 8px;
padding-bottom: 8px;
}
}
/* Highlight "Me" in the users table */