Merge pull request #14370 from reasonerjt/oidc-ping-refact

Switch API to ping OIDC endpoint to new model

api/v2.0/legacy_swagger.yaml

@@ -1737,37 +1737,6 @@ paths:
           $ref: '#/definitions/NotFoundChartAPIError'
         '500':
           $ref: '#/definitions/InternalChartAPIError'
-  '/system/oidc/ping':
-    post:
-      summary: Test the OIDC endpoint.
-      description: Test the OIDC endpoint, the setting of the endpoint is provided in the request.  This API can only
-        be called by system admin.
-      tags:
-        - Products
-        - System
-      parameters:
-        - name: endpoint
-          in: body
-          description: Request body for OIDC endpoint to be tested.
-          required: true
-          schema:
-            type: object
-            properties:
-              url:
-                type: string
-                description: The URL of OIDC endpoint to be tested.
-              verify_cert:
-                type: boolean
-                description: Whether the certificate should be verified
-      responses:
-        '200':
-          description: Ping succeeded.  The OIDC endpoint is valid.
-        '400':
-          description: The ping failed
-        '401':
-          description: User need to log in first.
-        '403':
-          description: User does not have permission to call this API
   '/system/CVEAllowlist':
     get:
       summary: Get the system level allowlist of CVE.

api/v2.0/swagger.yaml

@@ -2306,6 +2306,37 @@ paths:
           description: Not found the default root certificate.
         '500':
           $ref: '#/responses/500'
+  /system/oidc/ping:
+    post:
+      summary: Test the OIDC endpoint.
+      description: |
+        Test the OIDC endpoint, the setting of the endpoint is provided in the request.  This API can only be called by system admin.
+      tags:
+        - oidc
+      operationId: pingOIDC
+      parameters:
+        - name: endpoint
+          in: body
+          description: Request body for OIDC endpoint to be tested.
+          required: true
+          schema:
+            type: object
+            properties:
+              url:
+                type: string
+                description: The URL of OIDC endpoint to be tested.
+              verify_cert:
+                type: boolean
+                description: Whether the certificate should be verified
+      responses:
+        '200':
+          $ref: '#/responses/200'
+        '400':
+          $ref: '#/responses/400'
+        '401':
+          $ref: '#/responses/401'
+        '403':
+          $ref: '#/responses/403'
   /system/gc:
     get:
       summary: Get gc results.

src/common/rbac/const.go

@@ -72,4 +72,5 @@ const (
 	ResourceReplicationPolicy  = Resource("replication-policy")
 	ResourceScanAll            = Resource("scan-all")
 	ResourceSystemVolumes      = Resource("system-volumes")
+	ResourceOIDCEndpoint       = Resource("oidc-endpoint")
 )

src/common/rbac/system/policies.go

@@ -60,5 +60,8 @@ var (
 		{Resource: rbac.ResourceScanAll, Action: rbac.ActionList},
 
 		{Resource: rbac.ResourceSystemVolumes, Action: rbac.ActionRead},
+
+		{Resource: rbac.ResourceOIDCEndpoint, Action: rbac.ActionUpdate},
+		{Resource: rbac.ResourceOIDCEndpoint, Action: rbac.ActionRead},
 	}
 )

src/core/api/harborapi_test.go

@@ -122,7 +122,6 @@ func init() {
 	beego.Router("/api/labels", &LabelAPI{}, "post:Post;get:List")
 	beego.Router("/api/labels/:id([0-9]+", &LabelAPI{}, "get:Get;put:Put;delete:Delete")
 	beego.Router("/api/system/CVEAllowlist", &SysCVEAllowlistAPI{}, "get:Get;put:Put")
-	beego.Router("/api/system/oidc/ping", &OIDCAPI{}, "post:Ping")
 
 	beego.Router("/api/replication/adapters", &ReplicationAdapterAPI{}, "get:List")
 

src/core/api/oidc.go

@@ -1,57 +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 api
-
-import (
-	"errors"
-
-	"github.com/goharbor/harbor/src/common/utils/oidc"
-	"github.com/goharbor/harbor/src/lib/log"
-)
-
-// OIDCAPI handles the requests to /api/system/oidc/xxx
-type OIDCAPI struct {
-	BaseController
-}
-
-// Prepare validates the request initially
-func (oa *OIDCAPI) Prepare() {
-	oa.BaseController.Prepare()
-	if !oa.SecurityCtx.IsAuthenticated() {
-		oa.SendUnAuthorizedError(errors.New("unauthorized"))
-		return
-	}
-	if !oa.SecurityCtx.IsSysAdmin() {
-		msg := "only system admin has permission to access this API"
-		log.Errorf(msg)
-		oa.SendForbiddenError(errors.New(msg))
-		return
-	}
-}
-
-// Ping will handles the request to test connection to OIDC endpoint
-func (oa *OIDCAPI) Ping() {
-	var c oidc.Conn
-	if err := oa.DecodeJSONReq(&c); err != nil {
-		log.Error("Failed to decode JSON request.")
-		oa.SendBadRequestError(err)
-		return
-	}
-	if err := oidc.TestEndpoint(c); err != nil {
-		log.Errorf("Failed to verify connection: %+v, err: %v", c, err)
-		oa.SendBadRequestError(errors.New("failed to verify connection"))
-		return
-	}
-}

src/core/api/oidc_test.go

@@ -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 api
-
-import (
-	"github.com/goharbor/harbor/src/common/utils/oidc"
-	"net/http"
-	"testing"
-)
-
-func TestOIDCAPI_Ping(t *testing.T) {
-	url := "/api/system/oidc/ping"
-	cases := []*codeCheckingCase{
-		{ // 401
-			request: &testingRequest{
-				method:   http.MethodPost,
-				bodyJSON: oidc.Conn{},
-				url:      url,
-			},
-			code: http.StatusUnauthorized,
-		},
-		{ // 403
-			request: &testingRequest{
-				method:     http.MethodPost,
-				bodyJSON:   oidc.Conn{},
-				url:        url,
-				credential: nonSysAdmin,
-			},
-			code: http.StatusForbidden,
-		},
-		{ // 400
-			request: &testingRequest{
-				method: http.MethodPost,
-				bodyJSON: oidc.Conn{
-					URL:        "https://www.baidu.com",
-					VerifyCert: true,
-				},
-				url:        url,
-				credential: sysAdmin,
-			},
-			code: http.StatusBadRequest,
-		},
-		{ // 200
-			request: &testingRequest{
-				method: http.MethodPost,
-				bodyJSON: oidc.Conn{
-					URL:        "https://accounts.google.com",
-					VerifyCert: true,
-				},
-				url:        url,
-				credential: sysAdmin,
-			},
-			code: http.StatusOK,
-		},
-	}
-	runCodeCheckingCases(t, cases...)
-}

src/core/controllers/oidc.go

@@ -24,11 +24,11 @@ import (
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
-	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/pkg/oidc"
 )
 
 const tokenKey = "oidc_token"

src/pkg/oidc/secret_test.go

@@ -3,8 +3,9 @@ package oidc
 import (
 	"context"
 	"fmt"
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestSecretVerifyError(t *testing.T) {

src/server/middleware/security/idtoken.go

@@ -22,10 +22,10 @@ import (
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/security/local"
-	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/pkg/oidc"
 )
 
 type idToken struct{}

src/server/middleware/security/oidc_cli.go

@@ -24,9 +24,9 @@ import (
 	"github.com/goharbor/harbor/src/common/api"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/security/local"
-	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/pkg/oidc"
 )
 
 var (

src/server/middleware/security/oidc_cli_test.go

@@ -16,13 +16,14 @@ package security
 
 import (
 	"fmt"
-	"github.com/goharbor/harbor/src/common"
-	"github.com/goharbor/harbor/src/common/utils/oidc"
-	"github.com/goharbor/harbor/src/lib"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"net/http"
 	"testing"
+
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/lib"
+	"github.com/goharbor/harbor/src/pkg/oidc"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestOIDCCli(t *testing.T) {

src/server/v2.0/handler/handler.go

@@ -48,6 +48,7 @@ func New() http.Handler {
 		GCAPI:          newGCAPI(),
 		QuotaAPI:       newQuotaAPI(),
 		RetentionAPI:   newRetentionAPI(),
+		OidcAPI:        newOIDCAPI(),
 	})
 	if err != nil {
 		log.Fatal(err)

src/server/v2.0/handler/oidc.go

@@ -0,0 +1,36 @@
+package handler
+
+import (
+	"context"
+
+	"github.com/go-openapi/runtime/middleware"
+	"github.com/goharbor/harbor/src/common/rbac"
+	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/lib/log"
+	oidcpkg "github.com/goharbor/harbor/src/pkg/oidc"
+	"github.com/goharbor/harbor/src/server/v2.0/restapi/operations/oidc"
+)
+
+type oidcAPI struct {
+	BaseAPI
+}
+
+func newOIDCAPI() *oidcAPI {
+	return &oidcAPI{}
+}
+
+func (o oidcAPI) PingOIDC(ctx context.Context, params oidc.PingOIDCParams) middleware.Responder {
+	if err := o.RequireSystemAccess(ctx, rbac.ActionUpdate, rbac.ResourceOIDCEndpoint); err != nil {
+		return o.SendError(ctx, err)
+	}
+	err := oidcpkg.TestEndpoint(oidcpkg.Conn{
+		URL:        params.Endpoint.URL,
+		VerifyCert: params.Endpoint.VerifyCert,
+	})
+
+	if err != nil {
+		log.Errorf("Failed to verify connection: %+v, err: %v", params.Endpoint, err)
+		return o.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("failed to verify connection"))
+	}
+	return oidc.NewPingOIDCOK()
+}

src/server/v2.0/route/legacy.go

@@ -43,7 +43,6 @@ func registerLegacyRoutes() {
 	beego.Router("/api/"+version+"/projects/:id([0-9]+)/metadatas/", &api.MetadataAPI{}, "post:Post")
 
 	beego.Router("/api/"+version+"/system/CVEAllowlist", &api.SysCVEAllowlistAPI{}, "get:Get;put:Put")
-	beego.Router("/api/"+version+"/system/oidc/ping", &api.OIDCAPI{}, "post:Ping")
 
 	beego.Router("/api/"+version+"/replication/adapters", &api.ReplicationAdapterAPI{}, "get:List")
 	beego.Router("/api/"+version+"/replication/adapterinfos", &api.ReplicationAdapterAPI{}, "get:ListAdapterInfos")