From f3fcc37244e256550d9fd0e34cbb87e6d6f4ae5f Mon Sep 17 00:00:00 2001
From: Daniel Jiang <jiangd@vmware.com>
Date: Wed, 25 Dec 2019 19:32:41 +0800
Subject: [PATCH] Onboard user when doing token review

This commit will make the "tokenreview" security filter onboard
user if the request carries a valid token.  If the "skipsearch" flag in
http_auth setting is set to false the onboard will fail.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
---
 src/core/filter/security.go      | 19 ++++++++++++++-----
 src/core/filter/security_test.go | 27 ++++-----------------------
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/src/core/filter/security.go b/src/core/filter/security.go
index 3572133e8..9e0a11b53 100644
--- a/src/core/filter/security.go
+++ b/src/core/filter/security.go
@@ -318,17 +318,26 @@ func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
 		log.Errorf("user name doesn't match with token: %s", rawUserName)
 		return false
 	}
-
 	user, err := dao.GetUser(models.User{
 		Username: rawUserName,
 	})
 	if err != nil {
-		log.Errorf("fail to get user: %v", err)
+		log.Errorf("fail to get user: %s, error: %v", rawUserName, err)
 		return false
 	}
-	if user == nil {
-		log.Errorf("User: %s has not been on boarded yet.", rawUserName)
-		return false
+	if user == nil { // onboard user if it's not yet onboarded.
+		uid, err := auth.SearchAndOnBoardUser(rawUserName)
+		if err != nil {
+			log.Errorf("Failed to search and onboard user, username: %s, error: %v", rawUserName, err)
+			return false
+		}
+		user, err = dao.GetUser(models.User{
+			UserID: uid,
+		})
+		if err != nil {
+			log.Errorf("Fail to get user, name: %s, ID: %d, error: %v", rawUserName, uid, err)
+			return false
+		}
 	}
 	u2, err := authproxy.UserFromReviewStatus(tokenReviewStatus)
 	if err != nil {
diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go
index 5c23dd7ec..1f481e195 100644
--- a/src/core/filter/security_test.go
+++ b/src/core/filter/security_test.go
@@ -39,6 +39,7 @@ import (
 	"github.com/goharbor/harbor/src/common/security/local"
 	"github.com/goharbor/harbor/src/common/security/secret"
 	"github.com/goharbor/harbor/src/common/utils/test"
+	_ "github.com/goharbor/harbor/src/core/auth/authproxy"
 	_ "github.com/goharbor/harbor/src/core/auth/db"
 	_ "github.com/goharbor/harbor/src/core/auth/ldap"
 	"github.com/goharbor/harbor/src/core/config"
@@ -270,33 +271,13 @@ func TestAuthProxyReqCtxModifier(t *testing.T) {
 	addToReqContext(req, AuthModeKey, common.HTTPAuth)
 	ctx, err := newContext(req)
 	if err != nil {
-		t.Fatalf("failed to crate context: %v", err)
+		t.Fatalf("failed to create context: %v", err)
 	}
 
 	modifier := &authProxyReqCtxModifier{}
 	modified := modifier.Modify(ctx)
-	assert.False(t, modified)
-
-	// Onboard
-	err = dao.OnBoardUser(&models.User{
-		Username: "administrator@vsphere.local",
-	})
-	assert.Nil(t, err)
-	req, err = http.NewRequest(http.MethodGet,
-		"http://127.0.0.1/service/token", nil)
-	if err != nil {
-		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)
-	}
-
-	modifier = &authProxyReqCtxModifier{}
-	modified = modifier.Modify(ctx)
 	assert.True(t, modified)
+
 }
 
 func TestBasicAuthReqCtxModifier(t *testing.T) {
@@ -347,7 +328,7 @@ func TestSessionReqCtxModifier(t *testing.T) {
 	addToReqContext(req, AuthModeKey, common.DBAuth)
 	ctx, err := newContext(req)
 	if err != nil {
-		t.Fatalf("failed to crate context: %v", err)
+		t.Fatalf("failed to create context: %v", err)
 	}
 
 	modifier := &sessionReqCtxModifier{}