From f4c5722e427de19ae82564cf96f9b8d91256a54e Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Tue, 2 Aug 2016 21:14:24 -0700 Subject: [PATCH] *: connectors use a different identity object than storage --- connector/connector.go | 26 +++++++---- connector/github/github.go | 7 ++- connector/ldap/ldap.go | 10 ++-- connector/mock/connectortest.go | 20 ++++++-- server/handlers.go | 83 ++++++++++++++++----------------- storage/kubernetes/types.go | 47 ++++++++++--------- storage/storage.go | 23 ++++----- 7 files changed, 121 insertions(+), 95 deletions(-) diff --git a/connector/connector.go b/connector/connector.go index 4cdbcae4..8235caae 100644 --- a/connector/connector.go +++ b/connector/connector.go @@ -1,11 +1,7 @@ // Package connector defines interfaces for federated identity strategies. package connector -import ( - "net/http" - - "github.com/coreos/poke/storage" -) +import "net/http" // Connector is a mechanism for federating login to a remote identity service. // @@ -15,18 +11,32 @@ type Connector interface { Close() error } +// Identity represents the ID Token claims supported by the server. +type Identity struct { + UserID string + Username string + Email string + EmailVerified bool + + // ConnectorData holds data used by the connector for subsequent requests after initial + // authentication, such as access tokens for upstream provides. + // + // This data is never shared with end users, OAuth clients, or through the API. + ConnectorData []byte +} + // PasswordConnector is an optional interface for password based connectors. type PasswordConnector interface { - Login(username, password string) (identity storage.Identity, validPassword bool, err error) + Login(username, password string) (identity Identity, validPassword bool, err error) } // CallbackConnector is an optional interface for callback based connectors. type CallbackConnector interface { LoginURL(callbackURL, state string) (string, error) - HandleCallback(r *http.Request) (identity storage.Identity, state string, err error) + HandleCallback(r *http.Request) (identity Identity, state string, err error) } // GroupsConnector is an optional interface for connectors which can map a user to groups. type GroupsConnector interface { - Groups(identity storage.Identity) ([]string, error) + Groups(identity Identity) ([]string, error) } diff --git a/connector/github/github.go b/connector/github/github.go index 3dbd2f05..2546f16b 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -14,7 +14,6 @@ import ( "golang.org/x/oauth2/github" "github.com/coreos/poke/connector" - "github.com/coreos/poke/storage" ) const baseURL = "https://api.github.com" @@ -85,7 +84,7 @@ func (e *oauth2Error) Error() string { return e.error + ": " + e.errorDescription } -func (c *githubConnector) HandleCallback(r *http.Request) (identity storage.Identity, state string, err error) { +func (c *githubConnector) HandleCallback(r *http.Request) (identity connector.Identity, state string, err error) { q := r.URL.Query() if errType := q.Get("error"); errType != "" { return identity, "", &oauth2Error{errType, q.Get("error_description")} @@ -128,7 +127,7 @@ func (c *githubConnector) HandleCallback(r *http.Request) (identity storage.Iden if username == "" { username = user.Login } - identity = storage.Identity{ + identity = connector.Identity{ UserID: strconv.Itoa(user.ID), Username: username, Email: user.Email, @@ -138,7 +137,7 @@ func (c *githubConnector) HandleCallback(r *http.Request) (identity storage.Iden return identity, q.Get("state"), nil } -func (c *githubConnector) Groups(identity storage.Identity) ([]string, error) { +func (c *githubConnector) Groups(identity connector.Identity) ([]string, error) { var data connectorData if err := json.Unmarshal(identity.ConnectorData, &data); err != nil { return nil, fmt.Errorf("decode connector data: %v", err) diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index 21c4fce8..746b410c 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -8,7 +8,6 @@ import ( "gopkg.in/ldap.v2" "github.com/coreos/poke/connector" - "github.com/coreos/poke/storage" ) // Config holds the configuration parameters for the LDAP connector. @@ -32,6 +31,8 @@ type ldapConnector struct { Config } +var _ connector.PasswordConnector = (*ldapConnector)(nil) + func (c *ldapConnector) do(f func(c *ldap.Conn) error) error { // TODO(ericchiang): Connection pooling. conn, err := ldap.Dial("tcp", c.Host) @@ -43,15 +44,16 @@ func (c *ldapConnector) do(f func(c *ldap.Conn) error) error { return f(conn) } -func (c *ldapConnector) Login(username, password string) (storage.Identity, error) { +func (c *ldapConnector) Login(username, password string) (connector.Identity, bool, error) { err := c.do(func(conn *ldap.Conn) error { return conn.Bind(fmt.Sprintf("uid=%s,%s", username, c.BindDN), password) }) if err != nil { - return storage.Identity{}, err + // TODO(ericchiang): Determine when the user has entered invalid credentials. + return connector.Identity{}, false, err } - return storage.Identity{Username: username}, nil + return connector.Identity{Username: username}, true, nil } func (c *ldapConnector) Close() error { diff --git a/connector/mock/connectortest.go b/connector/mock/connectortest.go index 4c7a821d..216681a7 100644 --- a/connector/mock/connectortest.go +++ b/connector/mock/connectortest.go @@ -2,12 +2,13 @@ package mock import ( + "bytes" + "errors" "fmt" "net/http" "net/url" "github.com/coreos/poke/connector" - "github.com/coreos/poke/storage" ) // New returns a mock connector which requires no user interaction. It always returns @@ -16,6 +17,11 @@ func New() connector.Connector { return mockConnector{} } +var ( + _ connector.CallbackConnector = mockConnector{} + _ connector.GroupsConnector = mockConnector{} +) + type mockConnector struct{} func (m mockConnector) Close() error { return nil } @@ -31,16 +37,22 @@ func (m mockConnector) LoginURL(callbackURL, state string) (string, error) { return u.String(), nil } -func (m mockConnector) HandleCallback(r *http.Request) (storage.Identity, string, error) { - return storage.Identity{ +var connectorData = []byte("foobar") + +func (m mockConnector) HandleCallback(r *http.Request) (connector.Identity, string, error) { + return connector.Identity{ UserID: "0-385-28089-0", Username: "Kilgore Trout", Email: "kilgore@kilgore.trout", EmailVerified: true, + ConnectorData: connectorData, }, r.URL.Query().Get("state"), nil } -func (m mockConnector) Groups(identity storage.Identity) ([]string, error) { +func (m mockConnector) Groups(identity connector.Identity) ([]string, error) { + if !bytes.Equal(identity.ConnectorData, connectorData) { + return nil, errors.New("connector data mismatch") + } return []string{"authors"}, nil } diff --git a/server/handlers.go b/server/handlers.go index 8a555391..6ce2f6e5 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -180,17 +180,14 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) { renderPasswordTmpl(w, state, r.URL.String(), "Invalid credentials") return } - - groups, ok, err := s.groups(identity, state, conn.Connector) + redirectURL, err := s.finalizeLogin(identity, state, connID, conn.Connector) if err != nil { + log.Printf("Failed to finalize login: %v", err) s.renderError(w, http.StatusInternalServerError, errServerError, "") return } - if ok { - identity.Groups = groups - } - s.redirectToApproval(w, r, identity, connID, state) + http.Redirect(w, r, redirectURL, http.StatusSeeOther) default: s.notFound(w, r) } @@ -215,54 +212,56 @@ func (s *Server) handleConnectorCallback(w http.ResponseWriter, r *http.Request) s.renderError(w, http.StatusInternalServerError, errServerError, "") return } - groups, ok, err := s.groups(identity, state, conn.Connector) + + redirectURL, err := s.finalizeLogin(identity, state, connID, conn.Connector) if err != nil { + log.Printf("Failed to finalize login: %v", err) s.renderError(w, http.StatusInternalServerError, errServerError, "") return } - if ok { - identity.Groups = groups - } - s.redirectToApproval(w, r, identity, connID, state) + + http.Redirect(w, r, redirectURL, http.StatusSeeOther) } -func (s *Server) redirectToApproval(w http.ResponseWriter, r *http.Request, identity storage.Identity, connectorID, state string) { - updater := func(a storage.AuthRequest) (storage.AuthRequest, error) { - a.Identity = &identity - a.ConnectorID = connectorID - return a, nil +func (s *Server) finalizeLogin(identity connector.Identity, authReqID, connectorID string, conn connector.Connector) (string, error) { + claims := storage.Identity{ + UserID: identity.UserID, + Username: identity.Username, + Email: identity.Email, + EmailVerified: identity.EmailVerified, } - if err := s.storage.UpdateAuthRequest(state, updater); err != nil { - log.Printf("Failed to updated auth request with identity: %v", err) - s.renderError(w, http.StatusInternalServerError, errServerError, "") - return - } - http.Redirect(w, r, path.Join(s.issuerURL.Path, "/approval")+"?state="+state, http.StatusSeeOther) -} -func (s *Server) groups(identity storage.Identity, authReqID string, conn connector.Connector) ([]string, bool, error) { groupsConn, ok := conn.(connector.GroupsConnector) - if !ok { - return nil, false, nil - } - authReq, err := s.storage.GetAuthRequest(authReqID) - if err != nil { - log.Printf("get auth request: %v", err) - return nil, false, err - } - reqGroups := func() bool { - for _, scope := range authReq.Scopes { - if scope == scopeGroups { - return true + if ok { + authReq, err := s.storage.GetAuthRequest(authReqID) + if err != nil { + return "", fmt.Errorf("get auth request: %v", err) + } + reqGroups := func() bool { + for _, scope := range authReq.Scopes { + if scope == scopeGroups { + return true + } + } + return false + }() + if reqGroups { + if claims.Groups, err = groupsConn.Groups(identity); err != nil { + return "", fmt.Errorf("getting groups: %v", err) } } - return false - }() - if !reqGroups { - return nil, false, nil } - groups, err := groupsConn.Groups(identity) - return groups, true, err + + updater := func(a storage.AuthRequest) (storage.AuthRequest, error) { + a.Identity = &claims + a.ConnectorID = connectorID + a.ConnectorData = identity.ConnectorData + return a, nil + } + if err := s.storage.UpdateAuthRequest(authReqID, updater); err != nil { + return "", fmt.Errorf("failed to update auth request: %v", err) + } + return path.Join(s.issuerURL.Path, "/approval") + "?state=" + authReqID, nil } func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) { diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go index 9124f205..f9f4a0f3 100644 --- a/storage/kubernetes/types.go +++ b/storage/kubernetes/types.go @@ -77,8 +77,6 @@ type Identity struct { Email string `json:"email"` EmailVerified bool `json:"emailVerified"` Groups []string `json:"groups,omitempty"` - - ConnectorData []byte `json:"connectorData,omitempty"` } func fromStorageIdentity(i storage.Identity) Identity { @@ -88,7 +86,6 @@ func fromStorageIdentity(i storage.Identity) Identity { Email: i.Email, EmailVerified: i.EmailVerified, Groups: i.Groups, - ConnectorData: i.ConnectorData, } } @@ -99,7 +96,6 @@ func toStorageIdentity(i Identity) storage.Identity { Email: i.Email, EmailVerified: i.EmailVerified, Groups: i.Groups, - ConnectorData: i.ConnectorData, } } @@ -126,7 +122,8 @@ type AuthRequest struct { // with a backend. Identity *Identity `json:"identity,omitempty"` // The connector used to login the user. Set when the user authenticates. - ConnectorID string `json:"connectorID,omitempty"` + ConnectorID string `json:"connectorID,omitempty"` + ConnectorData []byte `json:"connectorData,omitempty"` Expiry time.Time `json:"expiry"` } @@ -149,6 +146,7 @@ func toStorageAuthRequest(req AuthRequest) storage.AuthRequest { State: req.State, ForceApprovalPrompt: req.ForceApprovalPrompt, ConnectorID: req.ConnectorID, + ConnectorData: req.ConnectorData, Expiry: req.Expiry, } if req.Identity != nil { @@ -176,6 +174,7 @@ func (cli *client) fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { State: a.State, ForceApprovalPrompt: a.ForceApprovalPrompt, ConnectorID: a.ConnectorID, + ConnectorData: a.ConnectorData, Expiry: a.Expiry, } if a.Identity != nil { @@ -198,8 +197,10 @@ type AuthCode struct { Nonce string `json:"nonce,omitempty"` State string `json:"state,omitempty"` - Identity Identity `json:"identity,omitempty"` - ConnectorID string `json:"connectorID,omitempty"` + Identity Identity `json:"identity,omitempty"` + + ConnectorID string `json:"connectorID,omitempty"` + ConnectorData []byte `json:"connectorData,omitempty"` Expiry time.Time `json:"expiry"` } @@ -221,26 +222,28 @@ func (cli *client) fromStorageAuthCode(a storage.AuthCode) AuthCode { Name: a.ID, Namespace: cli.namespace, }, - ClientID: a.ClientID, - RedirectURI: a.RedirectURI, - ConnectorID: a.ConnectorID, - Nonce: a.Nonce, - Scopes: a.Scopes, - Identity: fromStorageIdentity(a.Identity), - Expiry: a.Expiry, + ClientID: a.ClientID, + RedirectURI: a.RedirectURI, + ConnectorID: a.ConnectorID, + ConnectorData: a.ConnectorData, + Nonce: a.Nonce, + Scopes: a.Scopes, + Identity: fromStorageIdentity(a.Identity), + Expiry: a.Expiry, } } func toStorageAuthCode(a AuthCode) storage.AuthCode { return storage.AuthCode{ - ID: a.ObjectMeta.Name, - ClientID: a.ClientID, - RedirectURI: a.RedirectURI, - ConnectorID: a.ConnectorID, - Nonce: a.Nonce, - Scopes: a.Scopes, - Identity: toStorageIdentity(a.Identity), - Expiry: a.Expiry, + ID: a.ObjectMeta.Name, + ClientID: a.ClientID, + RedirectURI: a.RedirectURI, + ConnectorID: a.ConnectorID, + ConnectorData: a.ConnectorData, + Nonce: a.Nonce, + Scopes: a.Scopes, + Identity: toStorageIdentity(a.Identity), + Expiry: a.Expiry, } } diff --git a/storage/storage.go b/storage/storage.go index 32b65b6c..536d9a96 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -104,12 +104,6 @@ type Identity struct { EmailVerified bool Groups []string - - // ConnectorData holds data used by the connector for subsequent requests after initial - // authentication, such as access tokens for upstream provides. - // - // This data is never shared with end users, OAuth clients, or through the API. - ConnectorData []byte } // AuthRequest represents a OAuth2 client authorization request. It holds the state @@ -133,8 +127,11 @@ type AuthRequest struct { // The identity of the end user. Generally nil until the user authenticates // with a backend. Identity *Identity - // The connector used to login the user. Set when the user authenticates. - ConnectorID string + + // The connector used to login the user and any data the connector wishes to persists. + // Set when the user authenticates. + ConnectorID string + ConnectorData []byte Expiry time.Time } @@ -145,7 +142,9 @@ type AuthCode struct { ClientID string RedirectURI string - ConnectorID string + + ConnectorID string + ConnectorData []byte Nonce string @@ -162,8 +161,10 @@ type Refresh struct { RefreshToken string // Client this refresh token is valid for. - ClientID string - ConnectorID string + ClientID string + + ConnectorID string + ConnectorData []byte // Scopes present in the initial request. Refresh requests may specify a set // of scopes different from the initial request when refreshing a token,