graph: Use userType property to distinguish between Member and Guest accounts
Fixes 5603 - Calling POST /graph/v1.0/users with userType not set will create a user as "Member" - Calling POST /graph/v1.0/users with userType set as "Member" or "Guest" will create a user as "Member" or "Guest" - Calling POST /graph/v1.0/users with userType set as anything but "Member" or "Guest" returns error - Calling POST /graph/v1.0/education/users with userType not set will create a user as "Member" - Calling POST /graph/v1.0/education/users with userType set as "Member" will create a user as "Member" and primary role as parameter specifies - Calling POST /graph/v1.0/education/users with userType set as "Guest" will create a user as "Guest" and primary role as parameter specifies - Calling POST /graph/v1.0/education/users with userType not set as anything but "Member" or "Guest" returns error - Calling PATCH on /users or /education/users will update attribute in the same way as for POST
This commit is contained in:
committed by
Ralf Haferkamp
parent
36d0c8c939
commit
23ba180e8a
@@ -132,6 +132,16 @@ func (g Graph) PostEducationUser(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if u.HasUserType() {
|
||||
if !isValidUserType(*u.UserType) {
|
||||
logger.Debug().Interface("user", u).Msg("invalid userType attribute")
|
||||
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'")
|
||||
return
|
||||
}
|
||||
} else {
|
||||
u.SetUserType("Member")
|
||||
}
|
||||
|
||||
if _, ok := u.GetPrimaryRoleOk(); !ok {
|
||||
logger.Debug().Err(err).Interface("user", u).Msg("could not create education user: missing required Attribute: 'primaryRole'")
|
||||
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing required Attribute: 'primaryRole'")
|
||||
@@ -368,6 +378,14 @@ func (g Graph) PatchEducationUser(w http.ResponseWriter, r *http.Request) {
|
||||
features = append(features, events.UserFeature{Name: "displayname", Value: *name})
|
||||
}
|
||||
|
||||
if changes.HasUserType() {
|
||||
if !isValidUserType(*changes.UserType) {
|
||||
logger.Debug().Interface("user", changes).Msg("invalid userType attribute")
|
||||
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
logger.Debug().Str("nameid", nameOrID).Interface("changes", *changes).Msg("calling update education user on backend")
|
||||
u, err := g.identityEducationBackend.UpdateEducationUser(r.Context(), nameOrID, *changes)
|
||||
if err != nil {
|
||||
|
||||
@@ -297,7 +297,65 @@ var _ = Describe("EducationUsers", func() {
|
||||
svc.PostEducationUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.EducationUser{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Member"))
|
||||
})
|
||||
|
||||
It("creates a guest user", func() {
|
||||
roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settingssvc.AssignRoleToUserResponse{}, nil)
|
||||
identityEducationBackend.On("CreateEducationUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.EducationUser) *libregraph.EducationUser {
|
||||
user.SetId("/users/user")
|
||||
return &user
|
||||
}, nil)
|
||||
|
||||
user.SetUserType("Guest")
|
||||
userJson, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/users", bytes.NewBuffer(userJson))
|
||||
r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
|
||||
svc.PostEducationUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.EducationUser{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Guest"))
|
||||
})
|
||||
|
||||
It("creates a member user", func() {
|
||||
roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settingssvc.AssignRoleToUserResponse{}, nil)
|
||||
identityEducationBackend.On("CreateEducationUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.EducationUser) *libregraph.EducationUser {
|
||||
user.SetId("/users/user")
|
||||
return &user
|
||||
}, nil)
|
||||
|
||||
user.SetUserType("Member")
|
||||
userJson, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/users", bytes.NewBuffer(userJson))
|
||||
r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
|
||||
svc.PostEducationUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.EducationUser{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Member"))
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
Describe("DeleteEducationUser", func() {
|
||||
@@ -406,6 +464,20 @@ var _ = Describe("EducationUsers", func() {
|
||||
Expect(rr.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("handles invalid userType", func() {
|
||||
user.SetUserType("Clown")
|
||||
data, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/users?$invalid=true", bytes.NewBuffer(data))
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("userID", user.GetId())
|
||||
r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
|
||||
svc.PatchEducationUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("updates attributes", func() {
|
||||
identityEducationBackend.On("UpdateEducationUser", mock.Anything, user.GetId(), mock.Anything).Return(user, nil)
|
||||
|
||||
|
||||
@@ -305,6 +305,16 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if u.HasUserType() {
|
||||
if !isValidUserType(*u.UserType) {
|
||||
logger.Debug().Interface("user", u).Msg("invalid userType attribute")
|
||||
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'")
|
||||
return
|
||||
}
|
||||
} else {
|
||||
u.SetUserType("Member")
|
||||
}
|
||||
|
||||
logger.Debug().Interface("user", u).Msg("calling create user on backend")
|
||||
if u, err = g.identityBackend.CreateUser(r.Context(), *u); err != nil {
|
||||
logger.Debug().Err(err).Msg("could not create user: backend error")
|
||||
@@ -652,6 +662,14 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
|
||||
features = append(features, events.UserFeature{Name: "displayname", Value: *name})
|
||||
}
|
||||
|
||||
if changes.HasUserType() {
|
||||
if !isValidUserType(*changes.UserType) {
|
||||
logger.Debug().Interface("user", changes).Msg("invalid userType attribute")
|
||||
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
logger.Debug().Str("nameid", nameOrID).Interface("changes", *changes).Msg("calling update user on backend")
|
||||
u, err := g.identityBackend.UpdateUser(r.Context(), nameOrID, *changes)
|
||||
if err != nil {
|
||||
@@ -743,3 +761,15 @@ func sortUsers(req *godata.GoDataRequest, users []*libregraph.User) ([]*libregra
|
||||
}
|
||||
return users, nil
|
||||
}
|
||||
|
||||
func isValidUserType(userType string) bool {
|
||||
userType = strings.ToLower(userType)
|
||||
|
||||
for _, value := range []string{"member", "guest"} {
|
||||
if userType == value {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -588,6 +588,11 @@ var _ = Describe("Users", func() {
|
||||
assertHandleBadAttributes(user)
|
||||
})
|
||||
|
||||
It("handles invalid userType", func() {
|
||||
user.SetUserType("Clown")
|
||||
assertHandleBadAttributes(user)
|
||||
})
|
||||
|
||||
It("creates a user", func() {
|
||||
roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{}, nil)
|
||||
identityBackend.On("CreateUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.User) *libregraph.User {
|
||||
@@ -602,6 +607,63 @@ var _ = Describe("Users", func() {
|
||||
svc.PostUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.User{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Member"))
|
||||
})
|
||||
|
||||
It("creates a guest user", func() {
|
||||
roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{}, nil)
|
||||
identityBackend.On("CreateUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.User) *libregraph.User {
|
||||
user.SetId("/users/user")
|
||||
return &user
|
||||
}, nil)
|
||||
|
||||
user.SetUserType("Guest")
|
||||
userJson, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users", bytes.NewBuffer(userJson))
|
||||
r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
|
||||
svc.PostUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.User{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Guest"))
|
||||
})
|
||||
|
||||
It("creates a member user", func() {
|
||||
roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{}, nil)
|
||||
identityBackend.On("CreateUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.User) *libregraph.User {
|
||||
user.SetId("/users/user")
|
||||
return &user
|
||||
}, nil)
|
||||
|
||||
user.SetUserType("Member")
|
||||
userJson, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users", bytes.NewBuffer(userJson))
|
||||
r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
|
||||
svc.PostUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
createdUser := libregraph.User{}
|
||||
err = json.Unmarshal(data, &createdUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(createdUser.GetUserType()).To(Equal("Member"))
|
||||
})
|
||||
|
||||
Describe("Handling usernames with spaces", func() {
|
||||
@@ -768,9 +830,25 @@ var _ = Describe("Users", func() {
|
||||
Expect(rr.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("handles invalid userType", func() {
|
||||
user.SetUserType("Clown")
|
||||
data, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users?$invalid=true", bytes.NewBuffer(data))
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("userID", user.GetId())
|
||||
r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))
|
||||
svc.PatchUser(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("updates attributes", func() {
|
||||
user.SetUserType("Member")
|
||||
identityBackend.On("UpdateUser", mock.Anything, user.GetId(), mock.Anything).Return(user, nil)
|
||||
|
||||
user.SetUserType(("Member"))
|
||||
user.SetDisplayName("New Display Name")
|
||||
data, err := json.Marshal(user)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@@ -788,6 +866,7 @@ var _ = Describe("Users", func() {
|
||||
updatedUser := libregraph.User{}
|
||||
err = json.Unmarshal(data, &updatedUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(updatedUser.GetUserType()).To(Equal("Member"))
|
||||
Expect(updatedUser.GetDisplayName()).To(Equal("New Display Name"))
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user