diff --git a/pkg/auth/oauth/handlers/grant.go b/pkg/auth/oauth/handlers/grant.go index 0248eae1c89d..8a6dae994ca7 100644 --- a/pkg/auth/oauth/handlers/grant.go +++ b/pkg/auth/oauth/handlers/grant.go @@ -6,13 +6,9 @@ import ( "net/url" "github.com/RangelReale/osin" - "github.com/golang/glog" "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" "github.com/openshift/origin/pkg/auth/api" - oapi "github.com/openshift/origin/pkg/oauth/api" - "github.com/openshift/origin/pkg/oauth/registry/clientauthorization" - "github.com/openshift/origin/pkg/oauth/scope" ) // GrantCheck implements osinserver.AuthorizeHandler to ensure requested scopes have been authorized @@ -54,18 +50,22 @@ func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseW RedirectURI: ar.RedirectUri, } - ok, err := h.check.HasAuthorizedClient(user, grant) + // Check if the user has already authorized this grant + authorized, err := h.check.HasAuthorizedClient(user, grant) if err != nil { return h.errorHandler.GrantError(err, w, ar.HttpRequest) } - if !ok { - return h.handler.GrantNeeded(user, grant, w, ar.HttpRequest) + if authorized { + ar.Authorized = true + return false, nil } - // Grant is verified - ar.Authorized = true - - return false, nil + // React to an unauthorized grant + authorized, handled, err := h.handler.GrantNeeded(user, grant, w, ar.HttpRequest) + if authorized { + ar.Authorized = true + } + return handled, err } type emptyGrant struct{} @@ -76,52 +76,21 @@ func NewEmptyGrant() GrantHandler { } // GrantNeeded implements the GrantHandler interface -func (emptyGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, error) { - return false, nil +func (emptyGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { + return false, false, nil } type autoGrant struct { - authregistry clientauthorization.Registry } -// NewAutoGrant returns a grant handler that automatically creates client authorizations -// when a grant is needed, then retries the original request -func NewAutoGrant(authregistry clientauthorization.Registry) GrantHandler { - return &autoGrant{authregistry} +// NewAutoGrant returns a grant handler that automatically approves client authorizations +func NewAutoGrant() GrantHandler { + return &autoGrant{} } // GrantNeeded implements the GrantHandler interface -func (g *autoGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, error) { - clientAuthID := g.authregistry.ClientAuthorizationName(user.GetName(), grant.Client.GetId()) - clientAuth, err := g.authregistry.GetClientAuthorization(clientAuthID) - if err == nil { - // Add new scopes and update - clientAuth.Scopes = scope.Add(clientAuth.Scopes, scope.Split(grant.Scope)) - err = g.authregistry.UpdateClientAuthorization(clientAuth) - if err != nil { - glog.V(4).Infof("Unable to update authorization: %v", err) - return false, err - } - } else { - // Make sure client name, user name, grant scope, expiration, and redirect uri match - clientAuth = &oapi.OAuthClientAuthorization{ - UserName: user.GetName(), - UserUID: user.GetUID(), - ClientName: grant.Client.GetId(), - Scopes: scope.Split(grant.Scope), - } - clientAuth.Name = clientAuthID - - err = g.authregistry.CreateClientAuthorization(clientAuth) - if err != nil { - glog.V(4).Infof("Unable to create authorization: %v", err) - return false, err - } - } - - // Retry the request - http.Redirect(w, req, req.URL.String(), http.StatusFound) - return true, nil +func (g *autoGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { + return true, false, nil } type redirectGrant struct { @@ -143,15 +112,15 @@ func NewRedirectGrant(url string) GrantHandler { } // GrantNeeded implements the GrantHandler interface -func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, error) { +func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { // If the current request has an error=grant_denied parameter, the user denied the grant if err := req.FormValue("error"); err == GrantDeniedError { - return false, nil + return false, false, nil } redirectURL, err := url.Parse(g.url) if err != nil { - return false, err + return false, false, err } redirectURL.RawQuery = url.Values{ "then": {req.URL.String()}, @@ -160,5 +129,5 @@ func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.Res "redirect_uri": {grant.RedirectURI}, }.Encode() http.Redirect(w, req, redirectURL.String(), http.StatusFound) - return true, nil + return false, true, nil } diff --git a/pkg/auth/oauth/handlers/grant_test.go b/pkg/auth/oauth/handlers/grant_test.go index 1c7e16baafb0..e32c8f1594a7 100644 --- a/pkg/auth/oauth/handlers/grant_test.go +++ b/pkg/auth/oauth/handlers/grant_test.go @@ -3,7 +3,6 @@ package handlers import ( "testing" - "github.com/openshift/origin/pkg/oauth/registry/test" "github.com/openshift/origin/pkg/oauth/server/osinserver" ) @@ -16,7 +15,7 @@ func TestEmptyGrant(t *testing.T) { } func TestAutoGrant(t *testing.T) { - _ = NewAutoGrant(&test.ClientAuthorizationRegistry{}) + _ = NewAutoGrant() } func TestRedirectGrant(t *testing.T) { diff --git a/pkg/auth/oauth/handlers/interfaces.go b/pkg/auth/oauth/handlers/interfaces.go index 3134ce158c5c..d7331d66a493 100644 --- a/pkg/auth/oauth/handlers/interfaces.go +++ b/pkg/auth/oauth/handlers/interfaces.go @@ -48,7 +48,9 @@ type GrantChecker interface { // GrantHandler handles errors during the grant process, or the client requests an unauthorized grant type GrantHandler interface { // GrantNeeded reacts when a client requests an unauthorized grant, and returns true if the response was written - GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (handled bool, err error) + // granted is true if authorization was granted + // handled is true if the response was already written + GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (granted, handled bool, err error) } // GrantErrorHandler reacts to grant errors diff --git a/pkg/auth/oauth/registry/registry_test.go b/pkg/auth/oauth/registry/registry_test.go index bceb49c2c629..42c2610802da 100644 --- a/pkg/auth/oauth/registry/registry_test.go +++ b/pkg/auth/oauth/registry/registry_test.go @@ -33,6 +33,7 @@ type testHandlers struct { AuthNeedHandled bool AuthNeedErr error + GrantNeedGranted bool GrantNeedHandled bool GrantNeedErr error @@ -53,9 +54,9 @@ func (h *testHandlers) AuthenticateRequest(req *http.Request) (user.Info, bool, return h.User, h.Authenticate, h.Err } -func (h *testHandlers) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, error) { +func (h *testHandlers) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) { h.GrantNeed = true - return h.GrantNeedHandled, h.GrantNeedErr + return h.GrantNeedGranted, h.GrantNeedHandled, h.GrantNeedErr } func (h *testHandlers) GrantError(err error, w http.ResponseWriter, req *http.Request) (bool, error) { diff --git a/pkg/cmd/server/origin/auth.go b/pkg/cmd/server/origin/auth.go index f515c66606cf..23e3efb61516 100644 --- a/pkg/cmd/server/origin/auth.go +++ b/pkg/cmd/server/origin/auth.go @@ -387,7 +387,7 @@ func (c *AuthConfig) getGrantHandler(mux cmdutil.Mux, auth authenticator.Request case GrantHandlerDeny: grantHandler = handlers.NewEmptyGrant() case GrantHandlerAuto: - grantHandler = handlers.NewAutoGrant(authregistry) + grantHandler = handlers.NewAutoGrant() case GrantHandlerPrompt: grantServer := grant.NewGrant(getCSRF(), auth, grant.DefaultFormRenderer, clientregistry, authregistry) grantServer.Install(mux, OpenShiftApprovePrefix) diff --git a/pkg/cmd/server/origin_master.go b/pkg/cmd/server/origin_master.go index afb78cf9a9a0..4967e0dbc8db 100644 --- a/pkg/cmd/server/origin_master.go +++ b/pkg/cmd/server/origin_master.go @@ -176,6 +176,7 @@ func (cfg Config) BuildAuthConfig() (*origin.AuthConfig, error) { // Password config PasswordAuth: origin.PasswordAuthType(env("OPENSHIFT_OAUTH_PASSWORD_AUTH", string(origin.PasswordAuthAnyPassword))), BasicAuthURL: env("OPENSHIFT_OAUTH_BASIC_AUTH_URL", ""), + HTPasswdFile: env("OPENSHIFT_OAUTH_HTPASSWD_FILE", ""), // Token config TokenStore: origin.TokenStoreType(env("OPENSHIFT_OAUTH_TOKEN_STORE", string(origin.TokenStoreOAuth))), TokenFilePath: env("OPENSHIFT_OAUTH_TOKEN_FILE_PATH", ""), diff --git a/pkg/cmd/util/tokencmd/challenging_client.go b/pkg/cmd/util/tokencmd/challenging_client.go index 76f6f7351fc0..9c8b449d7207 100644 --- a/pkg/cmd/util/tokencmd/challenging_client.go +++ b/pkg/cmd/util/tokencmd/challenging_client.go @@ -37,21 +37,23 @@ func (client *challengingClient) Do(req *http.Request) (*http.Response, error) { username := client.defaultUsername password := client.defaultPassword - uDefaulted := len(username) > 0 - pDefaulted := len(password) > 0 + missingUsername := len(username) == 0 + missingPassword := len(password) == 0 - if !(uDefaulted && pDefaulted) { + if (missingUsername || missingPassword) && (client.reader != nil) { fmt.Printf("Authenticate for \"%v\"\n", realm) - if !uDefaulted { + if missingUsername { username = util.PromptForString(client.reader, "Username: ") } - if !pDefaulted { + if missingPassword { password = util.PromptForPasswordString(client.reader, "Password: ") } } - client.delegate.Transport = kclient.NewBasicAuthRoundTripper(username, password, client.delegate.Transport) - return client.Do(resp.Request) + if len(username) > 0 || len(password) > 0 { + client.delegate.Transport = kclient.NewBasicAuthRoundTripper(username, password, client.delegate.Transport) + return client.delegate.Do(resp.Request) + } } } return resp, err diff --git a/test/integration/auth_proxy_test.go b/test/integration/auth_proxy_test.go index 0fcbf4baac92..b70cd6b5c764 100644 --- a/test/integration/auth_proxy_test.go +++ b/test/integration/auth_proxy_test.go @@ -58,7 +58,7 @@ func TestFrontProxyOnAuthorize(t *testing.T) { config := osinserver.NewDefaultServerConfig() grantChecker := oauthregistry.NewClientAuthorizationGrantChecker(oauthEtcd) - grantHandler := oauthhandlers.NewAutoGrant(oauthEtcd) + grantHandler := oauthhandlers.NewAutoGrant() server := osinserver.New( config, diff --git a/test/integration/cli_get_token_test.go b/test/integration/cli_get_token_test.go index d303b335d548..3deaf2f8b5d2 100644 --- a/test/integration/cli_get_token_test.go +++ b/test/integration/cli_get_token_test.go @@ -54,7 +54,7 @@ func TestGetToken(t *testing.T) { config := osinserver.NewDefaultServerConfig() grantChecker := oauthregistry.NewClientAuthorizationGrantChecker(oauthEtcd) - grantHandler := oauthhandlers.NewAutoGrant(oauthEtcd) + grantHandler := oauthhandlers.NewAutoGrant() server := osinserver.New( config, diff --git a/test/integration/oauth_htpasswd_test.go b/test/integration/oauth_htpasswd_test.go new file mode 100644 index 000000000000..1750f316f926 --- /dev/null +++ b/test/integration/oauth_htpasswd_test.go @@ -0,0 +1,76 @@ +// +build integration,!no-etcd + +package integration + +import ( + "io/ioutil" + "os" + "testing" + + kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + + "github.com/openshift/origin/pkg/client" + "github.com/openshift/origin/pkg/cmd/util/tokencmd" +) + +func TestHTPasswd(t *testing.T) { + htpasswdFile, err := ioutil.TempFile("", "test.htpasswd") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer os.Remove(htpasswdFile.Name()) + + // This is all that should be needed to enable htpasswd-based auth + // If these change, need to update documentation at http://docs.openshift.org/latest/architecture/authentication.html + os.Setenv("OPENSHIFT_OAUTH_PASSWORD_AUTH", "htpasswd") + os.Setenv("OPENSHIFT_OAUTH_HTPASSWD_FILE", htpasswdFile.Name()) + + startConfig, err := StartTestMaster() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + _, clientConfig, err := startConfig.GetOpenshiftClient() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Use the server and CA info + anonConfig := kclient.Config{} + anonConfig.Host = clientConfig.Host + anonConfig.CAFile = clientConfig.CAFile + anonConfig.CAData = clientConfig.CAData + + // Make sure we can't authenticate + if _, err := tokencmd.RequestToken(&anonConfig, nil, "username", "password"); err == nil { + t.Errorf("Expected error, got none", err) + } + + // Update the htpasswd file with output of `htpasswd -n -b username password` + userpass := "username:$apr1$4Ci5I8yc$85R9vc4fOgzAULsldiUuv." + ioutil.WriteFile(htpasswdFile.Name(), []byte(userpass), os.FileMode(0600)) + + // Make sure we can get a token + accessToken, err := tokencmd.RequestToken(&anonConfig, nil, "username", "password") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(accessToken) == 0 { + t.Errorf("Expected access token, got none") + } + + // Make sure we can use the token, and it represents who we expect + userConfig := anonConfig + userConfig.BearerToken = accessToken + userClient, err := client.New(&userConfig) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + user, err := userClient.Users().Get("~") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if user.Name != "htpasswd:username" { + t.Fatalf("Expected htpasswd:username as the user, got %v", user) + } +}