diff --git a/go.sum b/go.sum index 5d817b0d..8eb06155 100644 --- a/go.sum +++ b/go.sum @@ -54,8 +54,6 @@ github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dv github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A= github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= -github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= -github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE= github.com/magiconair/properties v1.8.10/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= @@ -67,17 +65,12 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-sqlite3 v1.10.0 h1:jbhqpg7tQe4SupckyijYiy0mJJ/pRyHvXf7JdWK860o= github.com/mattn/go-sqlite3 v1.10.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= -github.com/mattn/go-sqlite3 v1.14.32 h1:JD12Ag3oLy1zQA+BNn74xRgaBbdhbNIDYvQUEuuErjs= -github.com/mattn/go-sqlite3 v1.14.32/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI= github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5/go.mod h1:iIss55rKnNBTvrwdmkUpLnDpZoAHvWaiq5+iMmen4AE= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0= -github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= -github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= github.com/rs/zerolog v1.34.0 h1:k43nTLIwcTVQAncfCw4KZ2VY6ukYoZaBPNOE8txlOeY= github.com/rs/zerolog v1.34.0/go.mod h1:bJsvje4Z08ROH4Nhs5iH600c3IkWhwp44iRc54W6wYQ= github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8= @@ -90,8 +83,6 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/vektah/gqlparser/v2 v2.5.31 h1:YhWGA1mfTjID7qJhd1+Vxhpk5HTgydrGU9IgkWBTJ7k= github.com/vektah/gqlparser/v2 v2.5.31/go.mod h1:c1I28gSOVNzlfc4WuDlqU7voQnsqI6OG2amkBAFmgts= -golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A= -golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= diff --git a/schema.graphql b/schema.graphql index 6665d53f..4efef714 100644 --- a/schema.graphql +++ b/schema.graphql @@ -21,7 +21,7 @@ type RootMutation { login(username: String!, pass: String!, deviceName: String!, type: DeviceType!, cookie: Boolean!): Login createDevice(name: String!, type: DeviceType!): Login @hasRole(role: USER) - updateDevice(id: Int!, name: String!, type: DeviceType!): Device + updateDevice(id: Int!, name: String!, type: DeviceType!): Device @hasRole(role: USER) removeDevice(id: Int!): Device @hasRole(role: USER) removeCurrentDevice: Device! @hasRole(role: USER) @@ -36,9 +36,9 @@ type RootMutation { updateDashboard(id: Int!, name: String!): Dashboard! @hasRole(role: USER) removeDashboard(id: Int!): Dashboard! @hasRole(role: USER) - addDashboardRange(dashboardId: Int!, range: InputNamedDateRange!): NamedDateRange - updateDashboardRange(rangeId: Int!, range: InputNamedDateRange!): NamedDateRange - removeDashboardRange(rangeId: Int!): NamedDateRange + addDashboardRange(dashboardId: Int!, range: InputNamedDateRange!): NamedDateRange @hasRole(role: USER) + updateDashboardRange(rangeId: Int!, range: InputNamedDateRange!): NamedDateRange @hasRole(role: USER) + removeDashboardRange(rangeId: Int!): NamedDateRange @hasRole(role: USER) addDashboardEntry(dashboardId: Int!, entryType: EntryType!, title: String!, total: Boolean!, stats: InputStatsSelection!, pos: InputResponsiveDashboardEntryPos): DashboardEntry @hasRole(role: USER) updateDashboardEntry(entryId: Int!, entryType: EntryType, title: String, total: Boolean, stats: InputStatsSelection, pos: InputResponsiveDashboardEntryPos): DashboardEntry @hasRole(role: USER) diff --git a/tag/remove.go b/tag/remove.go index 56e9029d..5546d58c 100644 --- a/tag/remove.go +++ b/tag/remove.go @@ -19,10 +19,10 @@ func (r *ResolverForTag) RemoveTag(ctx context.Context, key string) (*gqlmodel.T } usedInEntries := []model.DashboardEntry{} - // Do not read the next statements, not proud of it. - if err := r.DB.Where("keys LIKE ?", "%"+key). - Or("keys like ?", "%"+key+"%"). - Or("keys like ?", key+"%"). + if err := r.DB. + Joins("INNER JOIN dashboards ON dashboard_entries.dashboard_id = dashboards.id"). + Where("dashboards.user_id = ?", userID). + Where("keys LIKE ? OR keys LIKE ? OR keys LIKE ?", "%"+key, "%"+key+"%", key+"%"). Find(&usedInEntries).Error; err != nil { return nil, err } diff --git a/tag/remove_test.go b/tag/remove_test.go index c942b757..64eb00ee 100644 --- a/tag/remove_test.go +++ b/tag/remove_test.go @@ -81,3 +81,26 @@ func TestGQL_RemoveTag_fails_notPermission(t *testing.T) { _, err := resolver.RemoveTag(fake.User(5), "existing") require.EqualError(t, err, "tag with key 'existing' does not exist") } + +func TestRemove_crossUserIsolation(t *testing.T) { + db := test.InMemoryDB(t) + defer db.Close() + userA := db.User(5) + userB := db.User(2) + + userA.NewTagDefinition("project") + userB.NewTagDefinition("project") + + dashboardB := userB.Dashboard("secretDashboard") + dashboardB.Entry("secretEntry") + entryB := dashboardB.Dashboard.Entries[0] + entryB.Keys = "project,secret" + db.Save(&entryB) + + resolver := ResolverForTag{DB: db.DB} + _, err := resolver.RemoveTag(fake.User(userA.User.ID), "project") + require.NoError(t, err, "removing user's own tag should succeed even if another user has a dashboard entry with that tag") + + userA.AssertHasTagDefinition("project", false) + userB.AssertHasTagDefinition("project", true) +} diff --git a/tag/update.go b/tag/update.go index e6baa574..9614b308 100644 --- a/tag/update.go +++ b/tag/update.go @@ -49,10 +49,10 @@ func (r *ResolverForTag) UpdateTag(ctx context.Context, key string, newKey *stri } usedInEntries := []model.DashboardEntry{} - // Do not read the next statements, not proud of it. - if err := tx.Where("keys LIKE ?", "%"+key). - Or("keys like ?", "%"+key+"%"). - Or("keys like ?", key+"%"). + if err := tx. + Joins("INNER JOIN dashboards ON dashboard_entries.dashboard_id = dashboards.id"). + Where("dashboards.user_id = ?", userID). + Where("keys LIKE ? OR keys LIKE ? OR keys LIKE ?", "%"+key, "%"+key+"%", key+"%"). Find(&usedInEntries).Error; err != nil { tx.Rollback() return nil, err diff --git a/tag/update_test.go b/tag/update_test.go index ae3d7b28..3f78f672 100644 --- a/tag/update_test.go +++ b/tag/update_test.go @@ -123,3 +123,36 @@ func TestUpdate_noPermissions(t *testing.T) { require.EqualError(t, err, "tag with key 'coolio' does not exist") right.AssertHasTagDefinition("coolio", true) } + +func TestUpdate_dashboardEntryKey_crossUserIsolation(t *testing.T) { + db := test.InMemoryDB(t) + defer db.Close() + userA := db.User(5) + userB := db.User(2) + + userA.NewTagDefinition("project") + userB.NewTagDefinition("project") + + dashboardA := userA.Dashboard("dashboardA") + dashboardA.Entry("entryA") + entryA := dashboardA.Dashboard.Entries[0] + entryA.Keys = "project,task1" + db.Save(&entryA) + + dashboardB := userB.Dashboard("dashboardB") + dashboardB.Entry("entryB") + entryB := dashboardB.Dashboard.Entries[0] + entryB.Keys = "project,task2" + db.Save(&entryB) + + newTag := "newproject" + resolver := ResolverForTag{DB: db.DB} + _, err := resolver.UpdateTag(fake.User(userA.User.ID), "project", &newTag, "#abc") + require.NoError(t, err) + + db.Find(&entryA) + require.Equal(t, "newproject,task1", entryA.Keys, "user A's entry should be updated") + + db.Find(&entryB) + require.Equal(t, "project,task2", entryB.Keys, "user B's entry should NOT be modified") +}