diff --git a/server/contexts/viewer/viewer.go b/server/contexts/viewer/viewer.go index 88deedbc31..d510918dc2 100644 --- a/server/contexts/viewer/viewer.go +++ b/server/contexts/viewer/viewer.go @@ -30,15 +30,6 @@ type Viewer struct { Session *kolide.Session } -// IsAdmin indicates whether or not the current user can perform administrative -// actions. -func (v Viewer) IsAdmin() bool { - if v.User != nil { - return v.User.Admin && v.User.Enabled - } - return false -} - // UserID is a helper that enables quick access to the user ID of the current // user. func (v Viewer) UserID() uint { @@ -54,7 +45,7 @@ func (v Viewer) Username() string { if v.User != nil { return v.User.Username } - return "none" + return "" } // FullName is a helper that enables quick access to the full name of the @@ -63,7 +54,7 @@ func (v Viewer) FullName() string { if v.User != nil { return v.User.Name } - return "none" + return "" } // SessionID returns the current user's session ID @@ -74,6 +65,12 @@ func (v Viewer) SessionID() uint { return 0 } +// IsUserID returns true if the given user id the same as the user which is +// represented by this ViewerContext +func (v Viewer) IsUserID(id uint) bool { + return v.UserID() == id +} + // IsLoggedIn determines whether or not the current VC is attached to a user // account func (v Viewer) IsLoggedIn() bool { @@ -101,10 +98,13 @@ func (v Viewer) CanPerformActions() bool { return false } -// IsUserID returns true if the given user id the same as the user which is -// represented by this ViewerContext -func (v Viewer) IsUserID(id uint) bool { - return v.UserID() == id +// CanPerformAdminActions indicates whether or not the current user can perform +// administrative actions. +func (v Viewer) CanPerformAdminActions() bool { + if v.User != nil { + return v.CanPerformActions() && v.User.Admin + } + return false } // CanPerformReadActionOnUser returns a bool indicating the current user's @@ -120,10 +120,17 @@ func (v Viewer) CanPerformReadActionOnUser(uid uint) bool { // ability to perform write actions on the given user func (v Viewer) CanPerformWriteActionOnUser(uid uint) bool { if v.User != nil { - // By not requiring v.CanPerformActions() here, we allow the - // user to update their password when they are in the forced - // password reset state. - return v.IsUserID(uid) || v.IsAdmin() + return (v.IsLoggedIn() && v.IsUserID(uid)) || v.CanPerformAdminActions() + } + return false +} + +// CanPerformPasswordReset returns a bool indicating the current user's +// ability to perform a password reset (in the case they have been required by +// the admin). +func (v Viewer) CanPerformPasswordReset() bool { + if v.User != nil { + return v.IsLoggedIn() && v.User.AdminForcedPasswordReset } return false } diff --git a/server/contexts/viewer/viewer_test.go b/server/contexts/viewer/viewer_test.go new file mode 100644 index 0000000000..1ab87007e6 --- /dev/null +++ b/server/contexts/viewer/viewer_test.go @@ -0,0 +1,212 @@ +package viewer + +import ( + "context" + "testing" + + "github.com/kolide/fleet/server/kolide" + "github.com/stretchr/testify/assert" +) + +var ( + // Weird states + nilViewer = Viewer{} + noSessionViewer = Viewer{ + User: &kolide.User{ + ID: 41, + Name: "No Session", + Username: "nosession", + Enabled: true, + }, + } + + // Regular users + userViewer = Viewer{ + User: &kolide.User{ + ID: 45, + Name: "Regular User", + Username: "user", + Admin: false, + Enabled: true, + }, + Session: &kolide.Session{ + ID: 4, + UserID: 45, + }, + } + disabledUserViewer = Viewer{ + User: &kolide.User{ + ID: 46, + Name: "Disabled Regular User", + Username: "disabled_user", + Admin: false, + Enabled: false, + }, + Session: &kolide.Session{ + ID: 5, + UserID: 46, + }, + } + + needsPasswordResetUserViewer = Viewer{ + User: &kolide.User{ + ID: 47, + Name: "Regular User Needs Password Reset", + Username: "reset_user", + Admin: false, + Enabled: true, + AdminForcedPasswordReset: true, + }, + Session: &kolide.Session{ + ID: 6, + UserID: 47, + }, + } + + // Admin users + adminViewer = Viewer{ + User: &kolide.User{ + ID: 42, + Name: "The Admin", + Username: "admin", + Admin: true, + Enabled: true, + }, + Session: &kolide.Session{ + ID: 1, + UserID: 42, + }, + } + disabledAdminViewer = Viewer{ + User: &kolide.User{ + ID: 43, + Name: "The Disabled Admin", + Username: "disabled_admin", + Admin: true, + Enabled: false, + }, + Session: &kolide.Session{ + ID: 2, + UserID: 43, + }, + } + needsPasswordResetAdminViewer = Viewer{ + User: &kolide.User{ + ID: 44, + Name: "The Admin Requires Password Reset", + Username: "reset_admin", + Admin: true, + Enabled: true, + AdminForcedPasswordReset: true, + }, + Session: &kolide.Session{ + ID: 3, + UserID: 44, + }, + } +) + +func TestContext(t *testing.T) { + ctx := NewContext(context.Background(), userViewer) + v, ok := FromContext(ctx) + assert.True(t, ok) + assert.Equal(t, userViewer, v) +} + +func TestIsUserID(t *testing.T) { + assert.True(t, adminViewer.IsUserID(42)) + assert.False(t, adminViewer.IsUserID(7)) + assert.True(t, userViewer.IsUserID(45)) + assert.True(t, disabledUserViewer.IsUserID(46)) +} + +func TestIsLoggedIn(t *testing.T) { + assert.Equal(t, false, nilViewer.IsLoggedIn()) + assert.Equal(t, false, noSessionViewer.IsLoggedIn()) + + assert.Equal(t, true, userViewer.IsLoggedIn()) + assert.Equal(t, false, disabledUserViewer.IsLoggedIn()) + assert.Equal(t, true, needsPasswordResetUserViewer.IsLoggedIn()) + + assert.Equal(t, true, adminViewer.IsLoggedIn()) + assert.Equal(t, false, disabledAdminViewer.IsLoggedIn()) + assert.Equal(t, true, needsPasswordResetAdminViewer.IsLoggedIn()) +} + +func TestCanPerformActions(t *testing.T) { + assert.Equal(t, false, nilViewer.CanPerformActions()) + assert.Equal(t, false, noSessionViewer.CanPerformActions()) + + assert.Equal(t, true, userViewer.CanPerformActions()) + assert.Equal(t, false, disabledUserViewer.CanPerformActions()) + assert.Equal(t, false, needsPasswordResetUserViewer.CanPerformActions()) + + assert.Equal(t, true, adminViewer.CanPerformActions()) + assert.Equal(t, false, disabledAdminViewer.CanPerformActions()) + assert.Equal(t, false, needsPasswordResetAdminViewer.CanPerformActions()) +} + +func TestCanPerformAdminActions(t *testing.T) { + assert.Equal(t, false, nilViewer.CanPerformAdminActions()) + assert.Equal(t, false, noSessionViewer.CanPerformAdminActions()) + + assert.Equal(t, false, userViewer.CanPerformAdminActions()) + assert.Equal(t, false, disabledUserViewer.CanPerformAdminActions()) + assert.Equal(t, false, needsPasswordResetUserViewer.CanPerformAdminActions()) + + assert.Equal(t, true, adminViewer.CanPerformAdminActions()) + assert.Equal(t, false, disabledAdminViewer.CanPerformAdminActions()) + assert.Equal(t, false, needsPasswordResetAdminViewer.CanPerformAdminActions()) +} + +func TestCanPerformReadActionOnUser(t *testing.T) { + assert.Equal(t, false, nilViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, false, noSessionViewer.CanPerformReadActionOnUser(1)) + + assert.Equal(t, true, userViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, true, userViewer.CanPerformReadActionOnUser(userViewer.User.ID)) + assert.Equal(t, false, disabledUserViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, false, disabledUserViewer.CanPerformReadActionOnUser(disabledUserViewer.User.ID)) + assert.Equal(t, false, needsPasswordResetUserViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, true, needsPasswordResetUserViewer.CanPerformReadActionOnUser(needsPasswordResetUserViewer.User.ID)) + + assert.Equal(t, true, adminViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, true, adminViewer.CanPerformReadActionOnUser(adminViewer.User.ID)) + assert.Equal(t, false, disabledAdminViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, false, disabledAdminViewer.CanPerformReadActionOnUser(disabledAdminViewer.User.ID)) + assert.Equal(t, false, needsPasswordResetAdminViewer.CanPerformReadActionOnUser(1)) + assert.Equal(t, true, needsPasswordResetAdminViewer.CanPerformReadActionOnUser(needsPasswordResetAdminViewer.User.ID)) +} + +func TestCanPerformWriteActionOnUser(t *testing.T) { + assert.Equal(t, false, nilViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, false, noSessionViewer.CanPerformWriteActionOnUser(1)) + + assert.Equal(t, false, userViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, true, userViewer.CanPerformWriteActionOnUser(userViewer.User.ID)) + assert.Equal(t, false, disabledUserViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, false, disabledUserViewer.CanPerformWriteActionOnUser(disabledUserViewer.User.ID)) + assert.Equal(t, false, needsPasswordResetUserViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, true, needsPasswordResetUserViewer.CanPerformWriteActionOnUser(needsPasswordResetUserViewer.User.ID)) + + assert.Equal(t, true, adminViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, true, adminViewer.CanPerformWriteActionOnUser(adminViewer.User.ID)) + assert.Equal(t, false, disabledAdminViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, false, disabledAdminViewer.CanPerformWriteActionOnUser(disabledAdminViewer.User.ID)) + assert.Equal(t, false, needsPasswordResetAdminViewer.CanPerformWriteActionOnUser(1)) + assert.Equal(t, true, needsPasswordResetAdminViewer.CanPerformWriteActionOnUser(needsPasswordResetAdminViewer.User.ID)) +} + +func TestCanPerformPasswordReset(t *testing.T) { + assert.Equal(t, false, nilViewer.CanPerformPasswordReset()) + assert.Equal(t, false, noSessionViewer.CanPerformPasswordReset()) + + assert.Equal(t, false, userViewer.CanPerformPasswordReset()) + assert.Equal(t, false, disabledUserViewer.CanPerformPasswordReset()) + assert.Equal(t, true, needsPasswordResetUserViewer.CanPerformPasswordReset()) + + assert.Equal(t, false, adminViewer.CanPerformPasswordReset()) + assert.Equal(t, false, disabledAdminViewer.CanPerformPasswordReset()) + assert.Equal(t, true, needsPasswordResetAdminViewer.CanPerformPasswordReset()) + +} diff --git a/server/service/endpoint_appconfig.go b/server/service/endpoint_appconfig.go index 25edfd1015..d42f0098bf 100644 --- a/server/service/endpoint_appconfig.go +++ b/server/service/endpoint_appconfig.go @@ -36,7 +36,7 @@ func makeGetAppConfigEndpoint(svc kolide.Service) endpoint.Endpoint { var smtpSettings *kolide.SMTPSettingsPayload var ssoSettings *kolide.SSOSettingsPayload // only admin can see smtp settings - if vc.IsAdmin() { + if vc.CanPerformAdminActions() { smtpSettings = smtpSettingsFromAppConfig(config) if smtpSettings.SMTPPassword != nil { *smtpSettings.SMTPPassword = "********" diff --git a/server/service/endpoint_middleware.go b/server/service/endpoint_middleware.go index 01fb9d80b8..ad2df2c5c3 100644 --- a/server/service/endpoint_middleware.go +++ b/server/service/endpoint_middleware.go @@ -126,7 +126,7 @@ func mustBeAdmin(next endpoint.Endpoint) endpoint.Endpoint { if !ok { return nil, errNoContext } - if !vc.IsAdmin() { + if !vc.CanPerformAdminActions() { return nil, permissionError{message: "must be an admin"} } return next(ctx, request) @@ -174,6 +174,19 @@ func canModifyUser(next endpoint.Endpoint) endpoint.Endpoint { } } +func canPerformPasswordReset(next endpoint.Endpoint) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + vc, ok := viewer.FromContext(ctx) + if !ok { + return nil, errNoContext + } + if !vc.CanPerformPasswordReset() { + return nil, permissionError{message: "cannot reset password"} + } + return next(ctx, request) + } +} + type permission int const ( diff --git a/server/service/handler.go b/server/service/handler.go index b1317db217..f120a83bea 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -128,7 +128,7 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint RequirePasswordReset: authenticatedUser(jwtKey, svc, mustBeAdmin(makeRequirePasswordResetEndpoint(svc))), // PerformRequiredPasswordReset needs only to authenticate the // logged in user - PerformRequiredPasswordReset: authenticatedUser(jwtKey, svc, makePerformRequiredPasswordResetEndpoint(svc)), + PerformRequiredPasswordReset: authenticatedUser(jwtKey, svc, canPerformPasswordReset(makePerformRequiredPasswordResetEndpoint(svc))), GetSessionsForUserInfo: authenticatedUser(jwtKey, svc, canReadUser(makeGetInfoAboutSessionsForUserEndpoint(svc))), DeleteSessionsForUser: authenticatedUser(jwtKey, svc, canModifyUser(makeDeleteSessionsForUserEndpoint(svc))), GetSessionInfo: authenticatedUser(jwtKey, svc, mustBeAdmin(makeGetInfoAboutSessionEndpoint(svc))), diff --git a/server/service/handler_test.go b/server/service/handler_test.go index 4c141045f4..0ffbc7cf9d 100644 --- a/server/service/handler_test.go +++ b/server/service/handler_test.go @@ -223,7 +223,7 @@ func TestModifyUserPermissions(t *testing.T) { ) ms := new(mock.Store) ms.SessionByKeyFunc = func(key string) (*kolide.Session, error) { - return &kolide.Session{AccessedAt: time.Now(), UserID: uid}, nil + return &kolide.Session{AccessedAt: time.Now(), UserID: uid, ID: 1}, nil } ms.DestroySessionFunc = func(session *kolide.Session) error { return nil diff --git a/server/service/validation_users.go b/server/service/validation_users.go index 07efb4b23f..a9ed2f279e 100644 --- a/server/service/validation_users.go +++ b/server/service/validation_users.go @@ -108,7 +108,7 @@ func passwordRequiredForEmailChange(ctx context.Context, uid uint, invalid *inva return true } // if an admin is changing another users email no password needed - if vc.IsAdmin() { + if vc.CanPerformAdminActions() { return false } // should never get here because a non admin can't change the email of another