Merge pull request #7335 from reasonerjt/oidc-onboard-e2e

OIDC E2E flow + secret support

make/migrations/postgresql/0004_1.8.0_schema.up.sql

@@ -16,6 +16,9 @@ CREATE TRIGGER robot_update_time_at_modtime BEFORE UPDATE ON robot FOR EACH ROW
 CREATE TABLE oidc_user (
  id SERIAL NOT NULL,
  user_id int NOT NULL,
+ /*
+ Encoded secret
+  */
  secret varchar(255) NOT NULL,
   /*
  Subject and Issuer
@@ -24,9 +27,14 @@ CREATE TABLE oidc_user (
   The sub (subject) and iss (issuer) Claims, used together, are the only Claims that an RP can rely upon as a stable identifier for the End-User
  */
  subiss varchar(255) NOT NULL,
+ /*
+ Encoded token
+  */
+ token text,
  creation_time timestamp default CURRENT_TIMESTAMP,
  update_time timestamp default CURRENT_TIMESTAMP,
  PRIMARY KEY (id),
+ FOREIGN KEY (user_id) REFERENCES harbor_user(user_id),
  UNIQUE (subiss)
 );
 

src/common/config/keyprovider.go

@@ -46,3 +46,13 @@ func (f *FileKeyProvider) Get(params map[string]interface{}) (string, error) {
 	}
 	return string(b), nil
 }
+
+// PresetKeyProvider returns the preset key disregarding the parm, this is for testing only
+type PresetKeyProvider struct {
+	Key string
+}
+
+// Get ...
+func (p *PresetKeyProvider) Get(params map[string]interface{}) (string, error) {
+	return p.Key, nil
+}

src/common/config/keyprovider_test.go

@@ -15,6 +15,7 @@
 package config
 
 import (
+	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -42,3 +43,12 @@ func TestGetOfFileKeyProvider(t *testing.T) {
 		return
 	}
 }
+
+func TestPresetKeyProvider(t *testing.T) {
+	kp := &PresetKeyProvider{
+		Key: "mykey",
+	}
+	k, err := kp.Get(nil)
+	assert.Nil(t, err)
+	assert.Equal(t, "mykey", k)
+}

src/common/models/oidc_user.go

@@ -6,10 +6,14 @@ import (
 
 // OIDCUser ...
 type OIDCUser struct {
-	ID           int64     `orm:"pk;auto;column(id)" json:"id"`
-	UserID       int       `orm:"column(user_id)" json:"user_id"`
-	Secret       string    `orm:"column(secret)" json:"secret"`
+	ID     int64 `orm:"pk;auto;column(id)" json:"id"`
+	UserID int   `orm:"column(user_id)" json:"user_id"`
+	// encrypted secret
+	Secret string `orm:"column(secret)" json:"-"`
+	// secret in plain text
+	PlainSecret  string    `orm:"-" json:"secret"`
 	SubIss       string    `orm:"column(subiss)" json:"subiss"`
+	Token        string    `orm:"column(token)" json:"-"`
 	CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"`
 	UpdateTime   time.Time `orm:"column(update_time);auto_now" json:"update_time"`
 }

src/common/utils/oidc/helper.go

@@ -90,16 +90,7 @@ func (p *providerHelper) create() error {
 		return errors.New("the configuration is not loaded")
 	}
 	s := p.setting.Load().(models.OIDCSetting)
-	var client *http.Client
-	if s.SkipCertVerify {
-		client = &http.Client{
-			Transport: insecureTransport,
-		}
-	} else {
-		client = &http.Client{}
-	}
-	ctx := context.Background()
-	gooidc.ClientContext(ctx, client)
+	ctx := clientCtx(context.Background(), s.SkipCertVerify)
 	provider, err := gooidc.NewProvider(ctx, s.Endpoint)
 	if err != nil {
 		return fmt.Errorf("failed to create OIDC provider, error: %v", err)
@@ -170,6 +161,8 @@ func ExchangeToken(ctx context.Context, code string) (*Token, error) {
 		log.Errorf("Failed to get OAuth configuration, error: %v", err)
 		return nil, err
 	}
+	setting := provider.setting.Load().(models.OIDCSetting)
+	ctx = clientCtx(ctx, setting.SkipCertVerify)
 	oauthToken, err := oauth.Exchange(ctx, code)
 	if err != nil {
 		return nil, err
@@ -184,5 +177,36 @@ func VerifyToken(ctx context.Context, rawIDToken string) (*gooidc.IDToken, error
 		return nil, err
 	}
 	verifier := p.Verifier(&gooidc.Config{ClientID: provider.setting.Load().(models.OIDCSetting).ClientID})
+	setting := provider.setting.Load().(models.OIDCSetting)
+	ctx = clientCtx(ctx, setting.SkipCertVerify)
 	return verifier.Verify(ctx, rawIDToken)
 }
+
+func clientCtx(ctx context.Context, skipCertVerify bool) context.Context {
+	var client *http.Client
+	if skipCertVerify {
+		client = &http.Client{
+			Transport: insecureTransport,
+		}
+	} else {
+		client = &http.Client{}
+	}
+	return gooidc.ClientContext(ctx, client)
+}
+
+// RefreshToken refreshes the token passed in parameter, and return the new token.
+func RefreshToken(ctx context.Context, token *Token) (*Token, error) {
+	oauth, err := getOauthConf()
+	if err != nil {
+		log.Errorf("Failed to get OAuth configuration, error: %v", err)
+		return nil, err
+	}
+	setting := provider.setting.Load().(models.OIDCSetting)
+	ctx = clientCtx(ctx, setting.SkipCertVerify)
+	ts := oauth.TokenSource(ctx, token.Token)
+	t, err := ts.Token()
+	if err != nil {
+		return nil, err
+	}
+	return &Token{Token: t, IDToken: t.Extra("id_token").(string)}, nil
+}

src/common/utils/oidc/helper_test.go

@@ -16,6 +16,7 @@ package oidc
 
 import (
 	"github.com/goharbor/harbor/src/common"
+	config2 "github.com/goharbor/harbor/src/common/config"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/stretchr/testify/assert"
@@ -36,8 +37,9 @@ func TestMain(m *testing.M) {
 		common.OIDCClientSecret:   "secret",
 		common.ExtEndpoint:        "https://harbor.test",
 	}
+	kp := &config2.PresetKeyProvider{Key: "naa4JtarA1Zsc3uY"}
 
-	config.InitWithSettings(conf)
+	config.InitWithSettings(conf, kp)
 
 	result := m.Run()
 	if result != 0 {

src/common/utils/oidc/secret.go

@@ -0,0 +1,130 @@
+package oidc
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"github.com/goharbor/harbor/src/common/dao"
+	"github.com/goharbor/harbor/src/common/utils"
+	"github.com/goharbor/harbor/src/common/utils/log"
+	"github.com/goharbor/harbor/src/core/config"
+	"github.com/pkg/errors"
+	"sync"
+)
+
+// SecretVerifyError wraps the different errors happened when verifying a secret for OIDC user.  When seeing this error,
+// the caller should consider this an authentication error.
+type SecretVerifyError struct {
+	cause error
+}
+
+func (se *SecretVerifyError) Error() string {
+	return fmt.Sprintf("failed to verify the secret: %v", se.cause)
+}
+
+func verifyError(err error) error {
+	return &SecretVerifyError{err}
+}
+
+// SecretManager is the interface for store and verify the secret
+type SecretManager interface {
+	// SetSecret sets the secret and token based on the ID of the user, when setting the secret the user has to be
+	// onboarded to Harbor DB.
+	SetSecret(userID int, secret string, token *Token) error
+	// VerifySecret verifies the secret and the token associated with it, it refreshes the token in the DB if it's
+	// refreshed during the verification
+	VerifySecret(ctx context.Context, userID int, secret string) error
+}
+
+type defaultManager struct {
+	sync.Mutex
+	key string
+}
+
+var m SecretManager = &defaultManager{}
+
+func (dm *defaultManager) getEncryptKey() (string, error) {
+	if dm.key == "" {
+		dm.Lock()
+		defer dm.Unlock()
+		if dm.key == "" {
+			key, err := config.SecretKey()
+			if err != nil {
+				return "", err
+			}
+			dm.key = key
+		}
+	}
+	return dm.key, nil
+}
+
+// SetSecret sets the secret and token based on the ID of the user, when setting the secret the user has to be
+// onboarded to Harbor DB.
+func (dm *defaultManager) SetSecret(userID int, secret string, token *Token) error {
+	key, err := dm.getEncryptKey()
+	if err != nil {
+		return fmt.Errorf("failed to load the key for encryption/decryption： %v", err)
+	}
+	oidcUser, err := dao.GetOIDCUserByUserID(userID)
+	if oidcUser == nil {
+		return fmt.Errorf("failed to get oidc user info, error: %v", err)
+	}
+	encSecret, _ := utils.ReversibleEncrypt(secret, key)
+	tb, _ := json.Marshal(token)
+	encToken, _ := utils.ReversibleEncrypt(string(tb), key)
+	oidcUser.Secret = encSecret
+	oidcUser.Token = encToken
+	return dao.UpdateOIDCUser(oidcUser)
+}
+
+// VerifySecret verifies the secret and the token associated with it, it tries to update the token in the DB if it's
+// refreshed during the verification
+func (dm *defaultManager) VerifySecret(ctx context.Context, userID int, secret string) error {
+	oidcUser, err := dao.GetOIDCUserByUserID(userID)
+	if err != nil {
+		return fmt.Errorf("failed to get oidc user info, error: %v", err)
+	}
+	if oidcUser == nil {
+		return fmt.Errorf("user is not onboarded as OIDC user")
+	}
+	key, err := dm.getEncryptKey()
+	if err != nil {
+		return fmt.Errorf("failed to load the key for encryption/decryption： %v", err)
+	}
+	plainSecret, err := utils.ReversibleDecrypt(oidcUser.Secret, key)
+	if err != nil {
+		return fmt.Errorf("failed to decrypt secret from DB: %v", err)
+	}
+	if secret != plainSecret {
+		return verifyError(errors.New("secret mismatch"))
+	}
+	tokenStr, err := utils.ReversibleDecrypt(oidcUser.Token, key)
+	if err != nil {
+		return verifyError(err)
+	}
+	token := &Token{}
+	err = json.Unmarshal(([]byte)(tokenStr), token)
+	if err != nil {
+		return verifyError(err)
+	}
+	_, err = VerifyToken(ctx, token.IDToken)
+	if err == nil {
+		return nil
+	}
+	log.Infof("Failed to verify ID Token, error: %v, refreshing...", err)
+	t, err := RefreshToken(ctx, token)
+	if err != nil {
+		return verifyError(err)
+	}
+	err = dm.SetSecret(oidcUser.UserID, secret, t)
+	if err != nil {
+		log.Warningf("Failed to update the token in DB: %v, ignore this error.", err)
+	}
+	return nil
+}
+
+// VerifySecret verifies the secret and the token associated with it, it tries to update the token in the DB if it's
+// refreshed during the verification
+func VerifySecret(ctx context.Context, userID int, secret string) error {
+	return m.VerifySecret(ctx, userID, secret)
+}

src/common/utils/oidc/secret_test.go

@@ -0,0 +1,32 @@
+package oidc
+
+import (
+	"context"
+	"fmt"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestSecretVerifyError(t *testing.T) {
+	sve := &SecretVerifyError{cause: fmt.Errorf("myerror")}
+	assert.Equal(t, "failed to verify the secret: myerror", sve.Error())
+	err := verifyError(fmt.Errorf("myerror"))
+	assert.Equal(t, sve, err)
+}
+
+func TestDefaultManagerGetEncryptKey(t *testing.T) {
+	d := &defaultManager{}
+	k, err := d.getEncryptKey()
+	assert.Nil(t, err)
+	assert.Equal(t, "naa4JtarA1Zsc3uY", k)
+	d2 := &defaultManager{key: "oldkey"}
+	k2, err := d2.getEncryptKey()
+	assert.Nil(t, err)
+	assert.Equal(t, "oldkey", k2)
+}
+
+func TestPkgVerifySecret(t *testing.T) {
+	SetHardcodeVerifierForTest("secret")
+	assert.Nil(t, VerifySecret(context.Background(), 1, "secret"))
+	assert.NotNil(t, VerifySecret(context.Background(), 1, "not-the-secret"))
+}

src/common/utils/oidc/testutils.go

@@ -0,0 +1,26 @@
+package oidc
+
+import "context"
+import "errors"
+
+// This is for testing only
+type fakeVerifier struct {
+	secret string
+}
+
+func (fv *fakeVerifier) SetSecret(uid int, s string, t *Token) error {
+	return nil
+}
+
+func (fv *fakeVerifier) VerifySecret(ctx context.Context, userID int, secret string) error {
+	if secret != fv.secret {
+		return verifyError(errors.New("mismatch"))
+	}
+	return nil
+}
+
+// SetHardcodeVerifierForTest overwrite the default secret manager for testing.
+// Be reminded this is for testing only.
+func SetHardcodeVerifierForTest(s string) {
+	m = &fakeVerifier{s}
+}

src/core/api/user.go

@@ -118,12 +118,21 @@ func (ua *UserAPI) Get() {
 		u, err := dao.GetUser(userQuery)
 		if err != nil {
 			log.Errorf("Error occurred in GetUser, error: %v", err)
-			ua.CustomAbort(http.StatusInternalServerError, "Internal error.")
+			ua.RenderFormatedError(http.StatusInternalServerError, err)
+			return
 		}
 		u.Password = ""
 		if ua.userID == ua.currentUserID {
 			u.HasAdminRole = ua.SecurityCtx.IsSysAdmin()
 		}
+		if ua.AuthMode == common.OIDCAuth {
+			o, err := ua.getOIDCUserInfo()
+			if err != nil {
+				ua.RenderFormatedError(http.StatusInternalServerError, err)
+				return
+			}
+			u.OIDCUserMeta = o
+		}
 		ua.Data["json"] = u
 		ua.ServeJSON()
 		return
@@ -429,6 +438,25 @@ func (ua *UserAPI) ListUserPermissions() {
 	return
 }
 
+func (ua *UserAPI) getOIDCUserInfo() (*models.OIDCUser, error) {
+	key, err := config.SecretKey()
+	if err != nil {
+		return nil, err
+	}
+	o, err := dao.GetOIDCUserByUserID(ua.userID)
+	if err != nil || o == nil {
+		return nil, err
+	}
+	if len(o.Secret) > 0 {
+		p, err := utils.ReversibleDecrypt(o.Secret, key)
+		if err != nil {
+			return nil, err
+		}
+		o.PlainSecret = p
+	}
+	return o, nil
+}
+
 // modifiable returns whether the modify is allowed based on current auth mode and context
 func (ua *UserAPI) modifiable() bool {
 	if ua.AuthMode == common.DBAuth {

src/core/config/config.go

@@ -80,11 +80,14 @@ func Init() error {
 	return nil
 }
 
-// InitWithSettings init config with predefined configs
-func InitWithSettings(cfgs map[string]interface{}) {
+// InitWithSettings init config with predefined configs, and optionally overwrite the keyprovider
+func InitWithSettings(cfgs map[string]interface{}, kp ...comcfg.KeyProvider) {
 	Init()
 	cfgMgr = comcfg.NewInMemoryManager()
 	cfgMgr.UpdateConfig(cfgs)
+	if len(kp) > 0 {
+		keyProvider = kp[0]
+	}
 }
 
 func initKeyProvider() {

src/core/controllers/base.go

@@ -35,6 +35,8 @@ import (
 	"github.com/goharbor/harbor/src/core/config"
 )
 
+const userKey = "user"
+
 // CommonController handles request from UI that doesn't expect a page, such as /SwitchLanguage /logout ...
 type CommonController struct {
 	beego.Controller
@@ -69,7 +71,7 @@ func (cc *CommonController) Login() {
 	if user == nil {
 		cc.CustomAbort(http.StatusUnauthorized, "")
 	}
-	cc.SetSession("user", *user)
+	cc.SetSession(userKey, *user)
 }
 
 // LogOut Habor UI

src/core/controllers/oidc.go

@@ -21,6 +21,7 @@ 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/log"
 	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/core/config"
@@ -29,14 +30,19 @@ import (
 	"strings"
 )
 
-const idTokenKey = "oidc_id_token"
+const tokenKey = "oidc_token"
 const stateKey = "oidc_state"
+const userInfoKey = "oidc_user_info"
 
 // OIDCController handles requests for OIDC login, callback and user onboard
 type OIDCController struct {
 	api.BaseController
 }
 
+type onboardReq struct {
+	Username string `json:"username"`
+}
+
 type oidcUserData struct {
 	Issuer   string `json:"iss"`
 	Subject  string `json:"sub"`
@@ -94,16 +100,33 @@ func (oc *OIDCController) Callback() {
 		oc.RenderFormatedError(http.StatusInternalServerError, err)
 		return
 	}
-	oc.SetSession(idTokenKey, string(ouDataStr))
-	// TODO: check and trigger onboard popup or redirect user to project page
-	oc.Data["json"] = d
-	oc.ServeFormatted()
+	u, err := dao.GetUserBySubIss(d.Subject, d.Issuer)
+	if err != nil {
+		oc.RenderFormatedError(http.StatusInternalServerError, err)
+		return
+	}
+	tokenBytes, err := json.Marshal(token)
+	if err != nil {
+		oc.RenderFormatedError(http.StatusInternalServerError, err)
+		return
+	}
+	oc.SetSession(tokenKey, tokenBytes)
+
+	if u == nil {
+		oc.SetSession(userInfoKey, string(ouDataStr))
+		oc.Controller.Redirect("/oidc-onboard", http.StatusFound)
+	} else {
+		oc.SetSession(userKey, *u)
+		oc.Controller.Redirect("/", http.StatusFound)
+	}
+
 }
 
 // Onboard handles the request to onboard an user authenticated via OIDC provider
 func (oc *OIDCController) Onboard() {
-
-	username := oc.GetString("username")
+	u := &onboardReq{}
+	oc.DecodeJSONReq(u)
+	username := u.Username
 	if utils.IsIllegalLength(username, 1, 255) {
 		oc.RenderFormatedError(http.StatusBadRequest, errors.New("username with illegal length"))
 		return
@@ -113,21 +136,36 @@ func (oc *OIDCController) Onboard() {
 		return
 	}
 
-	idTokenStr := oc.GetSession(idTokenKey)
+	userInfoStr, ok := oc.GetSession(userInfoKey).(string)
+	if !ok {
+		oc.RenderError(http.StatusBadRequest, "Failed to get OIDC user info from session")
+		return
+	}
+	log.Debugf("User info string: %s\n", userInfoStr)
+	tb, ok := oc.GetSession(tokenKey).([]byte)
+	if !ok {
+		oc.RenderError(http.StatusBadRequest, "Failed to get OIDC token from session")
+		return
+	}
+	s, t, err := secretAndToken(tb)
+	if err != nil {
+		oc.RenderFormatedError(http.StatusInternalServerError, err)
+		return
+	}
 	d := &oidcUserData{}
-	err := json.Unmarshal([]byte(idTokenStr.(string)), &d)
+	err = json.Unmarshal([]byte(userInfoStr), &d)
 	if err != nil {
 		oc.RenderFormatedError(http.StatusInternalServerError, err)
 		return
 	}
 	oidcUser := models.OIDCUser{
 		SubIss: d.Subject + d.Issuer,
-		// TODO: get secret with secret manager.
-		Secret: utils.GenerateRandomString(),
+		Secret: s,
+		Token:  t,
 	}
 
-	var email string
-	if d.Email == "" {
+	email := d.Email
+	if email == "" {
 		email = utils.GenerateRandomString() + "@harbor.com"
 	}
 	user := models.User{
@@ -139,12 +177,32 @@ func (oc *OIDCController) Onboard() {
 	err = dao.OnBoardOIDCUser(&user)
 	if err != nil {
 		if strings.Contains(err.Error(), dao.ErrDupUser.Error()) {
-			oc.RenderFormatedError(http.StatusConflict, err)
+			oc.RenderError(http.StatusConflict, "Duplicate username")
 			return
 		}
 		oc.RenderFormatedError(http.StatusInternalServerError, err)
+		oc.DelSession(userInfoKey)
 		return
 	}
 
-	oc.Controller.Redirect(config.GetPortalURL(), http.StatusMovedPermanently)
+	user.OIDCUserMeta = nil
+	oc.SetSession(userKey, user)
+	oc.DelSession(userInfoKey)
+}
+
+func secretAndToken(tokenBytes []byte) (string, string, error) {
+	key, err := config.SecretKey()
+	if err != nil {
+		return "", "", err
+	}
+	token, err := utils.ReversibleEncrypt((string)(tokenBytes), key)
+	if err != nil {
+		return "", "", err
+	}
+	str := utils.GenerateRandomString()
+	secret, err := utils.ReversibleEncrypt(str, key)
+	if err != nil {
+		return "", "", err
+	}
+	return secret, token, nil
 }

src/core/filter/security.go

@@ -17,6 +17,7 @@ package filter
 import (
 	"context"
 	"fmt"
+	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"net/http"
 	"regexp"
 
@@ -110,6 +111,7 @@ func Init() {
 	// standalone
 	reqCtxModifiers = []ReqCtxModifier{
 		&secretReqCtxModifier{config.SecretStore},
+		&oidcCliReqCtxModifier{},
 		&authProxyReqCtxModifier{},
 		&robotAuthReqCtxModifier{},
 		&basicAuthReqCtxModifier{},
@@ -205,6 +207,47 @@ func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
 	return true
 }
 
+type oidcCliReqCtxModifier struct{}
+
+func (oc *oidcCliReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
+	path := ctx.Request.URL.Path
+	if path != "/service/token" || strings.HasPrefix(path, "/chartrepo/") {
+		log.Debug("OIDC CLI modifer only handles request by docker CLI or helm CLI")
+		return false
+	}
+	authMode, err := config.AuthMode()
+	if err != nil {
+		log.Errorf("fail to get auth mode, %v", err)
+		return false
+	}
+	if authMode != common.OIDCAuth {
+		return false
+	}
+	username, secret, ok := ctx.Request.BasicAuth()
+	if !ok {
+		return false
+	}
+
+	user, err := dao.GetUser(models.User{
+		Username: username,
+	})
+	if err != nil {
+		log.Errorf("Failed to get user: %v", err)
+		return false
+	}
+	if user == nil {
+		return false
+	}
+	if err := oidc.VerifySecret(ctx.Request.Context(), user.UserID, secret); err != nil {
+		log.Errorf("Failed to verify secret: %v", err)
+		return false
+	}
+	pm := config.GlobalProjectMgr
+	sc := local.NewSecurityContext(user, pm)
+	setSecurCtxAndPM(ctx.Request, sc, pm)
+	return true
+}
+
 type authProxyReqCtxModifier struct{}
 
 func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
@@ -307,7 +350,6 @@ func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
 		return false
 	}
 
-	log.Debug("using local database project manager")
 	pm := config.GlobalProjectMgr
 	log.Debug("creating local database security context for auth proxy...")
 	securCtx := local.NewSecurityContext(user, pm)

src/core/filter/security_test.go

@@ -16,6 +16,8 @@ package filter
 
 import (
 	"context"
+	"github.com/goharbor/harbor/src/common/utils/oidc"
+	"github.com/stretchr/testify/require"
 	"log"
 	"net/http"
 	"net/http/httptest"
@@ -28,6 +30,7 @@ import (
 	"github.com/astaxie/beego"
 	beegoctx "github.com/astaxie/beego/context"
 	"github.com/astaxie/beego/session"
+	config2 "github.com/goharbor/harbor/src/common/config"
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
 	commonsecret "github.com/goharbor/harbor/src/common/secret"
@@ -118,6 +121,53 @@ func TestSecretReqCtxModifier(t *testing.T) {
 	assert.NotNil(t, projectManager(ctx))
 }
 
+func TestOIDCCliReqCtxModifier(t *testing.T) {
+	conf := map[string]interface{}{
+		common.AUTHMode:           common.OIDCAuth,
+		common.OIDCName:           "test",
+		common.OIDCEndpoint:       "https://accounts.google.com",
+		common.OIDCSkipCertVerify: "false",
+		common.OIDCScope:          "openid, profile, offline_access",
+		common.OIDCCLientID:       "client",
+		common.OIDCClientSecret:   "secret",
+		common.ExtEndpoint:        "https://harbor.test",
+	}
+
+	kp := &config2.PresetKeyProvider{Key: "naa4JtarA1Zsc3uY"}
+	config.InitWithSettings(conf, kp)
+
+	modifier := &oidcCliReqCtxModifier{}
+	req1, err := http.NewRequest(http.MethodGet,
+		"http://127.0.0.1/api/projects/", nil)
+	require.Nil(t, err)
+	ctx1, err := newContext(req1)
+	require.Nil(t, err)
+	assert.False(t, modifier.Modify(ctx1))
+	req2, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil)
+	require.Nil(t, err)
+	ctx2, err := newContext(req2)
+	require.Nil(t, err)
+	assert.False(t, modifier.Modify(ctx2))
+	username := "oidcModiferTester"
+	password := "oidcSecret"
+	u := &models.User{
+		Username: username,
+		Email:    "testtest@test.org",
+		Password: "12345678",
+	}
+	id, err := dao.Register(*u)
+	require.Nil(t, err)
+	oidc.SetHardcodeVerifierForTest(password)
+	req3, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil)
+	require.Nil(t, err)
+	req3.SetBasicAuth(username, password)
+	ctx3, err := newContext(req3)
+	assert.True(t, modifier.Modify(ctx3))
+	o := dao.GetOrmer()
+	_, err = o.Delete(&models.User{UserID: int(id)})
+	assert.Nil(t, err)
+}
+
 func TestRobotReqCtxModifier(t *testing.T) {
 	req, err := http.NewRequest(http.MethodGet,
 		"http://127.0.0.1/api/projects/", nil)
@@ -135,7 +185,7 @@ func TestRobotReqCtxModifier(t *testing.T) {
 	assert.False(t, modified)
 }
 
-func TestAutoProxyReqCtxModifier(t *testing.T) {
+func TestAuthProxyReqCtxModifier(t *testing.T) {
 
 	server, err := fiter_test.NewAuthProxyTestServer()
 	assert.Nil(t, err)