From 6fffe041209922313b6fa95fc47c1eeeec7b1941 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp <rhaferkamp@owncloud.com>
Date: Mon, 28 Mar 2022 11:23:23 +0200
Subject: [PATCH] graph: Fix handling of required attributes for users and
 groups

Fix a possible panic when checking for missing attribute ('err' was used
without actually being set). Return more helpful messages when required
attributes of a user or group are missing.
---
 graph/pkg/service/v0/groups.go |  4 ++--
 graph/pkg/service/v0/users.go  | 36 ++++++++++++++++++++--------------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/graph/pkg/service/v0/groups.go b/graph/pkg/service/v0/groups.go
index f261cfc10..76c74f212 100644
--- a/graph/pkg/service/v0/groups.go
+++ b/graph/pkg/service/v0/groups.go
@@ -42,7 +42,7 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if isNilOrEmpty(grp.DisplayName) {
+	if _, ok := grp.GetDisplayNameOk(); !ok {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Missing Required Attribute")
 		return
 	}
@@ -50,7 +50,7 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) {
 	// Disallow user-supplied IDs. It's supposed to be readonly. We're either
 	// generating them in the backend ourselves or rely on the Backend's
 	// storage (e.g. LDAP) to provide a unique ID.
-	if !isNilOrEmpty(grp.Id) {
+	if _, ok := grp.GetIdOk(); ok {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "group id is a read-only attribute")
 		return
 	}
diff --git a/graph/pkg/service/v0/users.go b/graph/pkg/service/v0/users.go
index ab7f3f9ec..636a8abff 100644
--- a/graph/pkg/service/v0/users.go
+++ b/graph/pkg/service/v0/users.go
@@ -60,26 +60,36 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if isNilOrEmpty(u.DisplayName) || isNilOrEmpty(u.OnPremisesSamAccountName) || isNilOrEmpty(u.Mail) {
-		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
+	if _, ok := u.GetDisplayNameOk(); !ok {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing required Attribute: 'displayName'")
+		return
+	}
+	if accountName, ok := u.GetOnPremisesSamAccountNameOk(); ok {
+		if !isValidUsername(*accountName) {
+			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest,
+				fmt.Sprintf("username '%s' must be at least the local part of an email", *u.OnPremisesSamAccountName))
+			return
+		}
+	} else {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing required Attribute: 'onPremisesSamAccountName'")
 		return
 	}
 
-	if !isValidUsername(*u.OnPremisesSamAccountName) {
-		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest,
-			fmt.Sprintf("username '%s' must be at least the local part of an email", *u.OnPremisesSamAccountName))
-		return
-	}
-	if !isValidEmail(*u.Mail) {
-		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest,
-			fmt.Sprintf("'%s' is not a valid email address", *u.Mail))
+	if mail, ok := u.GetMailOk(); ok {
+		if !isValidEmail(*mail) {
+			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest,
+				fmt.Sprintf("'%s' is not a valid email address", *u.Mail))
+			return
+		}
+	} else {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing required Attribute: 'mail'")
 		return
 	}
 
 	// Disallow user-supplied IDs. It's supposed to be readonly. We're either
 	// generating them in the backend ourselves or rely on the Backend's
 	// storage (e.g. LDAP) to provide a unique ID.
-	if !isNilOrEmpty(u.Id) {
+	if _, ok := u.GetIdOk(); ok {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "user id is a read-only attribute")
 		return
 	}
@@ -202,10 +212,6 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 
 }
 
-func isNilOrEmpty(s *string) bool {
-	return s == nil || *s == ""
-}
-
 // We want to allow email addresses as usernames so they show up when using them in ACLs on storages that allow integration with our glauth LDAP service
 // so we are adding a few restrictions from https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6
 // names should not start with numbers