Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 22 additions & 53 deletions pkg/auth/oauth/handlers/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -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()},
Expand All @@ -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
}
3 changes: 1 addition & 2 deletions pkg/auth/oauth/handlers/grant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package handlers
import (
"testing"

"github.com/openshift/origin/pkg/oauth/registry/test"
"github.com/openshift/origin/pkg/oauth/server/osinserver"
)

Expand All @@ -16,7 +15,7 @@ func TestEmptyGrant(t *testing.T) {
}

func TestAutoGrant(t *testing.T) {
_ = NewAutoGrant(&test.ClientAuthorizationRegistry{})
_ = NewAutoGrant()
}

func TestRedirectGrant(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/auth/oauth/handlers/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/auth/oauth/registry/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type testHandlers struct {
AuthNeedHandled bool
AuthNeedErr error

GrantNeedGranted bool
GrantNeedHandled bool
GrantNeedErr error

Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/server/origin_master.go
Original file line number Diff line number Diff line change
Expand Up @@ -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", ""),
Expand Down
16 changes: 9 additions & 7 deletions pkg/cmd/util/tokencmd/challenging_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/auth_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion test/integration/cli_get_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
76 changes: 76 additions & 0 deletions test/integration/oauth_htpasswd_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}