diff --git a/src/common/utils/oidc/helper.go b/src/common/utils/oidc/helper.go index 2d59e09ed..7de72a8b8 100644 --- a/src/common/utils/oidc/helper.go +++ b/src/common/utils/oidc/helper.go @@ -106,7 +106,7 @@ var insecureTransport = &http.Transport{ // Token wraps the attributes of a oauth2 token plus the attribute of ID token type Token struct { oauth2.Token - IDToken string `json:"id_token"` + IDToken string `json:"id_token,omitempty"` } func getOauthConf() (*oauth2.Config, error) { @@ -200,9 +200,10 @@ func RefreshToken(ctx context.Context, token *Token) (*Token, error) { if err != nil { return nil, err } + it, ok := t.Extra("id_token").(string) if !ok { - return nil, fmt.Errorf("failed to get id_token from refresh response") + log.Debugf("id_token not exist in refresh response") } return &Token{Token: *t, IDToken: it}, nil } diff --git a/src/common/utils/oidc/secret.go b/src/common/utils/oidc/secret.go index 199311073..861dc0e8d 100644 --- a/src/common/utils/oidc/secret.go +++ b/src/common/utils/oidc/secret.go @@ -11,6 +11,7 @@ import ( "github.com/goharbor/harbor/src/core/config" "github.com/pkg/errors" "sync" + "time" ) // SecretVerifyError wraps the different errors happened when verifying a secret for OIDC user. When seeing this error, @@ -83,7 +84,10 @@ func (dm *defaultManager) VerifySecret(ctx context.Context, userID int, secret s return dm.VerifyToken(ctx, oidcUser) } -// VerifyToken verifies the token in the model from parm in this implementation it will try to refresh the token +// VerifyToken checks the expiration of the token in the model, (we'll only do expiration checks b/c according to spec, +// the response may not have ID token: +// https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse +// and it will try to refresh the token // if it's expired, if the refresh is successful it will persist the token and consider the verification successful. func (dm *defaultManager) VerifyToken(ctx context.Context, user *models.OIDCUser) error { if user == nil { @@ -103,11 +107,10 @@ func (dm *defaultManager) VerifyToken(ctx context.Context, user *models.OIDCUser return verifyError(err) } log.Debugf("Token string for verify: %s", tokenStr) - _, err = VerifyToken(ctx, token.IDToken) - if err == nil { + if token.Expiry.After(time.Now()) { return nil } - log.Infof("Failed to verify ID Token, error: %v, refreshing...", err) + log.Info("Token string has expired, refreshing...") t, err := RefreshToken(ctx, token) if err != nil { return verifyError(err) diff --git a/src/core/filter/security.go b/src/core/filter/security.go index b56429252..b8bbbac5b 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -455,26 +455,12 @@ func (s *sessionReqCtxModifier) Modify(ctx *beegoctx.Context) bool { log.Info("can not get user information from session") return false } - if ctx.Request.Context().Value(AuthModeKey).(string) == common.OIDCAuth { - ou, err := dao.GetOIDCUserByUserID(user.UserID) - if err != nil { - log.Errorf("Failed to get OIDC user info, error: %v", err) - return false - } - if ou != nil { // If user does not have OIDC metadata, it means he is not onboarded via OIDC authn, - // so we can skip checking the token. - if err := oidc.VerifyAndPersistToken(ctx.Request.Context(), ou); err != nil { - log.Errorf("Failed to verify token, error: %v", err) - return false - } - } - } log.Debug("using local database project manager") pm := config.GlobalProjectMgr log.Debug("creating local database security context...") - securCtx := local.NewSecurityContext(&user, pm) + securityCtx := local.NewSecurityContext(&user, pm) - setSecurCtxAndPM(ctx.Request, securCtx, pm) + setSecurCtxAndPM(ctx.Request, securityCtx, pm) return true }