Fix some issues for oauth2 client (#166)

* Fix some issues for oauth2 client

Signed-off-by: Jose Fuentes <[email protected]>

* Better validation of credentials

Signed-off-by: Jose Fuentes <[email protected]>

Author: j-fuentes

Makefile

@@ -16,9 +16,9 @@ define LDFLAGS
 -X "github.com/jetstack/preflight/pkg/version.Commit=$(COMMIT)" \
 -X "github.com/jetstack/preflight/pkg/version.BuildDate=$(DATE)" \
 -X "github.com/jetstack/preflight/pkg/version.GoVersion=$(GOVERSION)" \
--X "github.com/jetstack/preflight/pkg/client.clientID=$(OAUTH_CLIENT_ID)" \
--X "github.com/jetstack/preflight/pkg/client.clientSecret=$(OAUTH_CLIENT_SECRET)" \
--X "github.com/jetstack/preflight/pkg/client.authServer=$(OAUTH_AUTH_SERVER)"
+-X "github.com/jetstack/preflight/pkg/client.ClientID=$(OAUTH_CLIENT_ID)" \
+-X "github.com/jetstack/preflight/pkg/client.ClientSecret=$(OAUTH_CLIENT_SECRET)" \
+-X "github.com/jetstack/preflight/pkg/client.AuthServerDomain=$(OAUTH_AUTH_SERVER_DOMAIN)"
 endef
 
 GO_BUILD:=go build -ldflags '$(LDFLAGS)'

pkg/agent/run.go

@@ -83,7 +83,7 @@ func gatherAndPostData(ctx context.Context) {
 
 	log.Printf("Loaded config: \n%s", dump)
 
-	var credentials *Credentials
+	var credentials *client.Credentials
 	if CredentialsPath != "" {
 		file, err = os.Open(CredentialsPath)
 		if err != nil {
@@ -93,7 +93,7 @@ func gatherAndPostData(ctx context.Context) {
 
 		b, err = ioutil.ReadAll(file)
 
-		credentials, err = ParseCredentials(b)
+		credentials, err = client.ParseCredentials(b)
 		if err != nil {
 			log.Fatalf("Failed to parse credentials file: %s", err)
 		}
@@ -105,7 +105,7 @@ func gatherAndPostData(ctx context.Context) {
 	var preflightClient *client.PreflightClient
 	if credentials != nil {
 		log.Printf("A credentials file was specified. Using OAuth2 authentication...")
-		preflightClient, err = client.New(agentMetadata, credentials.UserID, credentials.UserSecret, baseURL)
+		preflightClient, err = client.New(agentMetadata, credentials, baseURL)
 		if err != nil {
 			log.Fatalf("Error creating preflight client: %+v", err)
 		}

pkg/client/accessToken.go

@@ -35,10 +35,9 @@ func (c *PreflightClient) getValidAccessToken() (*accessToken, error) {
 }
 
 func (c *PreflightClient) renewAccessToken() error {
-	url := fmt.Sprintf("https://%s/oauth/token", authServer)
-	// TODO: audience will be dynamic in the future, but at the moment this client only sends readings.
-	audience := "https://preflight.jetstack.io/api/v1/datareading"
-	payload := fmt.Sprintf("grant_type=password&client_id=%s&client_secret=%s&audience=%s&username=%s&password=%s", clientID, clientSecret, audience, c.userID, c.userSecret)
+	url := fmt.Sprintf("https://%s/oauth/token", c.credentials.AuthServerDomain)
+	audience := "https://preflight.jetstack.io/api/v1"
+	payload := fmt.Sprintf("grant_type=password&client_id=%s&client_secret=%s&audience=%s&username=%s&password=%s", c.credentials.ClientID, c.credentials.ClientSecret, audience, c.credentials.UserID, c.credentials.UserSecret)
 	req, err := http.NewRequest("POST", url, strings.NewReader(payload))
 	if err != nil {
 		return errors.Trace(err)
@@ -57,6 +56,10 @@ func (c *PreflightClient) renewAccessToken() error {
 
 	defer res.Body.Close()
 
+	if status := res.StatusCode; status < 200 || status >= 300 {
+		return errors.Errorf("auth server did not provide an access token: (status %d) %s.", status, string(body))
+	}
+
 	response := struct {
 		Bearer    string `json:"access_token"`
 		ExpiresIn uint   `json:"expires_in"`
@@ -68,7 +71,7 @@ func (c *PreflightClient) renewAccessToken() error {
 	}
 
 	if response.ExpiresIn == 0 {
-		return fmt.Errorf("got wrong expiration for access token")
+		return errors.Errorf("got wrong expiration for access token")
 	}
 
 	c.accessToken.bearer = response.Bearer

pkg/client/client.go

@@ -11,16 +11,14 @@ import (
 )
 
 // These variables are injected at build time.
-
-var clientID string
-var clientSecret string
-var authServer string
+var ClientID string
+var ClientSecret string
+var AuthServerDomain string
 
 // PreflightClient can be used to talk to the Preflight backend.
 type PreflightClient struct {
 	// OAuth2
-	userID     string
-	userSecret string
+	credentials *Credentials
 	// accessToken is the current OAuth access token.
 	accessToken *accessToken
 
@@ -47,29 +45,34 @@ func NewWithBasicAuth(agentMetadata *api.AgentMetadata, authToken, baseURL strin
 }
 
 // New creates a new client that uses OAuth2.
-func New(agentMetadata *api.AgentMetadata, userID, userSecret, baseURL string) (*PreflightClient, error) {
-	if userID == "" || userSecret == "" {
-		return nil, fmt.Errorf("cannot create PreflightClient: neither userID or userSecret can be empty")
+func New(agentMetadata *api.AgentMetadata, credentials *Credentials, baseURL string) (*PreflightClient, error) {
+	if err := credentials.validate(); err != nil {
+		return nil, fmt.Errorf("cannot create PreflightClient: %v", err)
 	}
 	if baseURL == "" {
 		return nil, fmt.Errorf("cannot create PreflightClient: baseURL cannot be empty")
 	}
 
-	if clientID == "" || clientSecret == "" || authServer == "" {
-		return nil, fmt.Errorf("cannot create PreflightClient: this build does not have a valid OAuth client configuration")
+	if !credentials.IsClientSet() {
+		credentials.ClientID = ClientID
+		credentials.ClientSecret = ClientSecret
+		credentials.AuthServerDomain = AuthServerDomain
+	}
+
+	if !credentials.IsClientSet() {
+		return nil, fmt.Errorf("cannot create PreflightClient: invalid OAuth2 client configuration")
 	}
 
 	return &PreflightClient{
 		agentMetadata: agentMetadata,
-		userID:        userID,
-		userSecret:    userSecret,
+		credentials:   credentials,
 		baseURL:       baseURL,
 		accessToken:   &accessToken{},
 	}, nil
 }
 
 func (c *PreflightClient) usingOAuth2() bool {
-	return c.userID != ""
+	return c.credentials.UserID != ""
 }
 
 // PostDataReadings sends a slice of readings to Preflight.
@@ -96,7 +99,7 @@ func (c *PreflightClient) PostDataReadings(orgID string, readings []*api.DataRea
 		}
 		defer res.Body.Close()
 
-		return fmt.Errorf("Received response with status code %d. Body: %s", code, errorContent)
+		return fmt.Errorf("received response with status code %d. Body: %s", code, errorContent)
 	}
 
 	return nil

pkg/agent/credentials.go renamed to pkg/client/credentials.go

@@ -1,4 +1,4 @@
-package agent
+package client
 
 import (
 	"encoding/json"
@@ -13,11 +13,29 @@ type Credentials struct {
 	UserID string `json:"user_id"`
 	// UserSecret is the secret for the user or service account.
 	UserSecret string `json:"user_secret"`
+	// The following fields are optional as the default behaviour
+	// is to use the equivalent variables defined at package level
+	// and injected at build time.
+	// ClientID is the oauth2 client ID.
+	ClientID string `json:"client_id,omitempty"`
+	// ClientSecret is the oauth2 client secret.
+	ClientSecret string `json:"client_secret,omitempty"`
+	// AuthServerDomain is the domain for the auth server.
+	AuthServerDomain string `json:"auth_server_domain,omitempty"`
+}
+
+// IsClientSet returns whether the client credentials are set or not.
+func (c *Credentials) IsClientSet() bool {
+	return c.ClientID != "" && c.ClientSecret != "" && c.AuthServerDomain != ""
 }
 
 func (c *Credentials) validate() error {
 	var result *multierror.Error
 
+	if c == nil {
+		return fmt.Errorf("credentials are nil")
+	}
+
 	if c.UserID == "" {
 		result = multierror.Append(result, fmt.Errorf("user_id cannot be empty"))
 	}