internal: return error if no access_token present in server response

This behavior and test was introduced in 0ae3d4edc99f765dc1513a9605aa96e82877a20a.
It is not consistent with the other test introduced in the same commit,
where an incorrectly typed access_token does produce an error.  Since a
*Token with a blank AccessToken is invalid, it is allowing an invalid
token to be returned without error.

Cleans up some tests responding with invalid data.

Change-Id: I777eb7a82ef598dc9042542ae65f8dce6768902e
Reviewed-on: https://go-review.googlesource.com/85659
Reviewed-by: Andrew Bonventre <andybons@golang.org>

internal/token.go

@@ -6,6 +6,7 @@ package internal
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -250,6 +251,9 @@ func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string,
 	if token.RefreshToken == "" {
 		token.RefreshToken = v.Get("refresh_token")
 	}
+	if token.AccessToken == "" {
+		return token, errors.New("oauth2: server response missing access_token")
+	}
 	return token, nil
 }
 

internal/token_test.go

@@ -33,7 +33,8 @@ func TestRetrieveTokenBustedNoSecret(t *testing.T) {
 		if got, want := r.FormValue("client_secret"), ""; got != want {
 			t.Errorf("client_secret = %q; want empty", got)
 		}
-		io.WriteString(w, "{}") // something non-empty, required to set a Content-Type in Go 1.10
+		w.Header().Set("Content-Type", "application/json")
+		io.WriteString(w, `{"access_token": "ACCESS_TOKEN", "token_type": "bearer"}`)
 	}))
 	defer ts.Close()
 
@@ -85,7 +86,8 @@ func TestRetrieveTokenWithContexts(t *testing.T) {
 	const clientID = "client-id"
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		io.WriteString(w, "{}") // something non-empty, required to set a Content-Type in Go 1.10
+		w.Header().Set("Content-Type", "application/json")
+		io.WriteString(w, `{"access_token": "ACCESS_TOKEN", "token_type": "bearer"}`)
 	}))
 	defer ts.Close()
 

oauth2_test.go

@@ -278,12 +278,9 @@ func TestExchangeRequest_BadResponse(t *testing.T) {
 	}))
 	defer ts.Close()
 	conf := newConf(ts.URL)
-	tok, err := conf.Exchange(context.Background(), "code")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if tok.AccessToken != "" {
-		t.Errorf("Unexpected access token, %#v.", tok.AccessToken)
+	_, err := conf.Exchange(context.Background(), "code")
+	if err == nil {
+		t.Error("expected error from missing access_token")
 	}
 }
 
@@ -296,7 +293,7 @@ func TestExchangeRequest_BadResponseType(t *testing.T) {
 	conf := newConf(ts.URL)
 	_, err := conf.Exchange(context.Background(), "exchange-code")
 	if err == nil {
-		t.Error("expected error from invalid access_token type")
+		t.Error("expected error from non-string access_token")
 	}
 }