mirror of
https://github.com/goharbor/harbor.git
synced 2025-03-02 10:41:59 +01:00
Handle OIDC user invalidation from OIDC provider.
Ths commmit ensures that when user's token is invalidated OIDC provider, he cannot access protected resource in Harbor with the user info in his session. We share the code path with secret verification b/c the refresh token can be used only once, so it has to be stored in one place. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This commit is contained in:
parent
2eed72c445
commit
239b33c5fb
@ -113,7 +113,7 @@ var insecureTransport = &http.Transport{
|
||||
|
||||
// Token wraps the attributes of a oauth2 token plus the attribute of ID token
|
||||
type Token struct {
|
||||
*oauth2.Token
|
||||
oauth2.Token
|
||||
IDToken string `json:"id_token"`
|
||||
}
|
||||
|
||||
@ -167,7 +167,7 @@ func ExchangeToken(ctx context.Context, code string) (*Token, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &Token{Token: oauthToken, IDToken: oauthToken.Extra("id_token").(string)}, nil
|
||||
return &Token{Token: *oauthToken, IDToken: oauthToken.Extra("id_token").(string)}, nil
|
||||
}
|
||||
|
||||
// VerifyToken verifies the ID token based on the OIDC settings
|
||||
@ -203,10 +203,10 @@ func RefreshToken(ctx context.Context, token *Token) (*Token, error) {
|
||||
}
|
||||
setting := provider.setting.Load().(models.OIDCSetting)
|
||||
ctx = clientCtx(ctx, setting.VerifyCert)
|
||||
ts := oauth.TokenSource(ctx, token.Token)
|
||||
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
|
||||
return &Token{Token: *t, IDToken: t.Extra("id_token").(string)}, nil
|
||||
}
|
||||
|
@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"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/core/config"
|
||||
@ -28,12 +29,12 @@ func verifyError(err error) error {
|
||||
|
||||
// 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
|
||||
// VerifyToken verifies the token in the model from parm,
|
||||
// and refreshes the token in the DB if it's refreshed during the verification.
|
||||
VerifyToken(ctx context.Context, user *models.OIDCUser) error
|
||||
}
|
||||
|
||||
type defaultManager struct {
|
||||
@ -58,25 +59,6 @@ func (dm *defaultManager) getEncryptKey() (string, error) {
|
||||
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 {
|
||||
@ -98,7 +80,20 @@ func (dm *defaultManager) VerifySecret(ctx context.Context, userID int, secret s
|
||||
if secret != plainSecret {
|
||||
return verifyError(errors.New("secret mismatch"))
|
||||
}
|
||||
tokenStr, err := utils.ReversibleDecrypt(oidcUser.Token, key)
|
||||
return dm.VerifyToken(ctx, oidcUser)
|
||||
}
|
||||
|
||||
// VerifyToken verifies the token in the model from parm in this implementation 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 {
|
||||
return verifyError(fmt.Errorf("input user is nil"))
|
||||
}
|
||||
key, err := dm.getEncryptKey()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to load the key for encryption/decryption: %v", err)
|
||||
}
|
||||
tokenStr, err := utils.ReversibleDecrypt(user.Token, key)
|
||||
if err != nil {
|
||||
return verifyError(err)
|
||||
}
|
||||
@ -116,15 +111,25 @@ func (dm *defaultManager) VerifySecret(ctx context.Context, userID int, secret s
|
||||
if err != nil {
|
||||
return verifyError(err)
|
||||
}
|
||||
err = dm.SetSecret(oidcUser.UserID, secret, t)
|
||||
tb, err := json.Marshal(t)
|
||||
if err != nil {
|
||||
log.Warningf("Failed to encode the refreshed token, error: %v", err)
|
||||
}
|
||||
encToken, _ := utils.ReversibleEncrypt(string(tb), key)
|
||||
user.Token = encToken
|
||||
err = dao.UpdateOIDCUser(user)
|
||||
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
|
||||
// VerifySecret calls the manager to verify the secret.
|
||||
func VerifySecret(ctx context.Context, userID int, secret string) error {
|
||||
return m.VerifySecret(ctx, userID, secret)
|
||||
}
|
||||
|
||||
// VerifyAndPersistToken calls the manager to verify token and persist it if it's refreshed.
|
||||
func VerifyAndPersistToken(ctx context.Context, user *models.OIDCUser) error {
|
||||
return m.VerifyToken(ctx, user)
|
||||
}
|
||||
|
@ -1,6 +1,9 @@
|
||||
package oidc
|
||||
|
||||
import "context"
|
||||
import (
|
||||
"context"
|
||||
"github.com/goharbor/harbor/src/common/models"
|
||||
)
|
||||
import "errors"
|
||||
|
||||
// This is for testing only
|
||||
@ -8,10 +11,6 @@ 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"))
|
||||
@ -19,6 +18,10 @@ func (fv *fakeVerifier) VerifySecret(ctx context.Context, userID int, secret str
|
||||
return nil
|
||||
}
|
||||
|
||||
func (fv *fakeVerifier) VerifyToken(ctx context.Context, u *models.OIDCUser) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetHardcodeVerifierForTest overwrite the default secret manager for testing.
|
||||
// Be reminded this is for testing only.
|
||||
func SetHardcodeVerifierForTest(s string) {
|
||||
|
@ -66,6 +66,8 @@ const (
|
||||
|
||||
// PmKey is context value key for the project manager
|
||||
PmKey ContextValueKey = "harbor_project_manager"
|
||||
// AuthModeKey is context key for auth mode
|
||||
AuthModeKey ContextValueKey = "harbor_auth_mode"
|
||||
)
|
||||
|
||||
var (
|
||||
@ -110,6 +112,7 @@ func Init() {
|
||||
|
||||
// standalone
|
||||
reqCtxModifiers = []ReqCtxModifier{
|
||||
&configCtxModifier{},
|
||||
&secretReqCtxModifier{config.SecretStore},
|
||||
&oidcCliReqCtxModifier{},
|
||||
&authProxyReqCtxModifier{},
|
||||
@ -144,6 +147,20 @@ type ReqCtxModifier interface {
|
||||
Modify(*beegoctx.Context) bool
|
||||
}
|
||||
|
||||
// configCtxModifier populates to the configuration values to context, which are to be read by subsequent
|
||||
// filters.
|
||||
type configCtxModifier struct {
|
||||
}
|
||||
|
||||
func (c *configCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
m, err := config.AuthMode()
|
||||
if err != nil {
|
||||
log.Warningf("Failed to get auth mode, err: %v", err)
|
||||
}
|
||||
addToReqContext(ctx.Request, AuthModeKey, m)
|
||||
return false
|
||||
}
|
||||
|
||||
type secretReqCtxModifier struct {
|
||||
store *secstore.Store
|
||||
}
|
||||
@ -215,12 +232,7 @@ func (oc *oidcCliReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
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 {
|
||||
if ctx.Request.Context().Value(AuthModeKey).(string) != common.OIDCAuth {
|
||||
return false
|
||||
}
|
||||
username, secret, ok := ctx.Request.BasicAuth()
|
||||
@ -251,12 +263,7 @@ func (oc *oidcCliReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
type authProxyReqCtxModifier struct{}
|
||||
|
||||
func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
authMode, err := config.AuthMode()
|
||||
if err != nil {
|
||||
log.Errorf("fail to get auth mode, %v", err)
|
||||
return false
|
||||
}
|
||||
if authMode != common.HTTPAuth {
|
||||
if ctx.Request.Context().Value(AuthModeKey).(string) != common.HTTPAuth {
|
||||
return false
|
||||
}
|
||||
|
||||
@ -443,20 +450,28 @@ func (b *basicAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
type sessionReqCtxModifier struct{}
|
||||
|
||||
func (s *sessionReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
|
||||
var user models.User
|
||||
userInterface := ctx.Input.Session("user")
|
||||
|
||||
if userInterface == nil {
|
||||
log.Debug("can not get user information from session")
|
||||
return false
|
||||
}
|
||||
|
||||
log.Debug("got user information from session")
|
||||
user, ok := userInterface.(models.User)
|
||||
if !ok {
|
||||
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 err := oidc.VerifyAndPersistToken(ctx.Request.Context(), ou); err != nil {
|
||||
log.Errorf("Failed to verify secret, error: %v", err)
|
||||
return false
|
||||
}
|
||||
}
|
||||
log.Debug("using local database project manager")
|
||||
pm := config.GlobalProjectMgr
|
||||
log.Debug("creating local database security context...")
|
||||
|
@ -101,6 +101,28 @@ func TestSecurityFilter(t *testing.T) {
|
||||
assert.NotNil(t, projectManager(ctx))
|
||||
}
|
||||
|
||||
func TestConfigCtxModifier(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodGet,
|
||||
"http://127.0.0.1/api/projects/", nil)
|
||||
require.Nil(t, err)
|
||||
conf := map[string]interface{}{
|
||||
common.AUTHMode: common.OIDCAuth,
|
||||
common.OIDCName: "test",
|
||||
common.OIDCEndpoint: "https://accounts.google.com",
|
||||
common.OIDCVerifyCert: "true",
|
||||
common.OIDCScope: "openid, profile, offline_access",
|
||||
common.OIDCCLientID: "client",
|
||||
common.OIDCClientSecret: "secret",
|
||||
common.ExtEndpoint: "https://harbor.test",
|
||||
}
|
||||
config.InitWithSettings(conf)
|
||||
ctx, err := newContext(req)
|
||||
m := &configCtxModifier{}
|
||||
f := m.Modify(ctx)
|
||||
assert.False(t, f)
|
||||
assert.Equal(t, common.OIDCAuth, req.Context().Value(AuthModeKey).(string))
|
||||
}
|
||||
|
||||
func TestSecretReqCtxModifier(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodGet,
|
||||
"http://127.0.0.1/api/projects/", nil)
|
||||
@ -145,6 +167,7 @@ func TestOIDCCliReqCtxModifier(t *testing.T) {
|
||||
assert.False(t, modifier.Modify(ctx1))
|
||||
req2, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil)
|
||||
require.Nil(t, err)
|
||||
addToReqContext(req2, AuthModeKey, common.OIDCAuth)
|
||||
ctx2, err := newContext(req2)
|
||||
require.Nil(t, err)
|
||||
assert.False(t, modifier.Modify(ctx2))
|
||||
@ -161,6 +184,7 @@ func TestOIDCCliReqCtxModifier(t *testing.T) {
|
||||
req3, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/service/token", nil)
|
||||
require.Nil(t, err)
|
||||
req3.SetBasicAuth(username, password)
|
||||
addToReqContext(req3, AuthModeKey, common.OIDCAuth)
|
||||
ctx3, err := newContext(req3)
|
||||
assert.True(t, modifier.Modify(ctx3))
|
||||
o := dao.GetOrmer()
|
||||
@ -216,6 +240,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) {
|
||||
t.Fatalf("failed to create request: %v", req)
|
||||
}
|
||||
req.SetBasicAuth("tokenreview$administrator@vsphere.local", "reviEwt0k3n")
|
||||
addToReqContext(req, AuthModeKey, common.HTTPAuth)
|
||||
ctx, err := newContext(req)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to crate context: %v", err)
|
||||
@ -236,6 +261,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) {
|
||||
t.Fatalf("failed to create request: %v", req)
|
||||
}
|
||||
req.SetBasicAuth("tokenreview$administrator@vsphere.local", "reviEwt0k3n")
|
||||
addToReqContext(req, AuthModeKey, common.HTTPAuth)
|
||||
ctx, err = newContext(req)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to crate context: %v", err)
|
||||
@ -290,13 +316,8 @@ func TestSessionReqCtxModifier(t *testing.T) {
|
||||
t.Fatalf("failed to set session: %v", err)
|
||||
}
|
||||
|
||||
req, err = http.NewRequest(http.MethodGet,
|
||||
"http://127.0.0.1/api/projects/", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create request: %v", req)
|
||||
}
|
||||
addSessionIDToCookie(req, store.SessionID())
|
||||
|
||||
addToReqContext(req, AuthModeKey, common.DBAuth)
|
||||
ctx, err := newContext(req)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to crate context: %v", err)
|
||||
@ -336,12 +357,11 @@ func TestSessionReqCtxModifierFailed(t *testing.T) {
|
||||
t.Fatalf("failed to create request: %v", req)
|
||||
}
|
||||
addSessionIDToCookie(req, store.SessionID())
|
||||
|
||||
addToReqContext(req, AuthModeKey, common.DBAuth)
|
||||
ctx, err := newContext(req)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to crate context: %v", err)
|
||||
}
|
||||
|
||||
modifier := &sessionReqCtxModifier{}
|
||||
modified := modifier.Modify(ctx)
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user