Skip to content

Commit 449f0d0

Browse files
laurentiuNiculaelaurentiuNiculae
authored
fix(repoinfo): fix userprefs values for repos returned by expanded repo info (#1413)
- now isBookmarked and isStarred are updated correctly Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>
1 parent e299ae1 commit 449f0d0

11 files changed

Lines changed: 377 additions & 6 deletions

File tree

‎pkg/extensions/search/resolver.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ func expandedRepoInfo(ctx context.Context, repo string, repoDB repodb.RepoDB, cv
10961096
return &gql_generated.RepoInfo{}, nil //nolint:nilerr // don't give details to a potential attacker
10971097
}
10981098

1099-
repoMeta, err := repoDB.GetRepoMeta(repo)
1099+
repoMeta, err := repoDB.GetUserRepoMeta(ctx, repo)
11001100
if err != nil {
11011101
log.Error().Err(err).Str("repository", repo).Msg("resolver: can't retrieve repoMeta for repo")
11021102

‎pkg/extensions/search/resolver_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3274,7 +3274,7 @@ func TestExpandedRepoInfo(t *testing.T) {
32743274
graphql.DefaultRecover)
32753275

32763276
repoDB := mocks.RepoDBMock{
3277-
GetRepoMetaFn: func(repo string) (repodb.RepoMetadata, error) {
3277+
GetUserRepoMetaFn: func(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
32783278
return repodb.RepoMetadata{
32793279
Tags: map[string]repodb.Descriptor{
32803280
"tagManifest": {

‎pkg/extensions/search/search_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4699,7 +4699,7 @@ func TestRepoDBIndexOperations(t *testing.T) {
46994699

47004700
func RunRepoDBIndexTests(baseURL, port string) {
47014701
Convey("Push test index", func() {
4702-
repo := "repo"
4702+
const repo = "repo"
47034703

47044704
multiarchImage, err := GetRandomMultiarchImage("tag1")
47054705
So(err, ShouldBeNil)

‎pkg/extensions/search/userprefs_test.go‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,198 @@ func TestGlobalSearchWithUserPrefFiltering(t *testing.T) {
806806
})
807807
}
808808

809+
func TestExpandedRepoInfoWithUserPrefs(t *testing.T) {
810+
Convey("ExpandedRepoInfo with User Prefs", t, func() {
811+
dir := t.TempDir()
812+
port := GetFreePort()
813+
baseURL := GetBaseURL(port)
814+
conf := config.New()
815+
conf.HTTP.Port = port
816+
conf.Storage.RootDirectory = dir
817+
818+
simpleUser := "simpleUser"
819+
simpleUserPassword := "simpleUserPass"
820+
credTests := fmt.Sprintf("%s\n\n", getCredString(simpleUser, simpleUserPassword))
821+
822+
htpasswdPath := MakeHtpasswdFileFromString(credTests)
823+
defer os.Remove(htpasswdPath)
824+
825+
conf.HTTP.Auth = &config.AuthConfig{
826+
HTPasswd: config.AuthHTPasswd{
827+
Path: htpasswdPath,
828+
},
829+
}
830+
831+
conf.HTTP.AccessControl = &config.AccessControlConfig{
832+
Repositories: config.Repositories{
833+
"**": config.PolicyGroup{
834+
Policies: []config.Policy{
835+
{
836+
Users: []string{simpleUser},
837+
Actions: []string{"read", "create"},
838+
},
839+
},
840+
},
841+
},
842+
}
843+
844+
defaultVal := true
845+
conf.Extensions = &extconf.ExtensionConfig{
846+
Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}},
847+
}
848+
849+
ctlr := api.NewController(conf)
850+
851+
ctlrManager := NewControllerManager(ctlr)
852+
ctlrManager.StartAndWait(port)
853+
defer ctlrManager.StopServer()
854+
855+
preferencesBaseURL := baseURL + constants.FullUserPreferencesPrefix
856+
simpleUserClient := resty.R().SetBasicAuth(simpleUser, simpleUserPassword)
857+
858+
// ------ Add sbrepo and star/bookmark it
859+
sbrepo := "sbrepo"
860+
img, err := GetRandomImage("tag")
861+
So(err, ShouldBeNil)
862+
err = UploadImageWithBasicAuth(img, baseURL, sbrepo, simpleUser, simpleUserPassword)
863+
So(err, ShouldBeNil)
864+
865+
resp, err := simpleUserClient.Put(preferencesBaseURL + PutRepoStarURL(sbrepo))
866+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
867+
So(err, ShouldBeNil)
868+
869+
resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoBookmarkURL(sbrepo))
870+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
871+
So(err, ShouldBeNil)
872+
873+
// ExpandedRepoinfo
874+
875+
query := `
876+
{
877+
ExpandedRepoInfo(repo:"sbrepo"){
878+
Summary {
879+
Name IsStarred IsBookmarked
880+
}
881+
}
882+
}`
883+
884+
resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
885+
So(resp, ShouldNotBeNil)
886+
So(err, ShouldBeNil)
887+
So(resp.StatusCode(), ShouldEqual, 200)
888+
889+
responseStruct := ExpandedRepoInfoResp{}
890+
891+
err = json.Unmarshal(resp.Body(), &responseStruct)
892+
So(err, ShouldBeNil)
893+
894+
repoInfo := responseStruct.ExpandedRepoInfo.RepoInfo
895+
So(repoInfo.Summary.IsBookmarked, ShouldBeTrue)
896+
So(repoInfo.Summary.IsStarred, ShouldBeTrue)
897+
898+
// ------ Add srepo and star it
899+
srepo := "srepo"
900+
img, err = GetRandomImage("tag")
901+
So(err, ShouldBeNil)
902+
err = UploadImageWithBasicAuth(img, baseURL, srepo, simpleUser, simpleUserPassword)
903+
So(err, ShouldBeNil)
904+
905+
resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoStarURL(srepo))
906+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
907+
So(err, ShouldBeNil)
908+
909+
// ExpandedRepoinfo
910+
query = `
911+
{
912+
ExpandedRepoInfo(repo:"srepo"){
913+
Summary {
914+
Name IsStarred IsBookmarked
915+
}
916+
}
917+
}`
918+
919+
resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
920+
So(resp, ShouldNotBeNil)
921+
So(err, ShouldBeNil)
922+
So(resp.StatusCode(), ShouldEqual, 200)
923+
924+
responseStruct = ExpandedRepoInfoResp{}
925+
926+
err = json.Unmarshal(resp.Body(), &responseStruct)
927+
So(err, ShouldBeNil)
928+
929+
repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
930+
So(repoInfo.Summary.IsBookmarked, ShouldBeFalse)
931+
So(repoInfo.Summary.IsStarred, ShouldBeTrue)
932+
933+
// ------ Add brepo and bookmark it
934+
brepo := "brepo"
935+
img, err = GetRandomImage("tag")
936+
So(err, ShouldBeNil)
937+
err = UploadImageWithBasicAuth(img, baseURL, brepo, simpleUser, simpleUserPassword)
938+
So(err, ShouldBeNil)
939+
940+
resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoBookmarkURL(brepo))
941+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
942+
So(err, ShouldBeNil)
943+
944+
// ExpandedRepoinfo
945+
query = `
946+
{
947+
ExpandedRepoInfo(repo:"brepo"){
948+
Summary {
949+
Name IsStarred IsBookmarked
950+
}
951+
}
952+
}`
953+
954+
resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
955+
So(resp, ShouldNotBeNil)
956+
So(err, ShouldBeNil)
957+
So(resp.StatusCode(), ShouldEqual, 200)
958+
959+
responseStruct = ExpandedRepoInfoResp{}
960+
961+
err = json.Unmarshal(resp.Body(), &responseStruct)
962+
So(err, ShouldBeNil)
963+
964+
repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
965+
So(repoInfo.Summary.IsBookmarked, ShouldBeTrue)
966+
So(repoInfo.Summary.IsStarred, ShouldBeFalse)
967+
968+
// ------ Add repo without star/bookmark
969+
repo := "repo"
970+
img, err = GetRandomImage("tag")
971+
So(err, ShouldBeNil)
972+
err = UploadImageWithBasicAuth(img, baseURL, repo, simpleUser, simpleUserPassword)
973+
So(err, ShouldBeNil)
974+
975+
// ExpandedRepoinfo
976+
query = `
977+
{
978+
ExpandedRepoInfo(repo:"repo"){
979+
Summary {
980+
Name IsStarred IsBookmarked
981+
}
982+
}
983+
}`
984+
985+
resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
986+
So(resp, ShouldNotBeNil)
987+
So(err, ShouldBeNil)
988+
So(resp.StatusCode(), ShouldEqual, 200)
989+
990+
responseStruct = ExpandedRepoInfoResp{}
991+
992+
err = json.Unmarshal(resp.Body(), &responseStruct)
993+
So(err, ShouldBeNil)
994+
995+
repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
996+
So(repoInfo.Summary.IsBookmarked, ShouldBeFalse)
997+
So(repoInfo.Summary.IsStarred, ShouldBeFalse)
998+
})
999+
}
1000+
8091001
func PutRepoStarURL(repo string) string {
8101002
return fmt.Sprintf("?repo=%s&action=toggleStar", repo)
8111003
}

‎pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,36 @@ func (bdw *DBWrapper) GetRepoMeta(repo string) (repodb.RepoMetadata, error) {
533533
return repoMeta, err
534534
}
535535

536+
func (bdw *DBWrapper) GetUserRepoMeta(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
537+
var repoMeta repodb.RepoMetadata
538+
539+
err := bdw.DB.Update(func(tx *bbolt.Tx) error {
540+
buck := tx.Bucket([]byte(bolt.RepoMetadataBucket))
541+
userBookmarks := getUserBookmarks(ctx, tx)
542+
userStars := getUserStars(ctx, tx)
543+
544+
repoMetaBlob := buck.Get([]byte(repo))
545+
546+
// object not found
547+
if repoMetaBlob == nil {
548+
return zerr.ErrRepoMetaNotFound
549+
}
550+
551+
// object found
552+
err := json.Unmarshal(repoMetaBlob, &repoMeta)
553+
if err != nil {
554+
return err
555+
}
556+
557+
repoMeta.IsBookmarked = zcommon.Contains(userBookmarks, repo)
558+
repoMeta.IsStarred = zcommon.Contains(userStars, repo)
559+
560+
return nil
561+
})
562+
563+
return repoMeta, err
564+
}
565+
536566
func (bdw *DBWrapper) SetRepoMeta(repo string, repoMeta repodb.RepoMetadata) error {
537567
err := bdw.DB.Update(func(tx *bbolt.Tx) error {
538568
buck := tx.Bucket([]byte(bolt.RepoMetadataBucket))

‎pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,30 @@ func TestWrapperErrors(t *testing.T) {
922922
)
923923
So(err, ShouldBeNil)
924924
})
925+
926+
Convey("GetUserRepoMeta unmarshal error", func() {
927+
acCtx := localCtx.AccessControlContext{
928+
ReadGlobPatterns: map[string]bool{
929+
"repo": true,
930+
},
931+
Username: "username",
932+
}
933+
authzCtxKey := localCtx.GetContextKey()
934+
ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
935+
936+
err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error {
937+
repoBuck := tx.Bucket([]byte(bolt.RepoMetadataBucket))
938+
939+
err := repoBuck.Put([]byte("repo"), []byte("bad repo"))
940+
So(err, ShouldBeNil)
941+
942+
return nil
943+
})
944+
So(err, ShouldBeNil)
945+
946+
_, err := boltdbWrapper.GetUserRepoMeta(ctx, "repo")
947+
So(err, ShouldNotBeNil)
948+
})
925949
})
926950
}
927951

‎pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,55 @@ func TestWrapperErrors(t *testing.T) {
10371037
err := dynamoWrapper.PatchDB()
10381038
So(err, ShouldNotBeNil)
10391039
})
1040+
1041+
Convey("GetUserRepoMeta client.GetItem error", func() {
1042+
dynamoWrapper.RepoMetaTablename = badTablename
1043+
1044+
_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
1045+
So(err, ShouldNotBeNil)
1046+
})
1047+
1048+
Convey("GetUserRepoMeta repoMeta not found", func() {
1049+
_, err = dynamoWrapper.GetUserRepoMeta(ctx, "unknown-repo-meta")
1050+
So(err, ShouldNotBeNil)
1051+
})
1052+
1053+
Convey("GetUserRepoMeta userMeta not found", func() {
1054+
err := dynamoWrapper.SetRepoReference("repo", "tag", digest.FromString("1"), ispec.MediaTypeImageManifest)
1055+
So(err, ShouldBeNil)
1056+
dynamoWrapper.UserDataTablename = badTablename
1057+
1058+
acCtx := localCtx.AccessControlContext{
1059+
ReadGlobPatterns: map[string]bool{
1060+
"repo": true,
1061+
},
1062+
Username: "username",
1063+
}
1064+
1065+
authzCtxKey := localCtx.GetContextKey()
1066+
ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
1067+
1068+
_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
1069+
So(err, ShouldNotBeNil)
1070+
})
1071+
1072+
Convey("GetUserRepoMeta unmarshal error", func() {
1073+
err := setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo")
1074+
So(err, ShouldBeNil)
1075+
1076+
acCtx := localCtx.AccessControlContext{
1077+
ReadGlobPatterns: map[string]bool{
1078+
"repo": true,
1079+
},
1080+
Username: "username",
1081+
}
1082+
1083+
authzCtxKey := localCtx.GetContextKey()
1084+
ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
1085+
1086+
_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
1087+
So(err, ShouldNotBeNil)
1088+
})
10401089
})
10411090

10421091
Convey("NewDynamoDBWrapper errors", t, func() {

0 commit comments

Comments
 (0)