Update OIDC token refresh process

1) Disassociate id token from user session

2) Some OIDC providers do not return id_token in the response of refresh
request:
https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse
When validating the CLI secret it will not validate the id token,
instead it will check the expiration of the access token, and try to
refresh it.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This commit is contained in:
Daniel Jiang 2019-10-16 16:42:41 +08:00
parent 32a2c41c3b
commit f0cb16cb86
3 changed files with 12 additions and 22 deletions

View File

@ -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
}

View File

@ -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)

View File

@ -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
}