From 4aa7b29debe1fa727d4c4ee2749996f4923a5980 Mon Sep 17 00:00:00 2001 From: Fittkau Luis Date: Wed, 14 Aug 2024 11:39:01 +0200 Subject: [PATCH] Extend oidc_cli security context generator to include calls to the v2 API, remove idtoken security context generator, rename and consolidate tests and names accordingly Signed-off-by: Fittkau Luis --- src/server/middleware/security/idtoken.go | 69 ------------------- .../middleware/security/idtoken_test.go | 50 -------------- .../{oidc_cli.go => oidc_cli_or_api.go} | 11 ++- ...dc_cli_test.go => oidc_cli_or_api_test.go} | 39 ++++++++--- src/server/middleware/security/security.go | 3 +- 5 files changed, 38 insertions(+), 134 deletions(-) delete mode 100644 src/server/middleware/security/idtoken.go delete mode 100644 src/server/middleware/security/idtoken_test.go rename src/server/middleware/security/{oidc_cli.go => oidc_cli_or_api.go} (91%) rename src/server/middleware/security/{oidc_cli_test.go => oidc_cli_or_api_test.go} (81%) diff --git a/src/server/middleware/security/idtoken.go b/src/server/middleware/security/idtoken.go deleted file mode 100644 index 11ec0624546..00000000000 --- a/src/server/middleware/security/idtoken.go +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package security - -import ( - "net/http" - "strings" - - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/security" - "github.com/goharbor/harbor/src/common/security/local" - "github.com/goharbor/harbor/src/controller/user" - "github.com/goharbor/harbor/src/lib" - "github.com/goharbor/harbor/src/lib/config" - "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/pkg/oidc" -) - -type idToken struct{} - -func (i *idToken) Generate(req *http.Request) security.Context { - ctx := req.Context() - log := log.G(ctx) - if lib.GetAuthMode(ctx) != common.OIDCAuth { - return nil - } - if !strings.HasPrefix(req.URL.Path, "/api") && req.URL.Path != "/service/token" { - return nil - } - token := bearerToken(req) - if len(token) == 0 { - return nil - } - claims, err := oidc.VerifyToken(ctx, token) - if err != nil { - log.Warningf("failed to verify token: %v", err) - return nil - } - u, err := user.Ctl.GetBySubIss(ctx, claims.Subject, claims.Issuer) - if err != nil { - log.Warningf("failed to get user based on token claims: %v", err) - return nil - } - setting, err := config.OIDCSetting(ctx) - if err != nil { - log.Errorf("failed to get OIDC settings: %v", err) - return nil - } - info, err := oidc.UserInfoFromIDToken(ctx, &oidc.Token{RawIDToken: token}, *setting) - if err != nil { - log.Errorf("Failed to get user info from ID token: %v", err) - return nil - } - oidc.InjectGroupsToUser(info, u) - log.Debugf("an ID token security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(u) -} diff --git a/src/server/middleware/security/idtoken_test.go b/src/server/middleware/security/idtoken_test.go deleted file mode 100644 index ce23044ceb0..00000000000 --- a/src/server/middleware/security/idtoken_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package security - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/lib" -) - -func TestIDToken(t *testing.T) { - idToken := &idToken{} - - // not the OIDC mode - req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) - require.Nil(t, err) - ctx := idToken.Generate(req) - assert.Nil(t, ctx) - - // contains no authorization header - req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) - require.Nil(t, err) - req = req.WithContext(lib.WithAuthMode(req.Context(), common.OIDCAuth)) - ctx = idToken.Generate(req) - assert.Nil(t, ctx) - - // contains no authorization header - req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token?service=harbor-registry&scope=repository:foo/bar:pull", nil) - require.Nil(t, err) - req = req.WithContext(lib.WithAuthMode(req.Context(), common.OIDCAuth)) - ctx = idToken.Generate(req) - assert.Nil(t, ctx) -} diff --git a/src/server/middleware/security/oidc_cli.go b/src/server/middleware/security/oidc_cli_or_api.go similarity index 91% rename from src/server/middleware/security/oidc_cli.go rename to src/server/middleware/security/oidc_cli_or_api.go index f62ae2049a0..11d5abeb9fe 100644 --- a/src/server/middleware/security/oidc_cli.go +++ b/src/server/middleware/security/oidc_cli_or_api.go @@ -43,9 +43,9 @@ var ( uctl = user.Ctl ) -type oidcCli struct{} +type oidcCliOrAPI struct{} -func (o *oidcCli) Generate(req *http.Request) security.Context { +func (o *oidcCliOrAPI) Generate(req *http.Request) security.Context { ctx := req.Context() if lib.GetAuthMode(ctx) != common.OIDCAuth { return nil @@ -78,7 +78,7 @@ func (o *oidcCli) Generate(req *http.Request) security.Context { return local.NewSecurityContext(u) } -func (o *oidcCli) valid(req *http.Request) bool { +func (o *oidcCliOrAPI) valid(req *http.Request) bool { path := strings.TrimSuffix(req.URL.Path, "/") if path == "/service/token" || @@ -104,5 +104,10 @@ func (o *oidcCli) valid(req *http.Request) bool { if req.Method == http.MethodDelete && tagsAPIRe.MatchString(path) { // deleting tags return true } + + // The request was sent to the v2 API directly + if strings.HasPrefix(req.URL.Path, "/api") || req.URL.Path == "/service/token" { + return true + } return false } diff --git a/src/server/middleware/security/oidc_cli_test.go b/src/server/middleware/security/oidc_cli_or_api_test.go similarity index 81% rename from src/server/middleware/security/oidc_cli_test.go rename to src/server/middleware/security/oidc_cli_or_api_test.go index de32ef7e524..e492febd599 100644 --- a/src/server/middleware/security/oidc_cli_test.go +++ b/src/server/middleware/security/oidc_cli_or_api_test.go @@ -31,17 +31,12 @@ import ( ) func TestOIDCCli(t *testing.T) { - oidcCli := &oidcCli{} - // not the candidate request - req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/v2.0/users/", nil) - require.Nil(t, err) - ctx := oidcCli.Generate(req) - assert.Nil(t, ctx) + oc := &oidcCliOrAPI{} // the auth mode isn't OIDC - req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil) + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil) require.Nil(t, err) - ctx = oidcCli.Generate(req) + ctx := oc.Generate(req) assert.Nil(t, ctx) // pass @@ -57,12 +52,12 @@ func TestOIDCCli(t *testing.T) { oidc.SetHardcodeVerifierForTest(password) req = req.WithContext(lib.WithAuthMode(req.Context(), common.OIDCAuth)) req.SetBasicAuth(username, password) - ctx = oidcCli.Generate(req) + ctx = oc.Generate(req) assert.NotNil(t, ctx) } func TestOIDCCliValid(t *testing.T) { - oc := &oidcCli{} + oc := &oidcCliOrAPI{} req1, _ := http.NewRequest(http.MethodPost, "https://test.goharbor.io/api/v2.0/projects", nil) req2, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects?name=test", nil) req3, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects/library/repositories/", nil) @@ -97,3 +92,27 @@ func TestOIDCCliValid(t *testing.T) { } } + +func TestOIDCAPI(t *testing.T) { + oc := &oidcCliOrAPI{} + + // not the OIDC mode + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) + require.Nil(t, err) + ctx := oc.Generate(req) + assert.Nil(t, ctx) + + // contains no authorization header + req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) + require.Nil(t, err) + req = req.WithContext(lib.WithAuthMode(req.Context(), common.OIDCAuth)) + ctx = oc.Generate(req) + assert.Nil(t, ctx) + + // contains no authorization header + req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token?service=harbor-registry&scope=repository:foo/bar:pull", nil) + require.Nil(t, err) + req = req.WithContext(lib.WithAuthMode(req.Context(), common.OIDCAuth)) + ctx = oc.Generate(req) + assert.Nil(t, ctx) +} diff --git a/src/server/middleware/security/security.go b/src/server/middleware/security/security.go index 8ecefc65fcf..c202b402fb3 100644 --- a/src/server/middleware/security/security.go +++ b/src/server/middleware/security/security.go @@ -27,9 +27,8 @@ import ( var ( generators = []generator{ &secret{}, - &oidcCli{}, + &oidcCliOrAPI{}, &v2Token{}, - &idToken{}, &authProxy{}, &robot{}, &basicAuth{},