Merge branch 'master' into bugfix/rls-pls

CHANGELOG.md

@@ -3,6 +3,14 @@
 ### UI Improvements
 ### Bug Fixes
 
+## v1.4.0.0-rc2 [unreleased]
+### UI Improvements
+1. [#2632](https://github.com/influxdata/chronograf/pull/2632): Tell user which organization they switched into and what role they have whenever they switch, including on Source Page
+
+### Bug Fixes
+1. [#2639](https://github.com/influxdata/chronograf/pull/2639): Prevent SuperAdmin from modifying their own status
+1. [#2632](https://github.com/influxdata/chronograf/pull/2632): Give SuperAdmin DefaultRole when switching to organization where they have no role
+
 ## v1.4.0.0-rc1 [2017-12-19]
 ### Features
 1. [#2593](https://github.com/influxdata/chronograf/pull/2593): Add option to use files for dashboards, organizations, data sources, and kapacitors

integrations/server_test.go

@@ -1241,6 +1241,59 @@ func TestServer(t *testing.T) {
 }`,
 			},
 		},
+		{
+			name:    "PATCH /users/1",
+			subName: "SuperAdmin modifying their own status",
+			fields: fields{
+				Users: []chronograf.User{
+					{
+						ID:         1, // This is artificial, but should be reflective of the users actual ID
+						Name:       "billibob",
+						Provider:   "github",
+						Scheme:     "oauth2",
+						SuperAdmin: true,
+						Roles: []chronograf.Role{
+							{
+								Name:         "admin",
+								Organization: "default",
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				server: &server.Server{
+					GithubClientID:     "not empty",
+					GithubClientSecret: "not empty",
+				},
+				method: "PATCH",
+				path:   "/chronograf/v1/users/1",
+				payload: map[string]interface{}{
+					"id":         "1",
+					"superAdmin": false,
+					"roles": []interface{}{
+						map[string]interface{}{
+							"name":         "admin",
+							"organization": "default",
+						},
+					},
+				},
+				principal: oauth2.Principal{
+					Organization: "default",
+					Subject:      "billibob",
+					Issuer:       "github",
+				},
+			},
+			wants: wants{
+				statusCode: http.StatusUnauthorized,
+				body: `
+{
+  "code": 401,
+  "message": "user cannot modify their own SuperAdmin status"
+}
+`,
+			},
+		},
 		{
 			name:    "PUT /me",
 			subName: "Change SuperAdmins current organization to org they dont belong to",
@@ -1301,7 +1354,7 @@ func TestServer(t *testing.T) {
       "organization": "default"
     },
     {
-      "name": "admin",
+      "name": "viewer",
       "organization": "1"
     }
   ],

server/me.go

@@ -11,7 +11,6 @@ import (
 	"github.com/influxdata/chronograf"
 	"github.com/influxdata/chronograf/oauth2"
 	"github.com/influxdata/chronograf/organizations"
-	"github.com/influxdata/chronograf/roles"
 )
 
 type meLinks struct {
@@ -96,7 +95,7 @@ func (s *Service) UpdateMe(auth oauth2.Authenticator) func(http.ResponseWriter,
 		}
 
 		// validate that the organization exists
-		_, err = s.Store.Organizations(serverCtx).Get(serverCtx, chronograf.OrganizationQuery{ID: &req.Organization})
+		org, err := s.Store.Organizations(serverCtx).Get(serverCtx, chronograf.OrganizationQuery{ID: &req.Organization})
 		if err != nil {
 			Error(w, http.StatusBadRequest, err.Error(), s.Logger)
 			return
@@ -151,8 +150,8 @@ func (s *Service) UpdateMe(auth oauth2.Authenticator) func(http.ResponseWriter,
 			// If the user is a super admin give them an admin role in the
 			// requested organization.
 			u.Roles = append(u.Roles, chronograf.Role{
-				Organization: req.Organization,
+				Organization: org.ID,
-				Name:         roles.AdminRoleName,
+				Name:         org.DefaultRole,
 			})
 			if err := s.Store.Users(serverCtx).Update(serverCtx, u); err != nil {
 				unknownErrorWithMessage(w, err, s.Logger)

server/users.go

@@ -273,6 +273,21 @@ func (s *Service) UpdateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Don't allow SuperAdmins to modify their own SuperAdmin status.
+	// Allowing them to do so could result in an application where there
+	// are no super admins.
+	ctxUser, ok := hasUserContext(ctx)
+	if !ok {
+		Error(w, http.StatusInternalServerError, "failed to retrieve user from context", s.Logger)
+		return
+	}
+	// If the user being updated is the user making the request and they are
+	// changing their SuperAdmin status, return an unauthorized error
+	if ctxUser.ID == u.ID && u.SuperAdmin == true && req.SuperAdmin == false {
+		Error(w, http.StatusUnauthorized, "user cannot modify their own SuperAdmin status", s.Logger)
+		return
+	}
+
 	if err := setSuperAdmin(ctx, req, u); err != nil {
 		Error(w, http.StatusUnauthorized, err.Error(), s.Logger)
 		return

server/users_test.go

@@ -667,6 +667,13 @@ func TestService_UpdateUser(t *testing.T) {
 					"http://any.url",
 					nil,
 				),
+				userKeyUser: &chronograf.User{
+					ID:         0,
+					Name:       "coolUser",
+					Provider:   "github",
+					Scheme:     "oauth2",
+					SuperAdmin: false,
+				},
 				user: &userRequest{
 					ID: 1336,
 					Roles: []chronograf.Role{
@@ -715,6 +722,13 @@ func TestService_UpdateUser(t *testing.T) {
 					"http://any.url",
 					nil,
 				),
+				userKeyUser: &chronograf.User{
+					ID:         0,
+					Name:       "coolUser",
+					Provider:   "github",
+					Scheme:     "oauth2",
+					SuperAdmin: false,
+				},
 				user: &userRequest{
 					ID: 1336,
 					Roles: []chronograf.Role{
@@ -786,6 +800,119 @@ func TestService_UpdateUser(t *testing.T) {
 			wantContentType: "application/json",
 			wantBody:        `{"code":422,"message":"duplicate organization \"1\" in roles"}`,
 		},
+		{
+			name: "SuperAdmin modifying their own SuperAdmin Status - user missing from context",
+			fields: fields{
+				Logger: log.New(log.DebugLevel),
+				UsersStore: &mocks.UsersStore{
+					UpdateF: func(ctx context.Context, user *chronograf.User) error {
+						return nil
+					},
+					GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
+						switch *q.ID {
+						case 1336:
+							return &chronograf.User{
+								ID:         1336,
+								Name:       "bobbetta",
+								Provider:   "github",
+								Scheme:     "oauth2",
+								SuperAdmin: true,
+								Roles: []chronograf.Role{
+									{
+										Name:         roles.EditorRoleName,
+										Organization: "1",
+									},
+								},
+							}, nil
+						default:
+							return nil, fmt.Errorf("User with ID %d not found", *q.ID)
+						}
+					},
+				},
+			},
+			args: args{
+				w: httptest.NewRecorder(),
+				r: httptest.NewRequest(
+					"PATCH",
+					"http://any.url",
+					nil,
+				),
+				user: &userRequest{
+					ID:         1336,
+					SuperAdmin: false,
+					Roles: []chronograf.Role{
+						{
+							Name:         roles.AdminRoleName,
+							Organization: "1",
+						},
+					},
+				},
+			},
+			id:              "1336",
+			wantStatus:      http.StatusInternalServerError,
+			wantContentType: "application/json",
+			wantBody:        `{"code":500,"message":"failed to retrieve user from context"}`,
+		},
+		{
+			name: "SuperAdmin modifying their own SuperAdmin Status",
+			fields: fields{
+				Logger: log.New(log.DebugLevel),
+				UsersStore: &mocks.UsersStore{
+					UpdateF: func(ctx context.Context, user *chronograf.User) error {
+						return nil
+					},
+					GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
+						switch *q.ID {
+						case 1336:
+							return &chronograf.User{
+								ID:         1336,
+								Name:       "bobbetta",
+								Provider:   "github",
+								Scheme:     "oauth2",
+								SuperAdmin: true,
+								Roles: []chronograf.Role{
+									{
+										Name:         roles.EditorRoleName,
+										Organization: "1",
+									},
+								},
+							}, nil
+						default:
+							return nil, fmt.Errorf("User with ID %d not found", *q.ID)
+						}
+					},
+				},
+			},
+			args: args{
+				w: httptest.NewRecorder(),
+				r: httptest.NewRequest(
+					"PATCH",
+					"http://any.url",
+					nil,
+				),
+				user: &userRequest{
+					ID:         1336,
+					SuperAdmin: false,
+					Roles: []chronograf.Role{
+						{
+							Name:         roles.AdminRoleName,
+							Organization: "1",
+						},
+					},
+				},
+				userKeyUser: &chronograf.User{
+					ID:         1336,
+					Name:       "coolUser",
+					Provider:   "github",
+					Scheme:     "oauth2",
+					SuperAdmin: true,
+				},
+			},
+			id:              "1336",
+			wantStatus:      http.StatusUnauthorized,
+			wantContentType: "application/json",
+			wantBody:        `{"code":401,"message":"user cannot modify their own SuperAdmin status"}`,
+		},
 		{
 			name: "Update a SuperAdmin's Roles - without super admin context",
 			fields: fields{

ui/src/admin/components/chronograf/OrganizationsTable.js

@@ -44,7 +44,6 @@ class OrganizationsTable extends Component {
       currentOrganization,
       authConfig: {superAdminNewUsers},
       onChangeAuthConfig,
-      me,
     } = this.props
     const {isCreatingOrganization} = this.state
 
@@ -93,7 +92,6 @@ class OrganizationsTable extends Component {
               onRename={onRenameOrg}
               onChooseDefaultRole={onChooseDefaultRole}
               currentOrganization={currentOrganization}
-              userHasRoleInOrg={!!me.organizations.find(o => org.id === o.id)}
             />
           )}
           <Authorized requiredRole={SUPERADMIN_ROLE}>
@@ -146,14 +144,5 @@ OrganizationsTable.propTypes = {
   authConfig: shape({
     superAdminNewUsers: bool,
   }),
-  me: shape({
-    organizations: arrayOf(
-      shape({
-        id: string.isRequired,
-        name: string.isRequired,
-        defaultRole: string.isRequired,
-      })
-    ),
-  }),
 }
 export default OrganizationsTable

ui/src/admin/components/chronograf/OrganizationsTableRow.js

@@ -39,19 +39,9 @@ class OrganizationsTableRow extends Component {
   }
 
   handleChangeCurrentOrganization = async () => {
-    const {
+    const {router, links, meChangeOrganization, organization} = this.props
-      router,
-      links,
-      meChangeOrganization,
-      organization,
-      userHasRoleInOrg,
-    } = this.props
 
-    await meChangeOrganization(
+    await meChangeOrganization(links.me, {organization: organization.id})
-      links.me,
-      {organization: organization.id},
-      {userHasRoleInOrg}
-    )
     router.push('')
   }
 
@@ -204,7 +194,7 @@ class OrganizationsTableRow extends Component {
   }
 }
 
-const {arrayOf, bool, func, shape, string} = PropTypes
+const {arrayOf, func, shape, string} = PropTypes
 
 OrganizationsTableRow.propTypes = {
   organization: shape({
@@ -235,7 +225,6 @@ OrganizationsTableRow.propTypes = {
     }),
   }),
   meChangeOrganization: func.isRequired,
-  userHasRoleInOrg: bool.isRequired,
 }
 
 OrganizationsTableRowDeleteButton.propTypes = {

ui/src/admin/components/chronograf/UsersTableRow.js

@@ -59,6 +59,7 @@ const UsersTableRow = ({
             active={user.superAdmin}
             onToggle={onChangeSuperAdmin(user)}
             size="xs"
+            disabled={userIsMe}
           />
         </td>
       </Authorized>

ui/src/shared/actions/auth.js

@@ -5,6 +5,8 @@ import {linksReceived} from 'shared/actions/links'
 import {publishAutoDismissingNotification} from 'shared/dispatchers'
 import {errorThrown} from 'shared/actions/errors'
 
+import {LONG_NOTIFICATION_DISMISS_DELAY} from 'shared/constants'
+
 export const authExpired = auth => ({
   type: 'AUTH_EXPIRED',
   payload: {
@@ -84,18 +86,20 @@ export const getMeAsync = ({shouldResetMe = false} = {}) => async dispatch => {
 
 export const meChangeOrganizationAsync = (
   url,
-  organization,
+  organization
-  {userHasRoleInOrg = true} = {}
 ) => async dispatch => {
   dispatch(meChangeOrganizationRequested())
   try {
     const {data: me, auth, logoutLink} = await updateMeAJAX(url, organization)
+    const currentRole = me.roles.find(
+      r => r.organization === me.currentOrganization.id
+    )
     dispatch(
       publishAutoDismissingNotification(
         'success',
-        `Now signed in to ${me.currentOrganization.name}${userHasRoleInOrg
+        `Now logged in to '${me.currentOrganization
-          ? ''
+          .name}' as '${currentRole.name}'`,
-          : ' with Admin role.'}`
+        LONG_NOTIFICATION_DISMISS_DELAY
       )
     )
     dispatch(meChangeOrganizationCompleted())

ui/src/shared/components/SlideToggle.js

@@ -14,7 +14,11 @@ class SlideToggle extends Component {
   }
 
   handleClick = () => {
-    const {onToggle} = this.props
+    const {onToggle, disabled} = this.props
+
+    if (disabled) {
+      return
+    }
 
     this.setState({active: !this.state.active}, () => {
       onToggle(this.state.active)
@@ -22,15 +26,15 @@ class SlideToggle extends Component {
   }
 
   render() {
-    const {size} = this.props
+    const {size, disabled} = this.props
     const {active} = this.state
 
-    const classNames = active
+    const className = `slide-toggle slide-toggle__${size} ${active
-      ? `slide-toggle slide-toggle__${size} active`
+      ? 'active'
-      : `slide-toggle slide-toggle__${size}`
+      : null} ${disabled ? 'disabled' : null}`
 
     return (
-      <div className={classNames} onClick={this.handleClick}>
+      <div className={className} onClick={this.handleClick}>
         <div className="slide-toggle--knob" />
       </div>
     )
@@ -46,6 +50,7 @@ SlideToggle.propTypes = {
   active: bool,
   size: string,
   onToggle: func.isRequired,
+  disabled: bool,
 }
 
 export default SlideToggle

ui/src/shared/constants/index.js

@@ -387,6 +387,7 @@ export const PRESENTATION_MODE_ANIMATION_DELAY = 0 // In milliseconds.
 export const PRESENTATION_MODE_NOTIFICATION_DELAY = 2000 // In milliseconds.
 
 export const SHORT_NOTIFICATION_DISMISS_DELAY = 2000 // in milliseconds
+export const LONG_NOTIFICATION_DISMISS_DELAY = 4000 // in milliseconds
 
 export const REVERT_STATE_DELAY = 1500 // ms
 

ui/src/sources/containers/SourcePage.js

@@ -10,6 +10,7 @@ import {
 import {publishNotification} from 'shared/actions/notifications'
 import {connect} from 'react-redux'
 
+import Notifications from 'shared/components/Notifications'
 import SourceForm from 'src/sources/components/SourceForm'
 import FancyScrollbar from 'shared/components/FancyScrollbar'
 import SourceIndicator from 'shared/components/SourceIndicator'
@@ -200,42 +201,45 @@ class SourcePage extends Component {
     }
 
     return (
-      <div className={`${isInitialSource ? '' : 'page'}`}>
+      <div>
-        {isInitialSource
+        <Notifications />
-          ? null
+        <div className={`${isInitialSource ? '' : 'page'}`}>
-          : <div className="page-header">
+          {isInitialSource
-              <div className="page-header__container page-header__source-page">
+            ? null
-                <div className="page-header__col-md-8">
+            : <div className="page-header">
-                  <div className="page-header__left">
+                <div className="page-header__container page-header__source-page">
-                    <h1 className="page-header__title">
+                  <div className="page-header__col-md-8">
-                      {editMode ? 'Edit Source' : 'Add a New Source'}
+                    <div className="page-header__left">
-                    </h1>
+                      <h1 className="page-header__title">
-                  </div>
+                        {editMode ? 'Edit Source' : 'Add a New Source'}
-                  <div className="page-header__right">
+                      </h1>
-                    <SourceIndicator />
+                    </div>
+                    <div className="page-header__right">
+                      <SourceIndicator />
+                    </div>
                   </div>
                 </div>
-              </div>
+              </div>}
-            </div>}
+          <FancyScrollbar className="page-contents">
-        <FancyScrollbar className="page-contents">
+            <div className="container-fluid">
-          <div className="container-fluid">
+              <div className="row">
-            <div className="row">
+                <div className="col-md-8 col-md-offset-2">
-              <div className="col-md-8 col-md-offset-2">
+                  <div className="panel panel-minimal">
-                <div className="panel panel-minimal">
+                    <SourceForm
-                  <SourceForm
+                      source={source}
-                    source={source}
+                      editMode={editMode}
-                    editMode={editMode}
+                      onInputChange={this.handleInputChange}
-                    onInputChange={this.handleInputChange}
+                      onSubmit={this.handleSubmit}
-                    onSubmit={this.handleSubmit}
+                      onBlurSourceURL={this.handleBlurSourceURL}
-                    onBlurSourceURL={this.handleBlurSourceURL}
+                      isInitialSource={isInitialSource}
-                    isInitialSource={isInitialSource}
+                      gotoPurgatory={this.gotoPurgatory}
-                    gotoPurgatory={this.gotoPurgatory}
+                    />
-                  />
+                  </div>
                 </div>
               </div>
             </div>
-          </div>
+          </FancyScrollbar>
-        </FancyScrollbar>
+        </div>
       </div>
     )
   }

ui/src/style/components/slide-toggle.scss

@@ -29,6 +29,7 @@
   }
 }
 
+/* Active State */
 .slide-toggle.active .slide-toggle--knob {
   background-color: $c-rainforest;
   transform: translate(100%,-50%);
@@ -37,6 +38,23 @@
   background-color: $c-honeydew;
 }
 
+/* Disabled State */
+.slide-toggle.disabled {
+  &,
+  &:hover,
+  &.active,
+  &.active:hover {
+    background-color: $g6-smoke;
+    cursor: not-allowed;
+  }
+  .slide-toggle--knob,
+  &:hover .slide-toggle--knob,
+  &.active .slide-toggle--knob,
+  &.active:hover .slide-toggle--knob {
+    background-color: $g3-castle;
+  }
+}
+
 /* Size Modifiers */
 
 .slide-toggle {