Merge pull request #4043 from reasonerjt/uaa-bugfix

Read Email from UAA while onboarding user.

src/common/utils/uaa/client.go

@@ -41,6 +41,8 @@ const (
 	UsersURLSuffix = "/Users"
 )
 
+var uaaTransport = &http.Transport{}
+
 // Client provides funcs to interact with UAA.
 type Client interface {
 	//PasswordAuth accepts username and password, return a token if it's valid.
@@ -49,6 +51,8 @@ type Client interface {
 	GetUserInfo(token string) (*UserInfo, error)
 	//SearchUser searches a user based on user name.
 	SearchUser(name string) ([]*SearchUserEntry, error)
+	//UpdateConfig updates the config of the current client
+	UpdateConfig(cfg *ClientConfig) error
 }
 
 // ClientConfig values to initialize UAA Client
@@ -169,13 +173,13 @@ func (dc *defaultClient) prepareCtx() context.Context {
 	return context.WithValue(context.Background(), oauth2.HTTPClient, dc.httpClient)
 }
 
-// NewDefaultClient creates an instance of defaultClient.
-func NewDefaultClient(cfg *ClientConfig) (Client, error) {
+func (dc *defaultClient) UpdateConfig(cfg *ClientConfig) error {
 	url := cfg.Endpoint
 	if !strings.Contains(url, "://") {
 		url = "https://" + url
 	}
 	url = strings.TrimSuffix(url, "/")
+	dc.endpoint = url
 	tc := &tls.Config{
 		InsecureSkipVerify: cfg.SkipTLSVerify,
 	}
@@ -183,7 +187,7 @@ func NewDefaultClient(cfg *ClientConfig) (Client, error) {
 		if _, err := os.Stat(cfg.CARootPath); !os.IsNotExist(err) {
 			content, err := ioutil.ReadFile(cfg.CARootPath)
 			if err != nil {
-				return nil, err
+				return err
 			}
 			pool := x509.NewCertPool()
 			//Do not throw error if the certificate is malformed, so we can put a place holder.
@@ -196,11 +200,9 @@ func NewDefaultClient(cfg *ClientConfig) (Client, error) {
 			log.Warningf("The root certificate file %s is not found, skip configuring root cert in UAA client.", cfg.CARootPath)
 		}
 	}
-	hc := &http.Client{
-		Transport: &http.Transport{
-			TLSClientConfig: tc,
-		},
-	}
+	uaaTransport.TLSClientConfig = tc
+	dc.httpClient.Transport = uaaTransport
+	//dc.httpClient.Transport = transport.
 
 	oc := &oauth2.Config{
 		ClientID:     cfg.ClientID,
@@ -216,11 +218,17 @@ func NewDefaultClient(cfg *ClientConfig) (Client, error) {
 		ClientSecret: cfg.ClientSecret,
 		TokenURL:     url + TokenURLSuffix,
 	}
-
-	return &defaultClient{
-		httpClient: hc,
-		oauth2Cfg:  oc,
-		twoLegCfg:  cc,
-		endpoint:   url,
-	}, nil
+	dc.oauth2Cfg = oc
+	dc.twoLegCfg = cc
+	return nil
+}
+
+// NewDefaultClient creates an instance of defaultClient.
+func NewDefaultClient(cfg *ClientConfig) (Client, error) {
+	hc := &http.Client{}
+	c := &defaultClient{httpClient: hc}
+	if err := c.UpdateConfig(cfg); err != nil {
+		return nil, err
+	}
+	return c, nil
 }

src/common/utils/uaa/fake_client.go

@@ -19,6 +19,8 @@ import (
 	"golang.org/x/oauth2"
 )
 
+const fakeToken = "The Fake Token"
+
 // FakeClient is for test only
 type FakeClient struct {
 	Username string
@@ -28,14 +30,26 @@ type FakeClient struct {
 // PasswordAuth ...
 func (fc *FakeClient) PasswordAuth(username, password string) (*oauth2.Token, error) {
 	if username == fc.Username && password == fc.Password {
-		return &oauth2.Token{}, nil
+		return &oauth2.Token{AccessToken: fakeToken}, nil
 	}
 	return nil, fmt.Errorf("Invalide username and password")
 }
 
 // GetUserInfo ...
 func (fc *FakeClient) GetUserInfo(token string) (*UserInfo, error) {
-	return nil, nil
+	if token != fakeToken {
+		return nil, fmt.Errorf("Unexpected token: %s, expected: %s", token, fakeToken)
+	}
+	info := &UserInfo{
+		Name:  "fakeName",
+		Email: "fake@fake.com",
+	}
+	return info, nil
+}
+
+// UpdateConfig ...
+func (fc *FakeClient) UpdateConfig(cfg *ClientConfig) error {
+	return nil
 }
 
 // SearchUser ...

src/ui/auth/authenticator.go

@@ -110,7 +110,7 @@ func Login(m models.AuthModel) (*models.User, error) {
 		time.Sleep(frozenTime)
 	}
 
-	authenticator.PostAuthenticate(user)
+	err = authenticator.PostAuthenticate(user)
 
 	return user, err
 }

src/ui/auth/uaa/uaa.go

@@ -23,27 +23,12 @@ import (
 	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/common/utils/log"
 	"github.com/vmware/harbor/src/common/utils/uaa"
 	"github.com/vmware/harbor/src/ui/auth"
 	"github.com/vmware/harbor/src/ui/config"
 )
 
-//CreateClient create a UAA Client instance based on system configuration.
-func CreateClient() (uaa.Client, error) {
-	UAASettings, err := config.UAASettings()
-	if err != nil {
-		return nil, err
-	}
-	cfg := &uaa.ClientConfig{
-		ClientID:      UAASettings.ClientID,
-		ClientSecret:  UAASettings.ClientSecret,
-		Endpoint:      UAASettings.Endpoint,
-		SkipTLSVerify: !UAASettings.VerifyCert,
-		CARootPath:    os.Getenv("UAA_CA_ROOT"),
-	}
-	return uaa.NewDefaultClient(cfg)
-}
-
 // Auth is the implementation of AuthenticateHelper to access uaa for authentication.
 type Auth struct {
 	sync.Mutex
@@ -58,12 +43,17 @@ func (u *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	}
 	t, err := u.client.PasswordAuth(m.Principal, m.Password)
 	if t != nil && err == nil {
-		//TODO: See if it's possible to get more information from token.
 		user := &models.User{
 			Username: m.Principal,
 		}
-		err = u.OnBoardUser(user)
-		return user, err
+		info, err2 := u.client.GetUserInfo(t.AccessToken)
+		if err2 != nil {
+			log.Warningf("Failed to extract user info from UAA, error: %v", err2)
+		} else {
+			user.Email = info.Email
+			user.Realname = info.Name
+		}
+		return user, nil
 	}
 	return nil, err
 }
@@ -89,6 +79,28 @@ func (u *Auth) OnBoardUser(user *models.User) error {
 	return dao.OnBoardUser(user)
 }
 
+// PostAuthenticate will check if user exists in DB, if not on Board user, if he does, update the profile.
+func (u *Auth) PostAuthenticate(user *models.User) error {
+	dbUser, err := dao.GetUser(models.User{Username: user.Username})
+	if err != nil {
+		return err
+	}
+	if dbUser == nil {
+		return u.OnBoardUser(user)
+	}
+	if user.Email != "" {
+		dbUser.Email = user.Email
+	}
+	if user.Realname != "" {
+		dbUser.Realname = user.Realname
+	}
+	if err2 := dao.ChangeUserProfile(*user, "Email", "Realname"); err2 != nil {
+		log.Warningf("Failed to update user profile, user: %s, error: %v", user.Username, err2)
+	}
+
+	return nil
+}
+
 // SearchUser search user on uaa server, transform it to Harbor's user model
 func (u *Auth) SearchUser(username string) (*models.User, error) {
 	if err := u.ensureClient(); err != nil {
@@ -116,13 +128,27 @@ func (u *Auth) SearchUser(username string) (*models.User, error) {
 }
 
 func (u *Auth) ensureClient() error {
-	if u.client != nil {
-		return nil
+	var cfg *uaa.ClientConfig
+	UAASettings, err := config.UAASettings()
+	//	log.Debugf("Uaa settings: %+v", UAASettings)
+	if err != nil {
+		log.Warningf("Failed to get UAA setting from Admin Server, error: %v", err)
+	} else {
+		cfg = &uaa.ClientConfig{
+			ClientID:      UAASettings.ClientID,
+			ClientSecret:  UAASettings.ClientSecret,
+			Endpoint:      UAASettings.Endpoint,
+			SkipTLSVerify: !UAASettings.VerifyCert,
+			CARootPath:    os.Getenv("UAA_CA_ROOT"),
+		}
+	}
+	if u.client != nil && cfg != nil {
+		return u.client.UpdateConfig(cfg)
 	}
 	u.Lock()
 	defer u.Unlock()
 	if u.client == nil {
-		c, err := CreateClient()
+		c, err := uaa.NewDefaultClient(cfg)
 		if err != nil {
 			return err
 		}

src/ui/auth/uaa/uaa_test.go

@@ -102,11 +102,12 @@ func TestMain(m *testing.M) {
 	os.Exit(rc)
 }
 
-func TestCreateClient(t *testing.T) {
+func TestEnsureClient(t *testing.T) {
 	assert := assert.New(t)
-	c, err := CreateClient()
+	auth := Auth{client: nil}
+	err := auth.ensureClient()
 	assert.Nil(err)
-	assert.NotNil(c)
+	assert.NotNil(auth.client)
 }
 
 func TestAuthenticate(t *testing.T) {
@@ -123,6 +124,7 @@ func TestAuthenticate(t *testing.T) {
 	u1, err1 := auth.Authenticate(m1)
 	assert.Nil(err1)
 	assert.NotNil(u1)
+	assert.Equal("fake@fake.com", u1.Email)
 	m2 := models.AuthModel{
 		Principal: "wrong",
 		Password:  "wrong",
@@ -153,6 +155,30 @@ func TestOnBoardUser(t *testing.T) {
 	assert.Equal("test", user.Realname)
 	assert.Equal("test", user.Username)
 	assert.Equal("test@uaa.placeholder", user.Email)
+	err3 := dao.ClearTable(models.UserTable)
+	assert.Nil(err3)
+}
+
+func TestPostAuthenticate(t *testing.T) {
+	assert := assert.New(t)
+	auth := Auth{}
+	um := &models.User{
+		Username: "test",
+	}
+	err := auth.PostAuthenticate(um)
+	assert.Nil(err)
+	user, _ := dao.GetUser(models.User{Username: "test"})
+	assert.Equal("test@uaa.placeholder", user.Email)
+	um.Email = "newEmail@new.com"
+	um.Realname = "newName"
+	err2 := auth.PostAuthenticate(um)
+	assert.Nil(err2)
+	user2, _ := dao.GetUser(models.User{Username: "test"})
+	assert.Equal("newEmail@new.com", user2.Email)
+	assert.Equal("newName", user2.Realname)
+	err3 := dao.ClearTable(models.UserTable)
+	assert.Nil(err3)
+
 }
 
 func TestSearchUser(t *testing.T) {