Merge pull request #199 from ericchiang/validate_connector

api: validate local connector existence before creating user

admin/api.go

@@ -6,17 +6,18 @@ import (
 
 	"github.com/coreos/dex/schema/adminschema"
 	"github.com/coreos/dex/user"
+	"github.com/coreos/dex/user/manager"
 )
 
 // AdminAPI provides the logic necessary to implement the Admin API.
 type AdminAPI struct {
-	userManager      *user.Manager
+	userManager      *manager.UserManager
 	userRepo         user.UserRepo
 	passwordInfoRepo user.PasswordInfoRepo
 	localConnectorID string
 }
 
-func NewAdminAPI(userManager *user.Manager, userRepo user.UserRepo, pwiRepo user.PasswordInfoRepo, localConnectorID string) *AdminAPI {
+func NewAdminAPI(userManager *manager.UserManager, userRepo user.UserRepo, pwiRepo user.PasswordInfoRepo, localConnectorID string) *AdminAPI {
 	if localConnectorID == "" {
 		panic("must specify non-blank localConnectorID")
 	}

cmd/dex-overlord/main.go

@@ -17,7 +17,7 @@ import (
 	"github.com/coreos/dex/pkg/log"
 	ptime "github.com/coreos/dex/pkg/time"
 	"github.com/coreos/dex/server"
-	"github.com/coreos/dex/user"
+	"github.com/coreos/dex/user/manager"
 )
 
 var version = "DEV"
@@ -99,8 +99,9 @@ func main() {
 
 	userRepo := db.NewUserRepo(dbc)
 	pwiRepo := db.NewPasswordInfoRepo(dbc)
-	userManager := user.NewManager(userRepo,
-		pwiRepo, db.TransactionFactory(dbc), user.ManagerOptions{})
+	connCfgRepo := db.NewConnectorConfigRepo(dbc)
+	userManager := manager.NewUserManager(userRepo,
+		pwiRepo, connCfgRepo, db.TransactionFactory(dbc), manager.ManagerOptions{})
 	adminAPI := admin.NewAdminAPI(userManager, userRepo, pwiRepo, *localConnectorID)
 	kRepo, err := db.NewPrivateKeySetRepo(dbc, *useOldFormat, keySecrets.BytesSlice()...)
 	if err != nil {

connector/config_repo.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"io"
 	"os"
+
+	"github.com/coreos/dex/repo"
 )
 
 func newConnectorConfigsFromReader(r io.Reader) ([]ConnectorConfig, error) {
@@ -41,6 +43,19 @@ type memConnectorConfigRepo struct {
 	configs []ConnectorConfig
 }
 
+func NewConnectorConfigRepoFromConfigs(cfgs []ConnectorConfig) ConnectorConfigRepo {
+	return &memConnectorConfigRepo{configs: cfgs}
+}
+
 func (r *memConnectorConfigRepo) All() ([]ConnectorConfig, error) {
 	return r.configs, nil
 }
+
+func (r *memConnectorConfigRepo) GetConnectorByID(_ repo.Transaction, id string) (ConnectorConfig, error) {
+	for _, cfg := range r.configs {
+		if cfg.ConnectorID() == id {
+			return cfg, nil
+		}
+	}
+	return nil, ErrorNotFound
+}

connector/interface.go

@@ -1,14 +1,18 @@
 package connector
 
 import (
+	"errors"
 	"html/template"
 	"net/http"
 	"net/url"
 
+	"github.com/coreos/dex/repo"
 	"github.com/coreos/go-oidc/oidc"
 	"github.com/coreos/pkg/health"
 )
 
+var ErrorNotFound = errors.New("connector not found in repository")
+
 type Connector interface {
 	ID() string
 	LoginURL(sessionKey, prompt string) (string, error)
@@ -34,4 +38,5 @@ type ConnectorConfig interface {
 
 type ConnectorConfigRepo interface {
 	All() ([]ConnectorConfig, error)
+	GetConnectorByID(repo.Transaction, string) (ConnectorConfig, error)
 }

db/connector_config.go

@@ -1,6 +1,7 @@
 package db
 
 import (
+	"database/sql"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -9,6 +10,7 @@ import (
 	"github.com/lib/pq"
 
 	"github.com/coreos/dex/connector"
+	"github.com/coreos/dex/repo"
 )
 
 const (
@@ -91,6 +93,18 @@ func (r *ConnectorConfigRepo) All() ([]connector.ConnectorConfig, error) {
 	return cfgs, nil
 }
 
+func (r *ConnectorConfigRepo) GetConnectorByID(tx repo.Transaction, id string) (connector.ConnectorConfig, error) {
+	qt := pq.QuoteIdentifier(connectorConfigTableName)
+	q := fmt.Sprintf("SELECT * FROM %s WHERE id = $1", qt)
+	var c connectorConfigModel
+	if err := r.executor(tx).SelectOne(&c, q, id); err != nil {
+		if err == sql.ErrNoRows {
+			return nil, connector.ErrorNotFound
+		}
+	}
+	return c.ConnectorConfig()
+}
+
 func (r *ConnectorConfigRepo) Set(cfgs []connector.ConnectorConfig) error {
 	insert := make([]interface{}, len(cfgs))
 	for i, cfg := range cfgs {
@@ -119,3 +133,15 @@ func (r *ConnectorConfigRepo) Set(cfgs []connector.ConnectorConfig) error {
 
 	return tx.Commit()
 }
+
+func (r *ConnectorConfigRepo) executor(tx repo.Transaction) gorp.SqlExecutor {
+	if tx == nil {
+		return r.dbMap
+	}
+
+	gorpTx, ok := tx.(*gorp.Transaction)
+	if !ok {
+		panic("wrong kind of transaction passed to a DB repo")
+	}
+	return gorpTx
+}

functional/repo/connector_repo_test.go

@@ -0,0 +1,71 @@
+package repo
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/coreos/dex/connector"
+	"github.com/coreos/dex/db"
+)
+
+type connectorConfigRepoFactory func(cfgs []connector.ConnectorConfig) connector.ConnectorConfigRepo
+
+var makeTestConnectorConfigRepoFromConfigs connectorConfigRepoFactory
+
+func init() {
+	if dsn := os.Getenv("DEX_TEST_DSN"); dsn == "" {
+		makeTestConnectorConfigRepoFromConfigs = connector.NewConnectorConfigRepoFromConfigs
+	} else {
+		makeTestConnectorConfigRepoFromConfigs = makeTestConnectorConfigRepoMem(dsn)
+	}
+}
+
+func makeTestConnectorConfigRepoMem(dsn string) connectorConfigRepoFactory {
+	return func(cfgs []connector.ConnectorConfig) connector.ConnectorConfigRepo {
+		dbMap := initDB(dsn)
+
+		repo := db.NewConnectorConfigRepo(dbMap)
+		if err := repo.Set(cfgs); err != nil {
+			panic(fmt.Sprintf("Unable to set connector configs: %v", err))
+		}
+		return repo
+	}
+}
+
+func TestConnectorConfigRepoGetByID(t *testing.T) {
+	tests := []struct {
+		cfgs []connector.ConnectorConfig
+		id   string
+		err  error
+	}{
+		{
+			cfgs: []connector.ConnectorConfig{
+				&connector.LocalConnectorConfig{ID: "local"},
+			},
+			id: "local",
+		},
+		{
+			cfgs: []connector.ConnectorConfig{
+				&connector.LocalConnectorConfig{ID: "local1"},
+				&connector.LocalConnectorConfig{ID: "local2"},
+			},
+			id: "local2",
+		},
+		{
+			cfgs: []connector.ConnectorConfig{
+				&connector.LocalConnectorConfig{ID: "local1"},
+				&connector.LocalConnectorConfig{ID: "local2"},
+			},
+			id:  "foo",
+			err: connector.ErrorNotFound,
+		},
+	}
+
+	for i, tt := range tests {
+		repo := makeTestConnectorConfigRepoFromConfigs(tt.cfgs)
+		if _, err := repo.GetConnectorByID(nil, tt.id); err != tt.err {
+			t.Errorf("case %d: want=%v, got=%v", i, tt.err, err)
+		}
+	}
+}

integration/common_test.go

@@ -10,8 +10,10 @@ import (
 	"github.com/coreos/go-oidc/key"
 	"github.com/jonboulle/clockwork"
 
+	"github.com/coreos/dex/connector"
 	"github.com/coreos/dex/repo"
 	"github.com/coreos/dex/user"
+	"github.com/coreos/dex/user/manager"
 )
 
 var (
@@ -42,11 +44,14 @@ func (t *tokenHandlerTransport) RoundTrip(r *http.Request) (*http.Response, erro
 	return &resp, nil
 }
 
-func makeUserObjects(users []user.UserWithRemoteIdentities, passwords []user.PasswordInfo) (user.UserRepo, user.PasswordInfoRepo, *user.Manager) {
+func makeUserObjects(users []user.UserWithRemoteIdentities, passwords []user.PasswordInfo) (user.UserRepo, user.PasswordInfoRepo, *manager.UserManager) {
 	ur := user.NewUserRepoFromUsers(users)
 	pwr := user.NewPasswordInfoRepoFromPasswordInfos(passwords)
 
-	um := user.NewManager(ur, pwr, repo.InMemTransactionFactory, user.ManagerOptions{})
+	ccr := connector.NewConnectorConfigRepoFromConfigs(
+		[]connector.ConnectorConfig{&connector.LocalConnectorConfig{ID: "local"}},
+	)
+	um := manager.NewUserManager(ur, pwr, ccr, repo.InMemTransactionFactory, manager.ManagerOptions{})
 	um.Clock = clock
 	return ur, pwr, um
 }

server/config.go

@@ -22,6 +22,7 @@ import (
 	"github.com/coreos/dex/session"
 	"github.com/coreos/dex/user"
 	useremail "github.com/coreos/dex/user/email"
+	"github.com/coreos/dex/user/manager"
 )
 
 type ServerConfig struct {
@@ -133,7 +134,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error {
 	refTokRepo := refresh.NewRefreshTokenRepo()
 
 	txnFactory := repo.InMemTransactionFactory
-	userManager := user.NewManager(userRepo, pwiRepo, txnFactory, user.ManagerOptions{})
+	userManager := manager.NewUserManager(userRepo, pwiRepo, cfgRepo, txnFactory, manager.ManagerOptions{})
 	srv.ClientIdentityRepo = ciRepo
 	srv.KeySetRepo = kRepo
 	srv.ConnectorConfigRepo = cfgRepo
@@ -171,7 +172,7 @@ func (cfg *MultiServerConfig) Configure(srv *Server) error {
 	cfgRepo := db.NewConnectorConfigRepo(dbc)
 	userRepo := db.NewUserRepo(dbc)
 	pwiRepo := db.NewPasswordInfoRepo(dbc)
-	userManager := user.NewManager(userRepo, pwiRepo, db.TransactionFactory(dbc), user.ManagerOptions{})
+	userManager := manager.NewUserManager(userRepo, pwiRepo, cfgRepo, db.TransactionFactory(dbc), manager.ManagerOptions{})
 	refreshTokenRepo := db.NewRefreshTokenRepo(dbc)
 
 	sm := session.NewSessionManager(sRepo, skRepo)

server/email_verification.go

@@ -15,6 +15,7 @@ import (
 	"github.com/coreos/dex/pkg/log"
 	"github.com/coreos/dex/user"
 	useremail "github.com/coreos/dex/user/email"
+	"github.com/coreos/dex/user/manager"
 )
 
 // handleVerifyEmailResendFunc will resend an email-verification email given a valid JWT for the user and a redirect URL.
@@ -190,7 +191,7 @@ type emailVerifiedTemplateData struct {
 }
 
 func handleEmailVerifyFunc(verifiedTpl *template.Template, issuer url.URL, keysFunc func() ([]key.PublicKey,
-	error), userManager *user.Manager) http.HandlerFunc {
+	error), userManager *manager.UserManager) http.HandlerFunc {
 
 	return func(w http.ResponseWriter, r *http.Request) {
 		q := r.URL.Query()
@@ -217,12 +218,12 @@ func handleEmailVerifyFunc(verifiedTpl *template.Template, issuer url.URL, keysF
 		cbURL, err := userManager.VerifyEmail(ev)
 		if err != nil {
 			switch err {
-			case user.ErrorEmailAlreadyVerified:
+			case manager.ErrorEmailAlreadyVerified:
 				execTemplateWithStatus(w, verifiedTpl, emailVerifiedTemplateData{
 					Error:   "Invalid Verification Link",
 					Message: "Your email link has expired or has already been verified.",
 				}, http.StatusBadRequest)
-			case user.ErrorEVEmailDoesntMatch:
+			case manager.ErrorEVEmailDoesntMatch:
 				execTemplateWithStatus(w, verifiedTpl, emailVerifiedTemplateData{
 					Error:   "Invalid Verification Link",
 					Message: "Your email link does not match the email address on file. Perhaps you have a more recent verification link?",

server/invitation.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/coreos/dex/pkg/log"
 	"github.com/coreos/dex/user"
+	"github.com/coreos/dex/user/manager"
 	"github.com/coreos/go-oidc/jose"
 	"github.com/coreos/go-oidc/key"
 )
@@ -18,7 +19,7 @@ type invitationTemplateData struct {
 type InvitationHandler struct {
 	issuerURL              url.URL
 	passwordResetURL       url.URL
-	um                     *user.Manager
+	um                     *manager.UserManager
 	keysFunc               func() ([]key.PublicKey, error)
 	signerFunc             func() (jose.Signer, error)
 	redirectValidityWindow time.Duration
@@ -55,13 +56,13 @@ func (h *InvitationHandler) handleGET(w http.ResponseWriter, r *http.Request) {
 	}
 
 	_, err = h.um.VerifyEmail(invite)
-	if err != nil && err != user.ErrorEmailAlreadyVerified {
+	if err != nil && err != manager.ErrorEmailAlreadyVerified {
 		// Allow AlreadyVerified folks to pass through- otherwise
 		// folks who encounter an error after passing this point will
 		// never be able to set their passwords.
 		log.Debugf("error attempting to verify email: %v", err)
 		switch err {
-		case user.ErrorEVEmailDoesntMatch:
+		case manager.ErrorEVEmailDoesntMatch:
 			writeAPIError(w, http.StatusBadRequest, newAPIError(errorInvalidRequest,
 				"Your email does not match the email address on file"))
 			return

server/password.go

@@ -12,6 +12,7 @@ import (
 	"github.com/coreos/dex/session"
 	"github.com/coreos/dex/user"
 	useremail "github.com/coreos/dex/user/email"
+	"github.com/coreos/dex/user/manager"
 )
 
 type sendResetPasswordEmailData struct {
@@ -181,7 +182,7 @@ type resetPasswordTemplateData struct {
 type ResetPasswordHandler struct {
 	tpl       *template.Template
 	issuerURL url.URL
-	um        *user.Manager
+	um        *manager.UserManager
 	keysFunc  func() ([]key.PublicKey, error)
 }
 
@@ -237,7 +238,7 @@ func (r *resetPasswordRequest) handlePOST() {
 	cbURL, err := r.h.um.ChangePassword(r.pwReset, plaintext)
 	if err != nil {
 		switch err {
-		case user.ErrorPasswordAlreadyChanged:
+		case manager.ErrorPasswordAlreadyChanged:
 			r.data.Error = "Link Expired"
 			r.data.Message = "The link in your email is no longer valid. If you need to change your password, generate a new email."
 			r.data.DontShowForm = true

server/register.go

@@ -11,6 +11,7 @@ import (
 	"github.com/coreos/dex/pkg/log"
 	"github.com/coreos/dex/session"
 	"github.com/coreos/dex/user"
+	"github.com/coreos/dex/user/manager"
 	"github.com/coreos/go-oidc/oidc"
 )
 
@@ -222,7 +223,7 @@ func handleRegisterFunc(s *Server) http.HandlerFunc {
 	}
 }
 
-func registerFromLocalConnector(userManager *user.Manager, sessionManager *session.SessionManager, ses *session.Session, email, password string) (string, error) {
+func registerFromLocalConnector(userManager *manager.UserManager, sessionManager *session.SessionManager, ses *session.Session, email, password string) (string, error) {
 	userID, err := userManager.RegisterWithPassword(email, password, ses.ConnectorID)
 	if err != nil {
 		return "", err
@@ -237,7 +238,7 @@ func registerFromLocalConnector(userManager *user.Manager, sessionManager *sessi
 	return userID, nil
 }
 
-func registerFromRemoteConnector(userManager *user.Manager, ses *session.Session, email string, emailVerified bool) (string, error) {
+func registerFromRemoteConnector(userManager *manager.UserManager, ses *session.Session, email string, emailVerified bool) (string, error) {
 	if ses.Identity.ID == "" {
 		return "", errors.New("No Identity found in session.")
 	}