Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read extended user attributes from auth proxy #7547

Merged
merged 1 commit into from Feb 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 37 additions & 16 deletions pkg/auth/authenticator/request/headerrequest/requestheader.go
Expand Up @@ -11,13 +11,19 @@ import (
)

type Config struct {
// UserNameHeaders lists the headers to check (in order, case-insensitively) for a username. The first header with a value wins.
UserNameHeaders []string
// IDHeaders lists the headers to check (in order, case-insensitively) for an identity. The first header with a value wins.
IDHeaders []string
// NameHeaders lists the headers to check (in order, case-insensitively) for a display name. The first header with a value wins.
NameHeaders []string
// PreferredUsernameHeaders lists the headers to check (in order, case-insensitively) for a preferred username. The first header with a value wins. If empty, the ID is used
PreferredUsernameHeaders []string
// EmailHeaders lists the headers to check (in order, case-insensitively) for an email address. The first header with a value wins.
EmailHeaders []string
}

func NewDefaultConfig() *Config {
return &Config{
UserNameHeaders: []string{"X-Remote-User"},
IDHeaders: []string{"X-Remote-User", "Remote-User"},
}
}

Expand All @@ -32,22 +38,23 @@ func NewAuthenticator(providerName string, config *Config, mapper authapi.UserId
}

func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
username := ""
for _, header := range a.config.UserNameHeaders {
header = strings.TrimSpace(header)
if len(header) == 0 {
continue
}
username = req.Header.Get(header)
if len(username) != 0 {
break
}
}
if len(username) == 0 {
id := headerValue(req.Header, a.config.IDHeaders)
if len(id) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that this is incorrect, but since this is now a function, wouldn't the logic be better if we had it return a boolean success or failure, rather than running a len() on the return? I'd think that would be less error-prone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I had that bool, I'd still do the length check, because we can't successfully provision an identity with an empty id. when we have a case where we care about a present-but-empty value, it'd be fine to add then

return nil, false, nil
}

identity := authapi.NewDefaultUserIdentityInfo(a.providerName, username)
identity := authapi.NewDefaultUserIdentityInfo(a.providerName, id)

if email := headerValue(req.Header, a.config.EmailHeaders); len(email) > 0 {
identity.Extra[authapi.IdentityEmailKey] = email
}
if name := headerValue(req.Header, a.config.NameHeaders); len(name) > 0 {
identity.Extra[authapi.IdentityDisplayNameKey] = name
}
if preferredUsername := headerValue(req.Header, a.config.PreferredUsernameHeaders); len(preferredUsername) > 0 {
identity.Extra[authapi.IdentityPreferredUsernameKey] = preferredUsername
}

user, err := a.mapper.UserFor(identity)
if err != nil {
glog.V(4).Infof("Error creating or updating mapping for: %#v due to %v", identity, err)
Expand All @@ -57,3 +64,17 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,

return user, true, nil
}

func headerValue(h http.Header, headerNames []string) string {
for _, headerName := range headerNames {
headerName = strings.TrimSpace(headerName)
if len(headerName) == 0 {
continue
}
headerValue := h.Get(headerName)
if len(headerValue) > 0 {
return headerValue
}
}
return ""
}
75 changes: 58 additions & 17 deletions pkg/auth/authenticator/request/headerrequest/requestheader_test.go
Expand Up @@ -2,66 +2,99 @@ package headerrequest

import (
"net/http"
"reflect"
"testing"

"github.com/openshift/origin/pkg/auth/api"
"k8s.io/kubernetes/pkg/auth/user"
)

type TestUserIdentityMapper struct{}
type TestUserIdentityMapper struct {
Identity api.UserIdentityInfo
}

func (m *TestUserIdentityMapper) UserFor(identityInfo api.UserIdentityInfo) (user.Info, error) {
return &user.DefaultInfo{Name: identityInfo.GetProviderUserName()}, nil
m.Identity = identityInfo
username := identityInfo.GetProviderUserName()
if preferredUsername := identityInfo.GetExtra()[api.IdentityPreferredUsernameKey]; len(preferredUsername) > 0 {
username = preferredUsername
}
return &user.DefaultInfo{Name: username}, nil
}

func TestRequestHeader(t *testing.T) {
testcases := map[string]struct {
ConfiguredHeaders []string
RequestHeaders http.Header
ExpectedUsername string
Config Config
RequestHeaders http.Header
ExpectedUsername string
ExpectedIdentity api.UserIdentityInfo
}{
"empty": {
ExpectedUsername: "",
},
"no match": {
ConfiguredHeaders: []string{"X-Remote-User"},
ExpectedUsername: "",
Config: Config{IDHeaders: []string{"X-Remote-User"}},
ExpectedUsername: "",
},
"match": {
ConfiguredHeaders: []string{"X-Remote-User"},
RequestHeaders: http.Header{"X-Remote-User": {"Bob"}},
ExpectedUsername: "Bob",
Config: Config{IDHeaders: []string{"X-Remote-User"}},
RequestHeaders: http.Header{"X-Remote-User": {"Bob"}},
ExpectedUsername: "Bob",
},
"exact match": {
ConfiguredHeaders: []string{"X-Remote-User"},
Config: Config{IDHeaders: []string{"X-Remote-User"}},
RequestHeaders: http.Header{
"Prefixed-X-Remote-User-With-Suffix": {"Bob"},
"X-Remote-User-With-Suffix": {"Bob"},
},
ExpectedUsername: "",
},
"first match": {
ConfiguredHeaders: []string{
Config: Config{IDHeaders: []string{
"X-Remote-User",
"A-Second-X-Remote-User",
"Another-X-Remote-User",
},
}},
RequestHeaders: http.Header{
"X-Remote-User": {"", "First header, second value"},
"A-Second-X-Remote-User": {"Second header, first value", "Second header, second value"},
"Another-X-Remote-User": {"Third header, first value"}},
ExpectedUsername: "Second header, first value",
},
"case-insensitive": {
ConfiguredHeaders: []string{"x-REMOTE-user"}, // configured headers can be case-insensitive
RequestHeaders: http.Header{"X-Remote-User": {"Bob"}}, // the parsed headers are normalized by the http package
ExpectedUsername: "Bob",
Config: Config{IDHeaders: []string{"x-REMOTE-user"}}, // configured headers can be case-insensitive
RequestHeaders: http.Header{"X-Remote-User": {"Bob"}}, // the parsed headers are normalized by the http package
ExpectedUsername: "Bob",
},
"extended attributes": {
Config: Config{
IDHeaders: []string{"x-id", "x-id2"},
PreferredUsernameHeaders: []string{"x-preferred-username", "x-preferred-username2"},
EmailHeaders: []string{"x-email", "x-email2"},
NameHeaders: []string{"x-name", "x-name2"},
},
RequestHeaders: http.Header{
"X-Id2": {"12345"},
"X-Preferred-Username2": {"bob"},
"X-Email2": {"bob@example.com"},
"X-Name2": {"Bob"},
},
ExpectedUsername: "bob",
ExpectedIdentity: &api.DefaultUserIdentityInfo{
ProviderName: "testprovider",
ProviderUserName: "12345",
Extra: map[string]string{
api.IdentityDisplayNameKey: "Bob",
api.IdentityEmailKey: "bob@example.com",
api.IdentityPreferredUsernameKey: "bob",
},
},
},
}

for k, testcase := range testcases {
mapper := &TestUserIdentityMapper{}
auth := NewAuthenticator("testprovider", &Config{testcase.ConfiguredHeaders}, mapper)
auth := NewAuthenticator("testprovider", &testcase.Config, mapper)
req := &http.Request{Header: testcase.RequestHeaders}

user, ok, err := auth.AuthenticateRequest(req)
Expand All @@ -85,5 +118,13 @@ func TestRequestHeader(t *testing.T) {
continue
}
}
if testcase.ExpectedIdentity != nil {
if !reflect.DeepEqual(testcase.ExpectedIdentity.GetExtra(), mapper.Identity.GetExtra()) {
t.Errorf("%s: Expected %#v, got %#v", k, testcase.ExpectedIdentity.GetExtra(), mapper.Identity.GetExtra())
}
if !reflect.DeepEqual(testcase.ExpectedIdentity.GetProviderUserName(), mapper.Identity.GetProviderUserName()) {
t.Errorf("%s: Expected %#v, got %#v", k, testcase.ExpectedIdentity.GetProviderUserName(), mapper.Identity.GetProviderUserName())
}
}
}
}
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/types.go
Expand Up @@ -675,6 +675,12 @@ type RequestHeaderIdentityProvider struct {
ClientCA string
// Headers is the set of headers to check for identity information
Headers []string
// PreferredUsernameHeaders is the set of headers to check for the preferred username
PreferredUsernameHeaders []string
// NameHeaders is the set of headers to check for the display name
NameHeaders []string
// EmailHeaders is the set of headers to check for the email address
EmailHeaders []string
}

type GitHubIdentityProvider struct {
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/v1/types.go
Expand Up @@ -669,6 +669,12 @@ type RequestHeaderIdentityProvider struct {
ClientCA string `json:"clientCA"`
// Headers is the set of headers to check for identity information
Headers []string `json:"headers"`
// PreferredUsernameHeaders is the set of headers to check for the preferred username
PreferredUsernameHeaders []string `json:"preferredUsernameHeaders"`
// NameHeaders is the set of headers to check for the display name
NameHeaders []string `json:"nameHeaders"`
// EmailHeaders is the set of headers to check for the email address
EmailHeaders []string `json:"emailHeaders"`
}

// GitHubIdentityProvider provides identities for users authenticating using GitHub credentials
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/server/api/v1/types_test.go
Expand Up @@ -245,9 +245,12 @@ oauthConfig:
apiVersion: v1
challengeURL: ""
clientCA: ""
emailHeaders: null
headers: null
kind: RequestHeaderIdentityProvider
loginURL: ""
nameHeaders: null
preferredUsernameHeaders: null
- challenge: false
login: false
mappingMethod: ""
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/server/origin/auth.go
Expand Up @@ -615,7 +615,10 @@ func (c *AuthConfig) getAuthenticationRequestHandler() (authenticator.Request, e
var authRequestHandler authenticator.Request

authRequestConfig := &headerrequest.Config{
UserNameHeaders: provider.Headers,
IDHeaders: provider.Headers,
NameHeaders: provider.NameHeaders,
EmailHeaders: provider.EmailHeaders,
PreferredUsernameHeaders: provider.PreferredUsernameHeaders,
}
authRequestHandler = headerrequest.NewAuthenticator(identityProvider.Name, authRequestConfig, identityMapper)

Expand Down