From 077877ed54a9f70c359bc684ffd245e9ec715c99 Mon Sep 17 00:00:00 2001
From: Anatoly Milkov <anatoly.milko@gmail.com>
Date: Sun, 3 Jun 2018 01:27:36 -0500
Subject: [PATCH] refactor Github OAuth provider

---
 app/rest/auth/provider.go       |  3 ++-
 app/rest/auth/providers.go      |  6 +-----
 app/rest/auth/providers_test.go | 10 ++--------
 3 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/app/rest/auth/provider.go b/app/rest/auth/provider.go
index b6399832..e3748e96 100644
--- a/app/rest/auth/provider.go
+++ b/app/rest/auth/provider.go
@@ -48,7 +48,8 @@ type Params struct {
 type userData map[string]interface{}
 
 func (u userData) value(key string) string {
-	if val, ok := u[key]; ok {
+	// json.Unmarshal converts json "null" value to go's "nil", in this case return empty string
+	if val, ok := u[key]; ok && val != nil {
 		return fmt.Sprintf("%v", val)
 	}
 	return ""
diff --git a/app/rest/auth/providers.go b/app/rest/auth/providers.go
index 28600485..d046776c 100644
--- a/app/rest/auth/providers.go
+++ b/app/rest/auth/providers.go
@@ -35,7 +35,6 @@ func NewGoogle(p Params) Provider {
 
 // NewGithub makes github oauth2 provider
 func NewGithub(p Params) Provider {
-
 	return initProvider(p, Provider{
 		Name:        "github",
 		Endpoint:    github.Endpoint,
@@ -49,11 +48,8 @@ func NewGithub(p Params) Provider {
 				Picture: data.value("avatar_url"),
 			}
 			// github may have no user name, use login in this case
-			if userInfo.Name == "<nil>" {
-				userInfo.Name = data.value("login")
-			}
 			if userInfo.Name == "" {
-				userInfo.Name = userInfo.ID[0:16]
+				userInfo.Name = data.value("login")
 			}
 			return userInfo
 		},
diff --git a/app/rest/auth/providers_test.go b/app/rest/auth/providers_test.go
index 8a308ca7..d65bb295 100644
--- a/app/rest/auth/providers_test.go
+++ b/app/rest/auth/providers_test.go
@@ -32,17 +32,11 @@ func TestProviders_NewGithub(t *testing.T) {
 	assert.Equal(t, store.User{Name: "test user", ID: "github_e80b2d2608711cbb3312db7c4727a46fbad9601a",
 		Picture: "http://demo.remark42.com/blah.png", Admin: false, Blocked: false, IP: ""}, user, "got %+v", user)
 
-	// nil name in data
-	udata = userData{"login": "lll", "name": "<nil>", "avatar_url": "http://demo.remark42.com/blah.png"}
+	// nil name in data (json response contains `"name": null`); using login, it's always required
+	udata = userData{"login": "lll", "name": nil, "avatar_url": "http://demo.remark42.com/blah.png"}
 	user = r.MapUser(udata, nil)
 	assert.Equal(t, store.User{Name: "lll", ID: "github_e80b2d2608711cbb3312db7c4727a46fbad9601a",
 		Picture: "http://demo.remark42.com/blah.png", Admin: false, Blocked: false, IP: ""}, user, "got %+v", user)
-
-	// no name in data
-	udata = userData{"login": "lll", "avatar_url": "http://demo.remark42.com/blah.png"}
-	user = r.MapUser(udata, nil)
-	assert.Equal(t, store.User{Name: "github_e80b2d260", ID: "github_e80b2d2608711cbb3312db7c4727a46fbad9601a",
-		Picture: "http://demo.remark42.com/blah.png", Admin: false, Blocked: false, IP: ""}, user, "got %+v", user)
 }
 
 func TestProviders_NewFacebook(t *testing.T) {