Merge pull request #7392 from reasonerjt/oidc-logout

Handle OIDC user invalidation from OIDC provider.
This commit is contained in:
Wenkai Yin 2019-04-19 12:46:36 +08:00 committed by GitHub
commit 059b75e97c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 101 additions and 58 deletions

View File

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

View File

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

View File

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

View File

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

View File

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