Get rid of case-sensitivity in authproxy setting

This commit removes the attribute to control case-sensitivity from
authproxy setting.
The result in token review status will be used as the single source of
truth, regardless the case of the letters in group names and user names.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This commit is contained in:
Daniel Jiang 2019-12-04 15:56:40 +08:00
parent bd28ad1ae7
commit 8d3df218d9
11 changed files with 80 additions and 59 deletions

View File

@ -140,7 +140,6 @@ var (
{Name: common.HTTPAuthProxyVerifyCert, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}},
{Name: common.HTTPAuthProxySkipSearch, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}},
{Name: common.HTTPAuthProxyServerCertificate, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}},
{Name: common.HTTPAuthProxyCaseSensitive, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}},
{Name: common.OIDCName, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}},
{Name: common.OIDCEndpoint, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}},

View File

@ -105,7 +105,6 @@ const (
HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint"
HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert"
HTTPAuthProxySkipSearch = "http_authproxy_skip_search"
HTTPAuthProxyCaseSensitive = "http_authproxy_case_sensitive"
HTTPAuthProxyServerCertificate = "http_authproxy_server_certificate"
OIDCName = "oidc_name"
OIDCEndpoint = "oidc_endpoint"

View File

@ -74,7 +74,6 @@ type HTTPAuthProxy struct {
VerifyCert bool `json:"verify_cert"`
SkipSearch bool `json:"skip_search"`
ServerCertificate string `json:"server_certificate"`
CaseSensitive bool `json:"case_sensitive"`
}
// OIDCSetting wraps the settings for OIDC auth endpoint

View File

@ -126,7 +126,7 @@ func (c *ConfigAPI) validateCfg(cfgs map[string]interface{}) (bool, error) {
return true, err
}
if !flag {
if failedKeys := checkUnmodifiable(c.cfgManager, cfgs, common.AUTHMode, common.HTTPAuthProxyCaseSensitive); len(failedKeys) > 0 {
if failedKeys := checkUnmodifiable(c.cfgManager, cfgs, common.AUTHMode); len(failedKeys) > 0 {
return false, fmt.Errorf("the keys %v can not be modified as new users have been inserted into database", failedKeys)
}
}

View File

@ -53,12 +53,8 @@ type Auth struct {
Endpoint string
TokenReviewEndpoint string
SkipSearch bool
// When this attribute is set to false, the name of user/group will be converted to lower-case when onboarded to Harbor, so
// as long as the authentication is successful there's no difference in terms of upper or lower case that is used.
// It will be mapped to one entry in Harbor's User/Group table.
CaseSensitive bool
settingTimeStamp time.Time
client *http.Client
settingTimeStamp time.Time
client *http.Client
}
type session struct {
@ -83,26 +79,25 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
if err != nil {
return nil, err
}
data, err := ioutil.ReadAll(resp.Body)
if err != nil {
log.Warningf("Failed to read response body, error: %v", err)
return nil, auth.ErrAuth{}
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusOK {
name := a.normalizeName(m.Principal)
user := &models.User{Username: name}
data, err := ioutil.ReadAll(resp.Body)
if err != nil {
log.Warningf("Failed to read response body, error: %v", err)
return nil, auth.ErrAuth{}
}
s := session{}
err = json.Unmarshal(data, &s)
if err != nil {
return nil, auth.NewErrAuth(fmt.Sprintf("failed to read session %v", err))
}
if err := a.tokenReview(s.SessionID, user); err != nil {
return nil, auth.NewErrAuth(err.Error())
user, err := a.tokenReview(s.SessionID)
if err != nil {
return nil, auth.NewErrAuth(fmt.Sprintf("failed to do token review, error: %v", err))
}
return user, nil
} else if resp.StatusCode == http.StatusUnauthorized {
return nil, auth.ErrAuth{}
return nil, auth.NewErrAuth(string(data))
} else {
data, err := ioutil.ReadAll(resp.Body)
if err != nil {
@ -112,26 +107,24 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
}
}
func (a *Auth) tokenReview(sessionID string, user *models.User) error {
func (a *Auth) tokenReview(sessionID string) (*models.User, error) {
httpAuthProxySetting, err := config.HTTPAuthProxySetting()
if err != nil {
return err
return nil, err
}
reviewStatus, err := authproxy.TokenReview(sessionID, httpAuthProxySetting)
if err != nil {
return err
return nil, err
}
u2, err := authproxy.UserFromReviewStatus(reviewStatus)
u, err := authproxy.UserFromReviewStatus(reviewStatus)
if err != nil {
return err
return nil, err
}
user.GroupIDs = u2.GroupIDs
return nil
return u, nil
}
// OnBoardUser delegates to dao pkg to insert/update data in DB.
func (a *Auth) OnBoardUser(u *models.User) error {
u.Username = a.normalizeName(u.Username)
return dao.OnBoardUser(u)
}
@ -154,7 +147,6 @@ func (a *Auth) SearchUser(username string) (*models.User, error) {
if err != nil {
log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err)
}
username = a.normalizeName(username)
var u *models.User
if a.SkipSearch {
u = &models.User{Username: username}
@ -171,7 +163,6 @@ func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) {
if err != nil {
log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err)
}
groupKey = a.normalizeName(groupKey)
var ug *models.UserGroup
if a.SkipSearch {
ug = &models.UserGroup{
@ -190,7 +181,6 @@ func (a *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error {
return errors.New("Should provide a group name")
}
u.GroupType = common.HTTPGroupType
u.GroupName = a.normalizeName(u.GroupName)
err := group.OnBoardUserGroup(u)
if err != nil {
return err
@ -248,13 +238,6 @@ func getTLSConfig(setting *models.HTTPAuthProxy) (*tls.Config, error) {
return &tls.Config{InsecureSkipVerify: !setting.VerifyCert}, nil
}
func (a *Auth) normalizeName(n string) string {
if !a.CaseSensitive {
return strings.ToLower(n)
}
return n
}
func init() {
auth.Register(common.HTTPAuth, &Auth{})
}

View File

@ -44,13 +44,11 @@ func TestMain(m *testing.M) {
a = &Auth{
Endpoint: mockSvr.URL + "/test/login",
TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview",
CaseSensitive: false,
}
cfgMap := cut.GetUnitTestConfig()
conf := map[string]interface{}{
common.HTTPAuthProxyEndpoint: a.Endpoint,
common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint,
common.HTTPAuthProxyCaseSensitive: a.CaseSensitive,
common.HTTPAuthProxyVerifyCert: false,
common.PostGreSQLSSLMode: cfgMap[common.PostGreSQLSSLMode],
common.PostGreSQLUsername: cfgMap[common.PostGreSQLUsername],
@ -200,7 +198,7 @@ func TestAuth_OnBoardGroup(t *testing.T) {
assert.True(t, input.ID > 0, "The OnBoardGroup should have a valid group ID")
g, er := group.GetUserGroup(input.ID)
assert.Nil(t, er)
assert.Equal(t, "onboardtest", g.GroupName)
assert.Equal(t, "OnBoardTest", g.GroupName)
emptyGroup := &models.UserGroup{}
err := a.OnBoardGroup(emptyGroup, "")
@ -226,7 +224,6 @@ func TestGetTLSConfig(t *testing.T) {
VerifyCert: false,
SkipSearch: false,
ServerCertificate: "",
CaseSensitive: false,
},
expect: result{
hasError: false,
@ -241,7 +238,6 @@ func TestGetTLSConfig(t *testing.T) {
VerifyCert: false,
SkipSearch: false,
ServerCertificate: "This does not look like a cert",
CaseSensitive: false,
},
expect: result{
hasError: false,
@ -256,7 +252,6 @@ func TestGetTLSConfig(t *testing.T) {
VerifyCert: true,
SkipSearch: false,
ServerCertificate: "This does not look like a cert",
CaseSensitive: false,
},
expect: result{
hasError: true,
@ -269,7 +264,6 @@ func TestGetTLSConfig(t *testing.T) {
VerifyCert: true,
SkipSearch: false,
ServerCertificate: "",
CaseSensitive: false,
},
expect: result{
hasError: false,
@ -315,7 +309,6 @@ pkgODrJUf0p5dhcnLyA2nZolRV1rtwlgJstnEV4JpG1MwtmAZYZUilLvnfpVxTtA
y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
-----END CERTIFICATE-----
`,
CaseSensitive: false,
},
expect: result{
hasError: false,

View File

@ -15,14 +15,30 @@
package test
import (
"encoding/json"
"fmt"
"github.com/goharbor/harbor/src/common/utils"
"io/ioutil"
"k8s.io/api/authentication/v1beta1"
"net/http"
"net/http/httptest"
"strings"
)
type authHandler struct {
m map[string]string
type userEntry struct {
username string
password string
sessionID string
reviewStatus string
}
type authHandler struct {
entries []userEntry
m map[string]string
}
var reviewStatusTpl = `{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "%s", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`
// ServeHTTP handles HTTP requests
func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
if req.Method != http.MethodPost {
@ -34,27 +50,64 @@ func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
} else if pass, ok := ah.m[u]; !ok || pass != p {
http.Error(rw, "", http.StatusUnauthorized)
} else {
_, e := rw.Write([]byte(`{"session_id": "hgx59wuWI3b0jcbtidv5mU1YCp-DOQ9NKR1iYKACdKCvbVn7"}`))
if e != nil {
panic(e)
for _, e := range ah.entries {
if e.username == strings.ToLower(u) {
_, err := rw.Write([]byte(fmt.Sprintf(`{"session_id": "%s"}`, e.sessionID)))
if err != nil {
panic(err)
} else {
return
}
}
}
http.Error(rw, fmt.Sprintf("Do not find entry in entrylist, username: %s", u), http.StatusUnauthorized)
}
}
type reviewTokenHandler struct {
entries []userEntry
}
func (rth *reviewTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
if req.Method != http.MethodPost {
http.Error(rw, "", http.StatusMethodNotAllowed)
}
rw.Write([]byte(`{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "administrator@vsphere.local", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`))
bodyBytes, err := ioutil.ReadAll(req.Body)
if err != nil {
http.Error(rw, fmt.Sprintf("failed to read request body, error: %v", err), http.StatusBadRequest)
}
reviewData := &v1beta1.TokenReview{}
if err := json.Unmarshal(bodyBytes, reviewData); err != nil {
http.Error(rw, fmt.Sprintf("failed to decode request body, error: %v", err), http.StatusBadRequest)
}
defer req.Body.Close()
for _, e := range rth.entries {
if reviewData.Spec.Token == e.sessionID {
_, err := rw.Write([]byte(fmt.Sprintf(reviewStatusTpl, e.username)))
if err != nil {
panic(err)
} else {
return
}
}
}
http.Error(rw, fmt.Sprintf("failed to match token: %s, entrylist: %+v", reviewData.Spec.Token, rth.entries), http.StatusUnauthorized)
}
// NewMockServer creates the mock server for testing
func NewMockServer(creds map[string]string) *httptest.Server {
mux := http.NewServeMux()
mux.Handle("/test/login", &authHandler{m: creds})
mux.Handle("/test/tokenreview", &reviewTokenHandler{})
entryList := []userEntry{}
for user, pwd := range creds {
e := userEntry{
username: strings.ToLower(user),
password: pwd,
sessionID: utils.GenerateRandomString(),
reviewStatus: fmt.Sprintf(reviewStatusTpl, user),
}
entryList = append(entryList, e)
}
mux.Handle("/test/login", &authHandler{m: creds, entries: entryList})
mux.Handle("/test/tokenreview", &reviewTokenHandler{entries: entryList})
return httptest.NewTLSServer(mux)
}

View File

@ -476,7 +476,6 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) {
VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(),
SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(),
ServerCertificate: cfgMgr.Get(common.HTTPAuthProxyServerCertificate).GetString(),
CaseSensitive: cfgMgr.Get(common.HTTPAuthProxyCaseSensitive).GetBool(),
}, nil
}

View File

@ -251,7 +251,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
m := map[string]interface{}{
common.HTTPAuthProxySkipSearch: "true",
common.HTTPAuthProxyVerifyCert: "true",
common.HTTPAuthProxyCaseSensitive: "false",
common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix",
common.HTTPAuthProxyServerCertificate: certificate,
}
@ -262,7 +261,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
Endpoint: "https://auth.proxy/suffix",
SkipSearch: true,
VerifyCert: true,
CaseSensitive: false,
ServerCertificate: certificate,
})
}

View File

@ -257,7 +257,6 @@ func TestAuthProxyReqCtxModifier(t *testing.T) {
Endpoint: "https://auth.proxy/suffix",
SkipSearch: true,
VerifyCert: false,
CaseSensitive: true,
TokenReviewEndpoint: server.URL,
})

View File

@ -132,7 +132,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
VerifyCert: false,
SkipSearch: false,
ServerCertificate: "",
CaseSensitive: false,
},
expect: rest.TLSClientConfig{
Insecure: true,