*: connectors use a different identity object than storage

connector/connector.go

 // 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)
 }

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)

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 {

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
 }
 

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,40 +212,30 @@ 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
-	}
-	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
+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,
 	}
-	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
-	}
+	if ok {
 		authReq, err := s.storage.GetAuthRequest(authReqID)
 		if err != nil {
-		log.Printf("get auth request: %v", err)
-		return nil, false, err
+			return "", fmt.Errorf("get auth request: %v", err)
 		}
 		reqGroups := func() bool {
 			for _, scope := range authReq.Scopes {
@@ -258,11 +245,23 @@ func (s *Server) groups(identity storage.Identity, authReqID string, conn connec
 			}
 			return false
 		}()
-	if !reqGroups {
-		return nil, false, nil
+		if reqGroups {
+			if claims.Groups, err = groupsConn.Groups(identity); err != nil {
+				return "", fmt.Errorf("getting groups: %v", 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)
 	}
-	groups, err := groupsConn.Groups(identity)
-	return groups, true, err
+	return path.Join(s.issuerURL.Path, "/approval") + "?state=" + authReqID, nil
 }
 
 func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {

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,
 	}
 }
 
@@ -127,6 +123,7 @@ type AuthRequest struct {
 	Identity *Identity `json:"identity,omitempty"`
 	// The connector used to login the user. Set when the user authenticates.
 	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 {
@@ -199,7 +198,9 @@ type AuthCode struct {
 	State string `json:"state,omitempty"`
 
 	Identity Identity `json:"identity,omitempty"`
+
 	ConnectorID   string `json:"connectorID,omitempty"`
+	ConnectorData []byte `json:"connectorData,omitempty"`
 
 	Expiry time.Time `json:"expiry"`
 }
@@ -224,6 +225,7 @@ func (cli *client) fromStorageAuthCode(a storage.AuthCode) AuthCode {
 		ClientID:      a.ClientID,
 		RedirectURI:   a.RedirectURI,
 		ConnectorID:   a.ConnectorID,
+		ConnectorData: a.ConnectorData,
 		Nonce:         a.Nonce,
 		Scopes:        a.Scopes,
 		Identity:      fromStorageIdentity(a.Identity),
@@ -237,6 +239,7 @@ func toStorageAuthCode(a AuthCode) storage.AuthCode {
 		ClientID:      a.ClientID,
 		RedirectURI:   a.RedirectURI,
 		ConnectorID:   a.ConnectorID,
+		ConnectorData: a.ConnectorData,
 		Nonce:         a.Nonce,
 		Scopes:        a.Scopes,
 		Identity:      toStorageIdentity(a.Identity),

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.
+
+	// 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
+	ConnectorData []byte
 
 	Nonce string
 
@@ -163,7 +162,9 @@ type Refresh struct {
 
 	// Client this refresh token is valid for.
 	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,