client: Manager.New deals with public clients

* validation of client moved into its own method and tested
* public clients have different validation - must have no redirect URIs
  and must have a clientName set

client/client.go

@@ -15,12 +15,13 @@ import (
 )
 
 var (
-	ErrorInvalidClientID          = errors.New("not a valid client ID")
-	ErrorInvalidRedirectURL       = errors.New("not a valid redirect url for the given client")
-	ErrorCantChooseRedirectURL    = errors.New("must provide a redirect url; client has many")
-	ErrorNoValidRedirectURLs      = errors.New("no valid redirect URLs for this client.")
-	ErrorPublicClientRedirectURIs = errors.New("native clients cannot have redirect URIs")
-	ErrorPublicClientMissingName  = errors.New("native clients must have a name")
+	ErrorInvalidClientID       = errors.New("not a valid client ID")
+	ErrorInvalidRedirectURL    = errors.New("not a valid redirect url for the given client")
+	ErrorCantChooseRedirectURL = errors.New("must provide a redirect url; client has many")
+	ErrorNoValidRedirectURLs   = errors.New("no valid redirect URLs for this client.")
+
+	ErrorPublicClientRedirectURIs = errors.New("public clients cannot have redirect URIs")
+	ErrorPublicClientMissingName  = errors.New("public clients must have a name")
 
 	ErrorMissingRedirectURI = errors.New("no client redirect url given")
 

client/manager/manager.go

@@ -2,6 +2,7 @@ package manager
 
 import (
 	"encoding/base64"
+	"net/url"
 
 	"errors"
 
@@ -21,6 +22,10 @@ const (
 	maxSecretLength = 72
 )
 
+var (
+	localHostRedirectURL = mustParseURL("http://localhost:0")
+)
+
 type ClientOptions struct {
 	TrustedPeers []string
 }
@@ -67,6 +72,10 @@ func NewClientManager(clientRepo client.ClientRepo, txnFactory repo.TransactionF
 	}
 }
 
+// New creates and persists a new client with the given options, returning the generated credentials.
+// Any Credenials provided with the client are ignored and overwritten by the generated  ID and Secret.
+// "Normal" (i.e. non-Public) clients must have at least one valid RedirectURI in their Metadata.
+// Public clients must not have any RedirectURIs and must have a client name.
 func (m *ClientManager) New(cli client.Client, options *ClientOptions) (*oidc.ClientCredentials, error) {
 	tx, err := m.begin()
 	if err != nil {
@@ -74,11 +83,14 @@ func (m *ClientManager) New(cli client.Client, options *ClientOptions) (*oidc.Cl
 	}
 	defer tx.Rollback()
 
+	if err := validateClient(cli); err != nil {
+		return nil, err
+	}
+
 	err = m.addClientCredentials(&cli)
 	if err != nil {
 		return nil, err
 	}
-
 	creds := cli.Credentials
 
 	// Save Client
@@ -171,11 +183,15 @@ func (m *ClientManager) Authenticate(creds oidc.ClientCredentials) (bool, error)
 }
 
 func (m *ClientManager) addClientCredentials(cli *client.Client) error {
-	// Generate Client ID
-	if len(cli.Metadata.RedirectURIs) < 1 {
-		return errors.New("no client redirect url given")
+	var seed string
+	if cli.Public {
+		seed = cli.Metadata.ClientName
+	} else {
+		seed = cli.Metadata.RedirectURIs[0].Host
 	}
-	clientID, err := m.clientIDGenerator(cli.Metadata.RedirectURIs[0].Host)
+
+	// Generate Client ID
+	clientID, err := m.clientIDGenerator(seed)
 	if err != nil {
 		return err
 	}
@@ -192,3 +208,37 @@ func (m *ClientManager) addClientCredentials(cli *client.Client) error {
 	}
 	return nil
 }
+
+func validateClient(cli client.Client) error {
+	// NOTE: please be careful changing the errors returned here; they are used
+	// downstream (eg. in the admin API) to determine the http errors returned.
+	if cli.Public {
+		if len(cli.Metadata.RedirectURIs) > 0 {
+			return client.ErrorPublicClientRedirectURIs
+		}
+		if cli.Metadata.ClientName == "" {
+			return client.ErrorPublicClientMissingName
+		}
+		cli.Metadata.RedirectURIs = []url.URL{
+			localHostRedirectURL,
+		}
+	} else {
+		if len(cli.Metadata.RedirectURIs) < 1 {
+			return client.ErrorMissingRedirectURI
+		}
+	}
+
+	err := cli.Metadata.Valid()
+	if err != nil {
+		return client.ValidationError{Err: err}
+	}
+	return nil
+}
+
+func mustParseURL(s string) url.URL {
+	u, err := url.Parse(s)
+	if err != nil {
+		panic(err)
+	}
+	return *u
+}

client/manager/manager_test.go

@@ -168,3 +168,53 @@ func TestAuthenticate(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateClient(t *testing.T) {
+	tests := []struct {
+		cli     client.Client
+		wantErr error
+	}{
+		{
+			cli: client.Client{
+				Metadata: oidc.ClientMetadata{
+					RedirectURIs: []url.URL{mustParseURL("http://auth.google.com")},
+				},
+			},
+		},
+		{
+			cli:     client.Client{},
+			wantErr: client.ErrorMissingRedirectURI,
+		},
+		{
+			cli: client.Client{
+				Metadata: oidc.ClientMetadata{
+					ClientName: "frank",
+				},
+				Public: true,
+			},
+		},
+		{
+			cli: client.Client{
+				Metadata: oidc.ClientMetadata{
+					RedirectURIs: []url.URL{mustParseURL("http://auth.google.com")},
+					ClientName:   "frank",
+				},
+				Public: true,
+			},
+			wantErr: client.ErrorPublicClientRedirectURIs,
+		},
+		{
+			cli: client.Client{
+				Public: true,
+			},
+			wantErr: client.ErrorPublicClientMissingName,
+		},
+	}
+
+	for i, tt := range tests {
+		err := validateClient(tt.cli)
+		if err != tt.wantErr {
+			t.Errorf("case %d: want=%v, got=%v", i, tt.wantErr, err)
+		}
+	}
+}