graph(education): Make 'schoolNumber' attribute optional

It's already optional in the spec. For mulit-tenant provisioning
we want it to be optional as well.

Related: #1597
This commit is contained in:
Ralf Haferkamp
2025-10-14 16:52:23 +02:00
committed by Ralf Haferkamp
parent 920a6916c4
commit 28ec9c3282
3 changed files with 31 additions and 41 deletions

View File

@@ -8,10 +8,10 @@ import (
"github.com/go-ldap/ldap/v3"
"github.com/gofrs/uuid"
libregraph "github.com/opencloud-eu/libre-graph-api-go"
"github.com/opencloud-eu/opencloud/pkg/log"
"github.com/opencloud-eu/opencloud/services/graph/pkg/config"
"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
libregraph "github.com/opencloud-eu/libre-graph-api-go"
)
type educationConfig struct {
@@ -119,16 +119,18 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ
}
// Check that the school number is not already used
_, err := i.getSchoolByNumber(school.GetSchoolNumber())
switch err {
case nil:
logger.Debug().Err(errSchoolNumberExists).Str("schoolNumber", school.GetSchoolNumber()).Msg("duplicate school number")
return nil, errSchoolNumberExists
case ErrNotFound:
break
default:
logger.Error().Err(err).Str("schoolNumber", school.GetSchoolNumber()).Msg("error looking up school by number")
return nil, errorcode.New(errorcode.GeneralException, "error looking up school by number")
if school.HasSchoolNumber() {
_, err := i.getSchoolByNumber(school.GetSchoolNumber())
switch err {
case nil:
logger.Debug().Err(errSchoolNumberExists).Str("schoolNumber", school.GetSchoolNumber()).Msg("duplicate school number")
return nil, errSchoolNumberExists
case ErrNotFound:
break
default:
logger.Error().Err(err).Str("schoolNumber", school.GetSchoolNumber()).Msg("error looking up school by number")
return nil, errorcode.New(errorcode.GeneralException, "error looking up school by number")
}
}
attributeTypeAndValue := ldap.AttributeTypeAndValue{
@@ -142,7 +144,9 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ
)
ar := ldap.NewAddRequest(dn, nil)
ar.Attribute(i.educationConfig.schoolAttributeMap.displayName, []string{school.GetDisplayName()})
ar.Attribute(i.educationConfig.schoolAttributeMap.schoolNumber, []string{school.GetSchoolNumber()})
if school.HasSchoolNumber() {
ar.Attribute(i.educationConfig.schoolAttributeMap.schoolNumber, []string{school.GetSchoolNumber()})
}
if !i.useServerUUID {
ar.Attribute(i.educationConfig.schoolAttributeMap.id, []string{uuid.Must(uuid.NewV4()).String()})
}
@@ -723,18 +727,22 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch
if err != nil && !errors.Is(err, errNotSet) {
i.logger.Error().Err(err).Str("dn", e.DN).Msg("Error reading termination date for LDAP entry")
}
if id != "" && displayName != "" && schoolNumber != "" {
school := libregraph.NewEducationSchool()
school.SetDisplayName(displayName)
school.SetSchoolNumber(schoolNumber)
school.SetId(id)
if t != nil {
school.SetTerminationDate(*t)
}
return school
if id == "" || displayName == "" {
i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("displayName", displayName).Msg("Invalid School. Missing required attribute")
return nil
}
i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("displayName", displayName).Str("schoolNumber", schoolNumber).Msg("Invalid School. Missing required attribute")
return nil
school := libregraph.NewEducationSchool()
school.SetDisplayName(displayName)
school.SetId(id)
if schoolNumber != "" {
school.SetSchoolNumber(schoolNumber)
}
if t != nil {
school.SetTerminationDate(*t)
}
return school
}
func (i *LDAP) getSchoolNumber(e *ldap.Entry) string {

View File

@@ -74,12 +74,6 @@ func (g Graph) PostEducationSchool(w http.ResponseWriter, r *http.Request) {
return
}
if _, ok := school.GetSchoolNumberOk(); !ok {
logger.Debug().Interface("school", school).Msg("could not create school: missing required attribute")
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Missing Required Attribute")
return
}
// validate terminationDate attribute, needs to be "far enough" in the future, terminationDate can be nil (means
// termination date is to be deleted
if terminationDate, ok := school.GetTerminationDateOk(); ok && terminationDate != nil {

View File

@@ -232,18 +232,6 @@ var _ = Describe("Schools", func() {
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})
It("handles missing school number", func() {
newSchool = libregraph.NewEducationSchool()
newSchool.SetDisplayName("New School")
newSchoolJson, err := json.Marshal(newSchool)
Expect(err).ToNot(HaveOccurred())
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/schools/", bytes.NewBuffer(newSchoolJson))
svc.PostEducationSchool(rr, r)
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})
It("disallows school create ids", func() {
newSchool = libregraph.NewEducationSchool()
newSchool.SetId("disallowed")