Skip to content

Commit

Permalink
Merge pull request #6839 from TheThingsNetwork/issue/6515-remove-stor…
Browse files Browse the repository at this point in the history 
…age-of-contact-info

Make `contact_info` a read-only field for entities
  • Loading branch information
@nicholaspcr
nicholaspcr authored Jan 22, 2024
2 parents 7168af5 + 9fd031b commit ac8fb94
Show file tree
Hide file tree
Showing 27 changed files with 503 additions and 454 deletions.
50 changes: 23 additions & 27 deletions pkg/identityserver/application_registry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.thethings.network/lorawan-stack/v3/pkg/events"
"go.thethings.network/lorawan-stack/v3/pkg/identityserver/blocklist"
"go.thethings.network/lorawan-stack/v3/pkg/identityserver/store"
"go.thethings.network/lorawan-stack/v3/pkg/rpcmiddleware/warning"
"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
"google.golang.org/protobuf/types/known/emptypb"
)
Expand Down Expand Up @@ -117,9 +118,6 @@ func (is *IdentityServer) createApplication( //nolint:gocyclo
); err != nil {
return nil, err
}
if err := validateContactInfo(req.Application.ContactInfo); err != nil {
return nil, err
}
err = is.store.Transact(ctx, func(ctx context.Context, st store.Store) (err error) {
app, err = st.CreateApplication(ctx, req.Application)
if err != nil {
Expand All @@ -133,13 +131,6 @@ func (is *IdentityServer) createApplication( //nolint:gocyclo
); err != nil {
return err
}
if len(req.Application.ContactInfo) > 0 {
cleanContactInfo(req.Application.ContactInfo)
app.ContactInfo, err = st.SetContactInfo(ctx, app.GetIds(), req.Application.ContactInfo)
if err != nil {
return err
}
}
return nil
})
if err != nil {
Expand All @@ -156,6 +147,11 @@ func (is *IdentityServer) getApplication(
if err = is.RequireAuthenticated(ctx); err != nil {
return nil, err
}
contactInfoInPath := ttnpb.HasAnyField(req.FieldMask.GetPaths(), "contact_info")
if contactInfoInPath {
req.FieldMask.Paths = ttnpb.ExcludeFields(req.FieldMask.Paths, "contact_info")
req.FieldMask.Paths = ttnpb.AddFields(req.FieldMask.Paths, "administrative_contact", "technical_contact")
}
req.FieldMask = cleanFieldMaskPaths(ttnpb.ApplicationFieldPathsNested, req.FieldMask, getPaths, nil)
if err = rights.RequireApplication(ctx, req.GetApplicationIds(), ttnpb.Right_RIGHT_APPLICATION_INFO); err != nil {
if !ttnpb.HasOnlyAllowedFields(req.FieldMask.GetPaths(), ttnpb.PublicApplicationFields...) {
Expand All @@ -168,8 +164,8 @@ func (is *IdentityServer) getApplication(
if err != nil {
return err
}
if ttnpb.HasAnyField(req.FieldMask.GetPaths(), "contact_info") {
app.ContactInfo, err = st.GetContactInfo(ctx, app.GetIds())
if contactInfoInPath {
app.ContactInfo, err = getContactsFromEntity(ctx, app, st)
if err != nil {
return err
}
Expand All @@ -186,6 +182,11 @@ func (is *IdentityServer) listApplications( // nolint:gocyclo
ctx context.Context,
req *ttnpb.ListApplicationsRequest,
) (apps *ttnpb.Applications, err error) {
contactInfoInPath := ttnpb.HasAnyField(req.FieldMask.GetPaths(), "contact_info")
if contactInfoInPath {
req.FieldMask.Paths = ttnpb.ExcludeFields(req.FieldMask.Paths, "contact_info")
req.FieldMask.Paths = ttnpb.AddFields(req.FieldMask.Paths, "administrative_contact", "technical_contact")
}
req.FieldMask = cleanFieldMaskPaths(ttnpb.ApplicationFieldPathsNested, req.FieldMask, getPaths, nil)

authInfo, err := is.authInfo(ctx)
Expand Down Expand Up @@ -271,6 +272,14 @@ func (is *IdentityServer) listApplications( // nolint:gocyclo
if err != nil {
return err
}
if contactInfoInPath {
for _, app := range apps.Applications {
app.ContactInfo, err = getContactsFromEntity(ctx, app, st)
if err != nil {
return err
}
}
}
return nil
})
if err != nil {
Expand Down Expand Up @@ -301,9 +310,8 @@ func (is *IdentityServer) updateApplication(
req.FieldMask = ttnpb.FieldMask(updatePaths...)
}
if ttnpb.HasAnyField(req.FieldMask.GetPaths(), "contact_info") {
if err := validateContactInfo(req.Application.ContactInfo); err != nil {
return nil, err
}
warning.Add(ctx, "Contact info is deprecated and will be removed in the next major release")
req.FieldMask.Paths = ttnpb.ExcludeFields(req.FieldMask.Paths, "contact_info")
}

if err := is.validateContactInfoRestrictions(
Expand Down Expand Up @@ -331,13 +339,6 @@ func (is *IdentityServer) updateApplication(
if err != nil {
return err
}
if ttnpb.HasAnyField(req.FieldMask.GetPaths(), "contact_info") {
cleanContactInfo(req.Application.ContactInfo)
app.ContactInfo, err = st.SetContactInfo(ctx, app.GetIds(), req.Application.ContactInfo)
if err != nil {
return err
}
}
return nil
})
if err != nil {
Expand Down Expand Up @@ -433,11 +434,6 @@ func (is *IdentityServer) purgeApplication(
if err != nil {
return err
}
// delete related contact info before purging the application
err = st.DeleteEntityContactInfo(ctx, ids)
if err != nil {
return err
}
return st.PurgeApplication(ctx, ids)
})
if err != nil {
Expand Down
26 changes: 26 additions & 0 deletions pkg/identityserver/application_registry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,32 @@ func TestApplicationsCRUD(t *testing.T) {
a.So(errors.IsPermissionDenied(err), should.BeTrue)
}

// TODO: Remove (https://github.com/TheThingsNetwork/lorawan-stack/issues/6804)
t.Run("Contact_info fieldmask", func(t *testing.T) { // nolint:paralleltest
a, ctx := test.New(t)
got, err := reg.Get(ctx, &ttnpb.GetApplicationRequest{
ApplicationIds: created.GetIds(),
FieldMask: ttnpb.FieldMask("contact_info"),
}, creds)
if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
a.So(got.ContactInfo, should.HaveLength, 2)
a.So(got.ContactInfo[0].Value, should.Equal, usr1.PrimaryEmailAddress)
a.So(got.ContactInfo[0].ContactType, should.Equal, ttnpb.ContactType_CONTACT_TYPE_OTHER)
a.So(got.ContactInfo[1].Value, should.Equal, usr1.PrimaryEmailAddress)
a.So(got.ContactInfo[1].ContactType, should.Equal, ttnpb.ContactType_CONTACT_TYPE_TECHNICAL)
}

// Testing the `PublicSafe` method, which should not return the contact_info's email address when the caller
// does not have the appropriate rights.
got, err = reg.Get(ctx, &ttnpb.GetApplicationRequest{
ApplicationIds: created.GetIds(),
FieldMask: ttnpb.FieldMask("contact_info"),
}, credsWithoutRights)
if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
a.So(got.ContactInfo, should.HaveLength, 0)
}
})

updated, err := reg.Update(ctx, &ttnpb.UpdateApplicationRequest{
Application: &ttnpb.Application{
Ids: created.GetIds(),
Expand Down
39 changes: 1 addition & 38 deletions pkg/identityserver/bunstore/application_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package store
import (
"context"
"fmt"
"sort"

"github.com/uptrace/bun"
"go.opentelemetry.io/otel/attribute"
Expand All @@ -44,8 +43,6 @@ type Application struct {

Attributes []*Attribute `bun:"rel:has-many,join:type=entity_type,join:id=entity_id,polymorphic"`

ContactInfo []*ContactInfo `bun:"rel:has-many,join:type=entity_type,join:id=entity_id,polymorphic"`

AdministrativeContactID *string `bun:"administrative_contact_id,type:uuid"`
AdministrativeContact *Account `bun:"rel:belongs-to,join:administrative_contact_id=id"`

Expand Down Expand Up @@ -94,14 +91,6 @@ func applicationToPB(m *Application, fieldMask ...string) (*ttnpb.Application, e
}
}

if len(m.ContactInfo) > 0 {
pb.ContactInfo = make([]*ttnpb.ContactInfo, len(m.ContactInfo))
for i, contactInfo := range m.ContactInfo {
pb.ContactInfo[i] = contactInfoToPB(contactInfo)
}
sort.Sort(contactInfoProtoSlice(pb.ContactInfo))
}

if m.AdministrativeContact != nil {
pb.AdministrativeContact = m.AdministrativeContact.GetOrganizationOrUserIdentifiers()
}
Expand Down Expand Up @@ -190,15 +179,6 @@ func (s *applicationStore) CreateApplication(
}
}

if len(pb.ContactInfo) > 0 {
applicationModel.ContactInfo, err = s.replaceContactInfo(
ctx, nil, pb.ContactInfo, "application", applicationModel.ID,
)
if err != nil {
return nil, err
}
}

pb, err = applicationToPB(applicationModel)
if err != nil {
return nil, err
Expand Down Expand Up @@ -231,8 +211,6 @@ func (*applicationStore) selectWithFields(q *bun.SelectQuery, fieldMask store.Fi
columns = append(columns, f)
case "attributes":
q = q.Relation("Attributes")
case "contact_info":
q = q.Relation("ContactInfo")
case "administrative_contact":
q = q.Relation("AdministrativeContact", func(q *bun.SelectQuery) *bun.SelectQuery {
return q.Column("uid", "account_type")
Expand Down Expand Up @@ -403,14 +381,6 @@ func (s *applicationStore) updateApplicationModel( //nolint:gocyclo
return err
}

case "contact_info":
model.ContactInfo, err = s.replaceContactInfo(
ctx, model.ContactInfo, pb.ContactInfo, "application", model.ID,
)
if err != nil {
return err
}

case "administrative_contact":
if contact := pb.AdministrativeContact; contact != nil {
account, err := s.getAccountModel(ctx, contact.EntityType(), contact.IDString())
Expand Down Expand Up @@ -570,7 +540,7 @@ func (s *applicationStore) PurgeApplication(ctx context.Context, id *ttnpb.Appli
model, err := s.getApplicationModelBy(
store.WithSoftDeleted(ctx, false),
s.selectWithID(ctx, id.GetApplicationId()),
store.FieldMask{"attributes", "contact_info"},
store.FieldMask{"attributes"},
)
if err != nil {
if errors.IsNotFound(err) {
Expand All @@ -588,13 +558,6 @@ func (s *applicationStore) PurgeApplication(ctx context.Context, id *ttnpb.Appli
}
}

if len(model.ContactInfo) > 0 {
_, err = s.replaceContactInfo(ctx, model.ContactInfo, nil, "application", model.ID)
if err != nil {
return err
}
}

_, err = s.DB.NewDelete().
Model(model).
WherePK().
Expand Down
39 changes: 1 addition & 38 deletions pkg/identityserver/bunstore/client_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package store
import (
"context"
"fmt"
"sort"

"github.com/uptrace/bun"
"go.opentelemetry.io/otel/attribute"
Expand All @@ -44,8 +43,6 @@ type Client struct {

Attributes []*Attribute `bun:"rel:has-many,join:type=entity_type,join:id=entity_id,polymorphic"`

ContactInfo []*ContactInfo `bun:"rel:has-many,join:type=entity_type,join:id=entity_id,polymorphic"`

AdministrativeContactID *string `bun:"administrative_contact_id,type:uuid"`
AdministrativeContact *Account `bun:"rel:belongs-to,join:administrative_contact_id=id"`

Expand Down Expand Up @@ -110,14 +107,6 @@ func clientToPB(m *Client, fieldMask ...string) (*ttnpb.Client, error) {
}
}

if len(m.ContactInfo) > 0 {
pb.ContactInfo = make([]*ttnpb.ContactInfo, len(m.ContactInfo))
for i, contactInfo := range m.ContactInfo {
pb.ContactInfo[i] = contactInfoToPB(contactInfo)
}
sort.Sort(contactInfoProtoSlice(pb.ContactInfo))
}

if m.AdministrativeContact != nil {
pb.AdministrativeContact = m.AdministrativeContact.GetOrganizationOrUserIdentifiers()
}
Expand Down Expand Up @@ -214,15 +203,6 @@ func (s *clientStore) CreateClient(
}
}

if len(pb.ContactInfo) > 0 {
clientModel.ContactInfo, err = s.replaceContactInfo(
ctx, nil, pb.ContactInfo, "client", clientModel.ID,
)
if err != nil {
return nil, err
}
}

pb, err = clientToPB(clientModel)
if err != nil {
return nil, err
Expand Down Expand Up @@ -259,8 +239,6 @@ func (*clientStore) selectWithFields(q *bun.SelectQuery, fieldMask store.FieldMa
columns = append(columns, "client_secret")
case "attributes":
q = q.Relation("Attributes")
case "contact_info":
q = q.Relation("ContactInfo")
case "administrative_contact":
q = q.Relation("AdministrativeContact", func(q *bun.SelectQuery) *bun.SelectQuery {
return q.Column("uid", "account_type")
Expand Down Expand Up @@ -431,14 +409,6 @@ func (s *clientStore) updateClientModel( //nolint:gocyclo
return err
}

case "contact_info":
model.ContactInfo, err = s.replaceContactInfo(
ctx, model.ContactInfo, pb.ContactInfo, "client", model.ID,
)
if err != nil {
return err
}

case "administrative_contact":
if contact := pb.AdministrativeContact; contact != nil {
account, err := s.getAccountModel(ctx, contact.EntityType(), contact.IDString())
Expand Down Expand Up @@ -615,7 +585,7 @@ func (s *clientStore) PurgeClient(ctx context.Context, id *ttnpb.ClientIdentifie
model, err := s.getClientModelBy(
store.WithSoftDeleted(ctx, false),
s.selectWithID(ctx, id.GetClientId()),
store.FieldMask{"attributes", "contact_info"},
store.FieldMask{"attributes"},
)
if err != nil {
if errors.IsNotFound(err) {
Expand All @@ -633,13 +603,6 @@ func (s *clientStore) PurgeClient(ctx context.Context, id *ttnpb.ClientIdentifie
}
}

if len(model.ContactInfo) > 0 {
_, err = s.replaceContactInfo(ctx, model.ContactInfo, nil, "client", model.ID)
if err != nil {
return err
}
}

_, err = s.DB.NewDelete().
Model(model).
WherePK().
Expand Down
2 changes: 0 additions & 2 deletions pkg/identityserver/bunstore/end_device_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,6 @@ func (*endDeviceStore) selectWithFields(q *bun.SelectQuery, fieldMask store.Fiel
columns = append(columns, "brand_id", "model_id", "hardware_version", "firmware_version", "band_id")
case "attributes":
q = q.Relation("Attributes")
case "contact_info":
q = q.Relation("ContactInfo")
case "locations":
q = q.Relation("Locations")
case "picture":
Expand Down
Loading

0 comments on commit ac8fb94

Please sign in to comment.