Check whether auth provider exists

The name of the auth provider is checked for existence rather panicking. refs #3
SkygearIO · Apr 11, 2016 · 9756437 · 9756437
1 parent 1fa0550
commit 9756437
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 6 deletions.
diff --git a/handler/auth.go b/handler/auth.go
@@ -139,7 +139,11 @@ func (h *SignupHandler) Handle(payload *router.Payload, response *router.Respons
 	} else if p.Provider != "" {
 		// Get AuthProvider and authenticates the user
 		log.Debugf(`Client requested auth provider: "%v".`, p.Provider)
-		authProvider := h.ProviderRegistry.GetAuthProvider(p.Provider)
+		authProvider, err := h.ProviderRegistry.GetAuthProvider(p.Provider)
+		if err != nil {
+			response.Err = skyerr.NewInvalidArgument(err.Error(), []string{"provider"})
+			return
+		}
 		principalID, authData, err := authProvider.Login(p.AuthData)
 		if err != nil {
 			response.Err = skyerr.NewError(skyerr.InvalidCredentials, "unable to login with the given credentials")
@@ -251,7 +255,11 @@ func (h *LoginHandler) Handle(payload *router.Payload, response *router.Response
 	if p.Provider != "" {
 		// Get AuthProvider and authenticates the user
 		log.Debugf(`Client requested auth provider: "%v".`, p.Provider)
-		authProvider := h.ProviderRegistry.GetAuthProvider(p.Provider)
+		authProvider, err := h.ProviderRegistry.GetAuthProvider(p.Provider)
+		if err != nil {
+			response.Err = skyerr.NewInvalidArgument(err.Error(), []string{"provider"})
+			return
+		}
 		principalID, authData, err := authProvider.Login(p.AuthData)
 		if err != nil {
 			response.Err = skyerr.NewError(skyerr.InvalidCredentials, "invalid authentication information")

diff --git a/handler/auth_test.go b/handler/auth_test.go
@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"testing"
 	"time"
 
@@ -231,6 +232,19 @@ func TestLoginHandlerWithProvider(t *testing.T) {
 			p.Database = txdb
 		})
 
+		Convey("login in non-existent provider", func() {
+			resp := r.POST(`{"provider": "com.non-existent", "auth_data": {"name": "johndoe"}}`)
+			So(resp.Body.Bytes(), ShouldEqualJSON, `{
+	"error": {
+		"code": 108,
+		"name": "InvalidArgument",
+		"info": {"arguments": ["provider"]},
+		"message": "no auth provider of name \"com.non-existent\""
+	}
+}`)
+			So(resp.Code, ShouldEqual, http.StatusBadRequest)
+		})
+
 		Convey("login in existing", func() {
 			userinfo := skydb.NewProvidedAuthUserInfo("com.example:johndoe", map[string]interface{}{"name": "boo"})
 			conn.userinfo = &userinfo
@@ -411,6 +425,19 @@ func TestSignupHandlerWithProvider(t *testing.T) {
 			p.Database = txdb
 		})
 
+		Convey("signs up with non-existent provider", func() {
+			resp := r.POST(`{"provider": "com.non-existent", "auth_data": {"name": "johndoe"}}`)
+			So(resp.Body.Bytes(), ShouldEqualJSON, `{
+	"error": {
+		"code": 108,
+		"name": "InvalidArgument",
+		"info": {"arguments": ["provider"]},
+		"message": "no auth provider of name \"com.non-existent\""
+	}
+}`)
+			So(resp.Code, ShouldEqual, http.StatusBadRequest)
+		})
+
 		Convey("signs up with user", func() {
 			resp := r.POST(`{"provider": "com.example", "auth_data": {"name": "johndoe"}}`)
 

diff --git a/handler/user.go b/handler/user.go
@@ -271,7 +271,11 @@ func (h *UserLinkHandler) Handle(payload *router.Payload, response *router.Respo
 
 	// Get AuthProvider and authenticates the user
 	log.Debugf(`Client requested auth provider: "%v".`, p.Provider)
-	authProvider := h.ProviderRegistry.GetAuthProvider(p.Provider)
+	authProvider, err := h.ProviderRegistry.GetAuthProvider(p.Provider)
+	if err != nil {
+		response.Err = skyerr.NewInvalidArgument(err.Error(), []string{"provider"})
+		return
+	}
 	principalID, authData, err := authProvider.Login(p.AuthData)
 	if err != nil {
 		response.Err = skyerr.NewError(skyerr.InvalidCredentials, "unable to login with the given credentials")

diff --git a/handler/user_test.go b/handler/user_test.go
@@ -15,6 +15,7 @@
 package handler
 
 import (
+	"net/http"
 	"testing"
 
 	"github.com/skygeario/skygear-server/handler/handlertest"
@@ -358,6 +359,19 @@ func TestUserLinkHandler(t *testing.T) {
 			p.UserInfo = &userInfo
 		})
 
+		Convey("link account with non-existent provider", func() {
+			resp := r.POST(`{"provider": "com.non-existent", "auth_data": {"name": "johndoe"}}`)
+			So(resp.Body.Bytes(), ShouldEqualJSON, `{
+	"error": {
+		"code": 108,
+		"name": "InvalidArgument",
+		"info": {"arguments": ["provider"]},
+		"message": "no auth provider of name \"com.non-existent\""
+	}
+}`)
+			So(resp.Code, ShouldEqual, http.StatusBadRequest)
+		})
+
 		Convey("link account", func() {
 			resp := r.POST(`{"provider": "com.example", "auth_data": {"name": "johndoe"}}`)
 			So(resp.Body.Bytes(), ShouldEqualJSON, `{}`)

diff --git a/plugin/provider/registry.go b/plugin/provider/registry.go
@@ -15,6 +15,7 @@
 package provider
 
 import (
+	"fmt"
 	"sync"
 )
 
@@ -41,9 +42,12 @@ func (r *Registry) RegisterAuthProvider(name string, p AuthProvider) {
 }
 
 // GetAuthProvider gets an AuthProvider from the registry.
-func (r *Registry) GetAuthProvider(name string) AuthProvider {
+func (r *Registry) GetAuthProvider(name string) (AuthProvider, error) {
 	r.mutex.RLock()
 	defer r.mutex.RUnlock()
-	provider := r.authProviders[name]
-	return provider
+	provider, ok := r.authProviders[name]
+	if !ok {
+		return nil, fmt.Errorf(`no auth provider of name "%s"`, name)
+	}
+	return provider, nil
 }